Skip to content

fix(agent-connector): detect bare npub mentions#621

Merged
erskingardner merged 3 commits into
masterfrom
pip/darkmatter-620
Jun 25, 2026
Merged

fix(agent-connector): detect bare npub mentions#621
erskingardner merged 3 commits into
masterfrom
pip/darkmatter-620

Conversation

@agent-p1p

@agent-p1p agent-p1p commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Teach agent-control inbound projection to treat p-tag-less bare @npub1… text as a self-mention.
  • Preserve existing p tag and nostr:<hex|npub> fallback handling.
  • Add regression coverage for bare @npub1… and glued foo@npub1… non-mentions.

Test Plan

  • cargo test -p agent-connector
  • just fast-ci

Closes #620


Open in Stage

Summary by CodeRabbit

  • Bug Fixes
    • Improved detection of self-mentions in inbound messages, including visible npub-style mentions and nostr: references.
    • Fixed cases where mentions could be missed or incorrectly counted due to formatting, token boundaries, or uppercase variants.
    • Reduced false positives so mentions only trigger when they appear as valid, visible references.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@agent-p1p, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 28 minutes and 59 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2b9ff5c1-6f92-4866-b453-7129deddab32

📥 Commits

Reviewing files that changed from the base of the PR and between 666b7bb and ee7679d.

📒 Files selected for processing (6)
  • crates/agent-connector/src/event_projection.rs
  • crates/agent-connector/src/tests.rs
  • crates/agent-control/src/lib.rs
  • integrations/openclaw/marmot/src/client.ts
  • integrations/openclaw/marmot/src/dispatch.ts
  • integrations/openclaw/marmot/src/inbound.ts

Walkthrough

The agent connector now recognizes visible bare @npub1… mentions in inbound messages, uses markdown-aware parsing for mention fallback, and adds regression tests for boundary, case, and self-account handling.

Changes

Agent connector mention fallback

Layer / File(s) Summary
Markdown dependency wiring
crates/agent-connector/Cargo.toml, crates/agent-connector/src/event_projection.rs
marmot-markdown is added to the crate, and event_projection.rs imports Block, Inline, and NostrHrp for mention parsing.
Mention fallback parsing
crates/agent-connector/src/event_projection.rs
message_mentions_account falls back to delimited nostr:<hex> scanning and visible @npub detection through markdown block and inline traversal.
Bare @npub tests
crates/agent-connector/src/tests.rs
A test helper exercises inbound_message_event_from_record, and new cases cover visible @npub1… matches, markdown boundary rejection, uppercase forms, and other-account handles.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

  • #617: The markdown-aware mention fallback now recognizes bare @npub1… mentions, matching the mention-classification gap described there.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR moves toward bare @npub mention support, but the new boundary logic does not fully match the shared visible-mention semantics. Reuse the shared mention classifier or align boundary and case rules exactly with marmot-markdown, then add regressions for slash, underscore, uppercase, and glued-text cases.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: fixing bare npub mention detection in agent-connector.
Out of Scope Changes check ✅ Passed All changes stay within agent-connector mention detection and related tests or dependency updates, with no unrelated scope visible.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pip/darkmatter-620

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@stage-review

stage-review Bot commented Jun 24, 2026

Copy link
Copy Markdown

Ready to review this PR? Stage has broken it down into 5 individual chapters for you:

Title
1 Update documentation for self-mention detection
2 Add markdown dependency and imports
3 Implement visible npub mention detection
4 Verify mention detection with regression tests
5 Other changes
Open in Stage

Chapters generated by Stage for commit ee7679d on Jun 24, 2026 10:44pm UTC.

@agent-p1p

Copy link
Copy Markdown
Contributor Author

Pip adversarial review.

BLOCKING:

  • Boundary check over-classifies non-visible @npub text as self-mentions: crates/agent-connector/src/event_projection.rs:72-80 uses only is_ascii_alphanumeric() as the left/right boundary test. The visible mention parser rejects [A-Za-z0-9_/] on both sides (crates/marmot-markdown/src/nostr.rs:85-96) and rejects uppercase bech32 (crates/marmot-markdown/tests/unit_nostr.rs:67-76, 90-93). This connector path will therefore set mentions_self = true for text like /@<self-npub>, a_@<self-npub>, @<self-npub>/tail, @<self-npub>_tail, and uppercase @NPUB..., even though marmot-markdown renders no mention. That violates the issue's “same visible mention semantics” requirement and can wake agent-control consumers on text that did not visibly address them. Please reuse/expose the markdown/app mention classifier, or make this helper use the exact same boundary and case rules, then add regression tests for slash/underscore and uppercase cases.

SUGGESTION:

  • Avoid leaving a second hand-rolled mention parser in agent-connector: crates/agent-connector/src/event_projection.rs:46-55. This PR is already close to the app/markdown semantics but not identical. A shared helper would keep AgentControlEvent::InboundMessage.mentions_self, notifications, and chat-list unread badges from drifting again.

NITPICK:

  • The new tests cover the happy @npub ... path and one glued-letter negative, but not the edge cases that define the visible-token boundary: crates/agent-connector/src/tests.rs:2399-2431. Add negative coverage for _ and / on both sides, uppercase @NPUB..., and a different-account @npub.
  • Current GitHub CI has Rust tests (2/4) red while Simulator and vectors is still pending. Local cargo test -p agent-connector passes 77/77, so this may be unrelated, but it must be resolved or re-run green before merge.

SUMMARY: The clean bare @npub1… happy path works and preserves the existing p-tag / nostr: fallbacks, but the new substring matcher does not match the visible mention tokenizer’s boundaries. That is a correctness issue in the behavior this PR is meant to fix.

VERDICT: REQUEST_CHANGES

SENSITIVE_PATHS: crates/agent-connector/src/event_projection.rs, crates/agent-connector/src/tests.rs (repo policy marks crates/agent-* sensitive).

@agent-p1p

agent-p1p commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Address-review updates are pushed and verified.

Commits:

  • 666b7bb — replaced the connector's hand-rolled @npub matcher with marmot-markdown parsing for visible npub references (@npub1… and nostr:npub1…), tightened the nostr:<hex> fallback boundaries, and added slash/underscore/uppercase/different-account tests.
  • ee7679d — addressed CodeRabbit: image alt text no longer counts as a visible mention, and the Rust/OpenClaw mentions_self contract docs no longer imply p-tag-only semantics.

Local verification:

  • cargo test -p agent-connector inbound_message_event -- --nocapture (13 passed)
  • cargo test -p agent-connector (80 passed)
  • cargo test -p agent-control (11 passed)
  • just fast-ci (passed)
  • npx --yes pnpm@9.15.9 install --frozen-lockfile && npx --yes pnpm@9.15.9 typecheck in integrations/openclaw/marmot (passed)

GitHub checks are green on ee7679d. Rust tests (2/4) had a transient unrelated darkmatter-cli::cli groups_leave_publishes_self_remove failure on an earlier run; the same test passed locally and the rerun passed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/agent-connector/src/event_projection.rs`:
- Around line 47-55: The mentions_self behavior in event_projection now includes
visible nostr:npub… and bare `@npub`… references without requiring a p tag, so the
public contract is stale. Update the InboundMessage documentation in
AgentControlEvent::InboundMessage and the corresponding client-side docs in
client.ts to describe mentions_self as a broader self-mention signal rather than
“p-tagged” only. Keep the docs aligned with the logic in
plaintext_has_nostr_hex_ref and the npub-based fallback so downstream consumers
do not assume a p tag is present when mentions_self is true.
- Around line 97-101: The mention detection in markdown_inlines_have_npub_ref is
incorrectly treating Inline::Image alt text as visible content, so images can
trigger mentions_self even when nothing is rendered. Update the Inline::Image
branch in event_projection.rs to skip recursing into alt text, and add a
regression test around markdown_inlines_have_npub_ref or mentions_self proving
that an image like ![...](...) with @<self-npub> does not count as a mention.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d7968d6b-3e6d-4417-b5a6-4729a940f2ef

📥 Commits

Reviewing files that changed from the base of the PR and between 5418ddf and 666b7bb.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • crates/agent-connector/Cargo.toml
  • crates/agent-connector/src/event_projection.rs
  • crates/agent-connector/src/tests.rs

Comment thread crates/agent-connector/src/event_projection.rs
Comment thread crates/agent-connector/src/event_projection.rs Outdated
@agent-p1p agent-p1p marked this pull request as ready for review June 24, 2026 22:53
@erskingardner erskingardner merged commit 5f919b6 into master Jun 25, 2026
20 checks passed
@erskingardner erskingardner deleted the pip/darkmatter-620 branch June 25, 2026 19:29
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.

Agent connector mentions_self ignores p-tag-less bare @npub mentions

2 participants