Skip to content

fix(nostr): strip internal tool-trace banners from outbound text#103361

Open
miorbnli wants to merge 4 commits into
openclaw:mainfrom
miorbnli:fix/nostr-outbound-tool-trace-sanitize
Open

fix(nostr): strip internal tool-trace banners from outbound text#103361
miorbnli wants to merge 4 commits into
openclaw:mainfrom
miorbnli:fix/nostr-outbound-tool-trace-sanitize

Conversation

@miorbnli

@miorbnli miorbnli commented Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Nostr direct messages could expose internal assistant tool-failure banners and tool-call XML instead of sending only user-visible reply text.

Related: #90684

Why This Change Was Made

Nostr opts into the shared assistant-visible-text sanitizer through its standard outbound adapter hook. Its inbound DM reply callback also applies the same sanitizer because it delivers directly through the Nostr bus and bypasses the core outbound adapter pipeline.

The narrowed Nostr adapter type requires the sanitizer hook, preventing a later adapter refactor from silently dropping this delivery invariant while still type-checking. The direct callback suppresses trace-only output before Markdown conversion and relay delivery.

This rebase preserves current main's Nostr ingress architecture: runPassiveAccountLifecycle, dispatchInboundDirectDm, and its ingress lifecycle handoff remain intact. It does not restore the retired runtime dispatcher path.

User Impact

Nostr recipients see intended assistant prose without internal tool traces or XML scaffolding. Ordinary prose remains unchanged, and a reply containing only internal trace content does not produce an empty DM.

Evidence

  • Standard outbound regression coverage verifies the adapter's shared sanitizer for mixed text, XML tool calls, multiline tool-response scaffolding, and ordinary prose.
  • Inbound direct-reply regressions exercise the current lifecycle-aware dispatcher callback and verify mixed visible/trace content is sanitized before Markdown conversion while trace-only output invokes neither Markdown conversion nor the relay reply callback.
  • Rebased exact head: 4947413d6f0e713700156323ec57948e4d0d563e.
  • Exact-head upstream CI: success.

Risk checklist

  • One concern, one PR: prevent internal assistant scaffolding in Nostr outbound text.
  • Standard outbound and inbound direct-reply bypasses both use the same shared sanitizer.
  • Current ingress lifecycle and dispatcher contracts are preserved.
  • No config/env/default surface, SDK expansion, dependency, or CHANGELOG change.
  • Allow edits by maintainers.

@openclaw-barnacle openclaw-barnacle Bot added channel: nostr Channel integration: nostr size: XS labels Jul 10, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 10, 2026
@clawsweeper

clawsweeper Bot commented Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 19, 2026, 7:24 AM ET / 11:24 UTC.

Summary
The branch applies sanitizeAssistantVisibleText to Nostr’s shared outbound adapter and direct inbound-DM reply callback, suppressing replies that become empty after sanitization and adding regression coverage.

PR surface: Source +10, Tests +106. Total +116 across 3 files.

Reproducibility: yes. at source level: the standard adapter and direct inbound-DM callback are separate Nostr delivery paths, and the branch’s focused regressions exercise both with mixed and trace-only text. The supplied PR evidence also reports a successful exact-head runtime verification.

Review metrics: none identified.

Root-cause cluster
Relationship: canonical
Canonical: #103361
Summary: The related PR addresses the same Nostr outbound and inbound-DM sanitizer invariant; this PR is the viable canonical candidate because it is mergeable, rebased, and proof-backed while the other branch is dirty.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Ask an area owner to select this branch or the overlapping PR as the one canonical Nostr implementation.

Risk before merge

  • [P1] Merging both open Nostr PRs would create duplicate implementations and duplicate regression coverage for the same outbound and inbound-DM delivery invariant.
  • [P1] The sanitizer intentionally changes delivered Nostr text and suppresses trace-only replies; maintainers should retain this branch’s two-path coverage when choosing the canonical PR.

Maintainer options:

  1. Consolidate on this reviewed branch (recommended)
    Choose this clean branch as the sole Nostr implementation, retain both delivery-path tests, and close the overlapping dirty PR before merge.
  2. Pause for a behavior comparison
    If the overlapping branch may contain a unique relay-path behavior, compare its final diff before choosing either PR as canonical.

Next step before merge

  • [P2] A human should select the single canonical Nostr PR; no mechanical repair is indicated on this branch.

Maintainer decision needed

  • Question: Which of the two overlapping Nostr sanitizer PRs should be retained as the single canonical implementation?
  • Rationale: Both branches target the same user-visible delivery invariant, but the related branch is dirty while this one is mergeable and freshly rebased; choosing one avoids duplicate maintenance without requiring a new product direction.
  • Likely owner: steipete — The timeline shows this person was explicitly brought into the overlapping Nostr review discussion.
  • Options:
    • Adopt this PR as canonical (recommended): Retain this mergeable, proof-backed branch and close the overlapping dirty implementation after confirming no unique behavior remains there.
    • Adopt the other PR instead: Only choose the overlapping branch if an owner identifies a concrete unique behavior or proof gap that this branch does not cover.

Security
Cleared: The three-file diff adds no dependencies, workflows, credentials, permissions, generated artifacts, or new execution path; it only applies an existing SDK text sanitizer before delivery.

Review details

Best possible solution:

Select this mergeable, proof-backed branch as the canonical Nostr fix, preserve its shared-adapter and direct-reply coverage, then close the overlapping dirty PR rather than landing both variants.

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

Yes, at source level: the standard adapter and direct inbound-DM callback are separate Nostr delivery paths, and the branch’s focused regressions exercise both with mixed and trace-only text. The supplied PR evidence also reports a successful exact-head runtime verification.

Is this the best way to solve the issue?

Unclear pending canonicalization: this is a narrow, maintainable fix that uses the existing shared sanitizer, but maintainers should select it or the overlapping implementation rather than merge both.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: Internal tool scaffolding can reach real Nostr recipients through outbound and inbound-DM reply delivery.
  • merge-risk: 🚨 message-delivery: The patch changes text shaping and trace-only suppression immediately before Nostr relay delivery.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The supplied PR context records exact-head real runtime verification for the patched Nostr paths and successful exact-head CI; redact any private relay or credential details in future proof updates.
  • proof: sufficient: Contributor real behavior proof is sufficient. The supplied PR context records exact-head real runtime verification for the patched Nostr paths and successful exact-head CI; redact any private relay or credential details in future proof updates.
Evidence reviewed

PR surface:

Source +10, Tests +106. Total +116 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 11 1 +10
Tests 2 107 1 +106
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 118 2 +116

What I checked:

  • Direct reply path is covered: The changed direct-DM delivery callback sanitizes assistant text before Markdown conversion and returns without relaying a trace-only result, covering the path that bypasses the outbound adapter. (extensions/nostr/src/gateway.ts:198, 4947413d6f0e)
  • Shared outbound invariant is made explicit: The Nostr adapter type now requires sanitizeText, and the adapter opts into the shared sanitizer so a later adapter change cannot silently omit that hook while type-checking. (extensions/nostr/src/gateway.ts:25, 4947413d6f0e)
  • Regression coverage addresses both delivery surfaces: The branch adds standard outbound sanitizer cases and inbound lifecycle-aware callback cases, including the trace-only no-reply behavior. (extensions/nostr/src/outbound-tool-trace-sanitize.test.ts:1, 4947413d6f0e)
  • Real behavior and CI evidence: The PR body records exact-head real runtime verification and the supplied GitHub context reports the exact-head CI run as successful; the PR is labeled proof: sufficient. (4947413d6f0e)
  • Overlapping implementation needs consolidation: The related Nostr PR covers the same two reply paths and same trace-only suppression behavior, but is reported dirty; this PR is reported mergeable and has a newer rebased head. (4947413d6f0e)

Likely related people:

  • steipete: The PR timeline explicitly records a review mention during the overlapping-implementation discussion, making this person a likely routing point for canonicalization. (role: requested reviewer; confidence: low; files: extensions/nostr/src/gateway.ts)
  • vincentkoc: The PR timeline explicitly records a review mention during the overlapping-implementation discussion, making this person a likely routing point for canonicalization. (role: requested reviewer; confidence: low; files: extensions/nostr/src/channel.inbound.test.ts, extensions/nostr/src/gateway.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (8 earlier review cycles)
  • reviewed 2026-07-10T10:26:23.019Z sha a28fffb :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-10T12:58:21.809Z sha e143dd3 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-10T13:10:51.286Z sha e143dd3 :: needs real behavior proof before merge. :: [P1] Sanitize the inbound Nostr reply path
  • reviewed 2026-07-10T14:02:34.724Z sha 43283b4 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-13T12:43:43.092Z sha 08cbd5b :: needs maintainer review before merge. :: none
  • reviewed 2026-07-13T12:59:38.700Z sha 08cbd5b :: needs maintainer review before merge. :: none
  • reviewed 2026-07-19T06:18:05.716Z sha 4b278f0 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-19T11:02:20.633Z sha 4947413 :: needs maintainer review before merge. :: none

@miorbnli
miorbnli force-pushed the fix/nostr-outbound-tool-trace-sanitize branch from a28fffb to e143dd3 Compare July 10, 2026 12:09
@miorbnli

miorbnli commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

PR body Evidence updated with exact-head terminal output from the patched adapter/function hook (real runtime verification, not a shared helper). Branch rebased onto current main. Requesting fresh review.

@clawsweeper

clawsweeper Bot commented Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@miorbnli

miorbnli commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@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. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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. labels Jul 10, 2026
@miorbnli

miorbnli commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@miorbnli

miorbnli commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@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. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 10, 2026
@clawsweeper clawsweeper Bot added the status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. label Jul 10, 2026
@miorbnli
miorbnli force-pushed the fix/nostr-outbound-tool-trace-sanitize branch from 43283b4 to 1b7189e Compare July 13, 2026 11:39
@miorbnli

miorbnli commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@miorbnli
miorbnli force-pushed the fix/nostr-outbound-tool-trace-sanitize branch from 1b7189e to d73cfd0 Compare July 13, 2026 12:12
@miorbnli

miorbnli commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@miorbnli
miorbnli force-pushed the fix/nostr-outbound-tool-trace-sanitize branch from d73cfd0 to 08cbd5b Compare July 13, 2026 12:22
@miorbnli

miorbnli commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 13, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 16, 2026
Open
@miorbnli
miorbnli force-pushed the fix/nostr-outbound-tool-trace-sanitize branch 2 times, most recently from 4b278f0 to 4947413 Compare July 19, 2026 10:58
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Jul 19, 2026
@miorbnli

miorbnli commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Rebased exact head: 4947413d6f0e713700156323ec57948e4d0d563e.

The rebase preserves current Nostr ingress lifecycle/dispatcher contracts and the exact-head CI is green: https://github.com/openclaw/openclaw/actions/runs/29684297405

@clawsweeper

clawsweeper Bot commented Jul 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@miorbnli miorbnli mentioned this pull request Jul 22, 2026
Open
miorbnli and others added 4 commits July 24, 2026 08:49
@miorbnli @claude
fix(nostr): strip internal tool-trace banners from outbound text
427bcec 
The Nostr outbound adapter had no sanitizeText hook, so internal assistant
tool-trace scaffolding (e.g. "⚠️ 🛠️ `exec (agent)` failed", raw
<tool_call>/<function_response> XML) leaked verbatim into delivered Nostr DMs.
The same one-line sanitizeText hook was already shipped to 13 sibling channels
(twitch openclaw#103109, mattermost openclaw#98693, feishu openclaw#98705, signal openclaw#97360, slack openclaw#97367,
matrix openclaw#97372, irc openclaw#97214, telegram, googlechat openclaw#90684, qqbot openclaw#90132, discord,
whatsapp openclaw#71830, sms).

Add sanitizeText to the NostrOutboundAdapter type and adapter, matching those
precedents.

Co-Authored-By: Claude <[email protected]>
@miorbnli @claude
fix(nostr): sanitize tool-trace in gateway reply path too
71f1a5d 
The gateway inbound reply path (the deliver callback in startNostrGatewayAccount)
calls reply() directly with the assistant text, bypassing nostrOutboundAdapter's
sanitizeText hook. Internal tool-trace scaffolding therefore still leaked into
Nostr DMs via replies even with the adapter hook in place.

Apply sanitizeAssistantVisibleText in the deliver callback too, matching the
adapter hook, so both the outbound adapter path and the gateway reply path strip
internal scaffolding before delivery.

Co-Authored-By: Claude <[email protected]>
@miorbnli
test(nostr): cover sanitized inbound replies
cb47c84
@miorbnli
test(nostr): preserve inbound sanitizer coverage
fd575c0
@miorbnli
miorbnli force-pushed the fix/nostr-outbound-tool-trace-sanitize branch from 4947413 to fd575c0 Compare July 24, 2026 00:56
@clawsweeper

clawsweeper Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(nostr): strip internal tool-trace banners from outbound text This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

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

channel: nostr Channel integration: nostr merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P1 High-priority user-facing bug, regression, or broken workflow. 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.

1 participant

@miorbnli