crueber/walhub

Watch

Fork 0

@username mentions: notification exists; rendered profile links missing in rendered bodies #440

New issue

Closed

opened 2026-09-13 13:23:43 +00:00 by crueber · 4 comments

crueber commented

2026-09-13 13:23:43 +00:00

Owner

@username mentions: server-side notification + rendered profile links

What's requested

Two linked behaviors for @name mentions in recorded issue comments/PR bodies (and review/thread comments), per the user's ask:

Notification: an @username in a recorded comment body or PR body triggers a server-side notification — the recipient's bell tray shows a mentioned message with a deep link to the thread.
Rendering: the rendered @username in the thread body links to that user's profile page — everywhere prescribed (issue comments, PR bodies, review comments), valid handles only, with conservative boundaries (no matches inside code spans/fences, emails, or @@/a@b token-adjacent forms).

Evidence — part 1 (notification) EXISTS and is correct

Parser: internal/identity/mentions.go — identity.ParseMentions, the 06 §3 grammar: code-strip, word-boundary regex, 50-token cap, dedupe/sort.
Emitters at write time, all three surfaces:
- issues: internal/issues/service.go:1174 (emitMentioned, called from open/comment)
- pulls: internal/pulls/service.go:1020 (PR opened body + comments)
- review: internal/review/review.go:316 (submitted reviews + thread comments)
Consumer validation + fan-out: internal/notify/emit.go — validPrincipal profile probe (users/<p>/profile.json, 404 = dropped), team expansion capped at 100, silent drop of invalid mentions.
Tray: web/src/pages/Notifications.jsx — reasonChip hot-styles mentioned/team_mention; threadHref deep-links to /{repo}/issues/{num} or /{repo}/pull/{num}; SSE live-prepend + unread badge.

Verified by execution: a throwaway probe test against the live tree returned users=[bob@example.com] teams=[acme/backend] for a mixed body — parse and emit machinery is green.

Evidence — part 2 (rendered profile links) DOES NOT EXIST

web/src/lib/render-md.js is the single markdown pipeline (marked → DOMPurify). It implements issue/PR ref autolinks (#N, issue #340) but has zero mention handling — grep -i mention web/src/lib/render-md.js is empty.
The only mention-adjacent UI is the advisory autocomplete datalist (web/src/pages/Mentions.jsx, CommentComposer.jsx); nothing links a rendered @name token to a profile.
Spec gap too: docs/features/06_notifications.md §3/§7 mandates parse/fan-out but never specifies rendered mention links; 08_ui_sdk.md doesn't either. This ticket is the spec amendment as well as the implementation.

Architecture notes

The principal grammar is email-or-username (ValidPrincipal, internal/identity/identity.go:136): usernames are the identity key since Forgejo #370; emails remain valid legacy spellings. The client-side rendered-link parser must match the same grammar — best shape is mirroring mentionTok in internal/identity/mentions.go as a pure, headless-testable function in web/src/lib/ (repo convention: render-md.js node-testable layers; the #N autolink layer at line ~170 is the exact pattern to copy).
Rendering happens client-side at render time in render-md.js's marked→resolve layer (the ref-autolink stage): rewrite matched @token text to <a href="/{principal}"> (profile page is route /:owner, web/src/index.jsx:86). Usernames are unambiguous; email-shaped tokens also link to /{principal} since /:owner resolves both spellings (matchPrincipal). @org/team tokens should NOT be linked (teams live under /:org/settings/...//:owner/teams/:slug — out of scope here unless trivial; planner's call, note it).
Boundaries must be conservative, identical to the server parser: skip fenced blocks + inline code spans (reuse the existing ref-autolink skipper's tag list), skip inside existing links/autolinks, require the same left boundary ([^A-Za-z0-9_@-] or start), trailing-punctuation trim like the server's .,;:!?")]} strip.
Render-valid ≠ mention-valid: the server drops unresolvable mentions silently; the renderer cannot probe profiles per token. Decision for planner (pick one, note it): (a) link ALL grammar-valid tokens and accept occasional dead profile links (GitHub behaves this way), or (b) none — link only from a participants-supplied allowlist. Recommend (a): deterministic, no extra fetches, matches "valid handles only, conservative boundaries."
Sanitizer: generated anchors are plain a + href relative — already in the DOMPurify allowlist, no config change (same reasoning as ref autolinks, render-md.js header).
Notification half needs NO code change; this ticket's implementation scope is the renderer + spec text + tests.

Acceptance criteria

web/src/lib/render-md.js gains a mention-autolink pass beside the #N ref-autolink pass, mirroring identity.ParseMentions boundaries (code spans/fences/links skipped, same left-boundary + trailing-punct rules)
Rendered @username (and legacy email principals) link to /{principal} in issue threads, PR threads, and review bodies (all consumers of renderBody)
Pure parser exported and covered by node --test headless tests (valid/invalid handles, code-span skip, email vs username, @@, a@b.com, boundary cases) — table mirrors internal/identity/mentions_test.go
docs/features/06_notifications.md §7 (and 08_ui_sdk.md renderer row) gain the rendered-mention-link contract
No server changes: notification fan-out already lands mentioned/team_mention with tray deep links (verified in evidence above)
No pinned-contract drift: hash twins and ref-autolink #N behavior byte-identical (existing tests stay green)

# @username mentions: server-side notification + rendered profile links ## What's requested Two linked behaviors for `@name` mentions in recorded issue comments/PR bodies (and review/thread comments), per the user's ask: 1. **Notification**: an `@username` in a recorded comment body or PR body triggers a server-side notification — the recipient's bell tray shows a `mentioned` message with a deep link to the thread. 2. **Rendering**: the rendered `@username` in the thread body links to that user's profile page — everywhere prescribed (issue comments, PR bodies, review comments), valid handles only, with conservative boundaries (no matches inside code spans/fences, emails, or `@@`/`a@b` token-adjacent forms). ## Evidence — part 1 (notification) EXISTS and is correct - Parser: `internal/identity/mentions.go` — `identity.ParseMentions`, the 06 §3 grammar: code-strip, word-boundary regex, 50-token cap, dedupe/sort. - Emitters at write time, all three surfaces: - issues: `internal/issues/service.go:1174` (`emitMentioned`, called from open/comment) - pulls: `internal/pulls/service.go:1020` (PR opened body + comments) - review: `internal/review/review.go:316` (submitted reviews + thread comments) - Consumer validation + fan-out: `internal/notify/emit.go` — `validPrincipal` profile probe (`users/<p>/profile.json`, 404 = dropped), team expansion capped at 100, silent drop of invalid mentions. - Tray: `web/src/pages/Notifications.jsx` — `reasonChip` hot-styles `mentioned`/`team_mention`; `threadHref` deep-links to `/{repo}/issues/{num}` or `/{repo}/pull/{num}`; SSE live-prepend + unread badge. **Verified by execution**: a throwaway probe test against the live tree returned `users=[bob@example.com] teams=[acme/backend]` for a mixed body — parse and emit machinery is green. ## Evidence — part 2 (rendered profile links) DOES NOT EXIST - `web/src/lib/render-md.js` is the single markdown pipeline (marked → DOMPurify). It implements issue/PR ref autolinks (`#N`, issue #340) but has **zero mention handling** — `grep -i mention web/src/lib/render-md.js` is empty. - The only mention-adjacent UI is the advisory autocomplete datalist (`web/src/pages/Mentions.jsx`, `CommentComposer.jsx`); nothing links a rendered `@name` token to a profile. - Spec gap too: `docs/features/06_notifications.md` §3/§7 mandates parse/fan-out but never specifies **rendered** mention links; `08_ui_sdk.md` doesn't either. This ticket is the spec amendment as well as the implementation. ## Architecture notes - **The principal grammar is email-or-username (`ValidPrincipal`, `internal/identity/identity.go:136`): usernames are the identity key since Forgejo #370; emails remain valid legacy spellings.** The client-side rendered-link parser must match the same grammar — best shape is mirroring `mentionTok` in `internal/identity/mentions.go` as a pure, headless-testable function in `web/src/lib/` (repo convention: `render-md.js` node-testable layers; the `#N` autolink layer at line ~170 is the exact pattern to copy). - Rendering happens client-side at render time in `render-md.js`'s marked→resolve layer (the ref-autolink stage): rewrite matched `@token` text to `<a href="/{principal}">` (profile page is route `/:owner`, `web/src/index.jsx:86`). Usernames are unambiguous; email-shaped tokens also link to `/{principal}` since `/:owner` resolves both spellings (`matchPrincipal`). `@org/team` tokens should NOT be linked (teams live under `/:org/settings/...`/`/:owner/teams/:slug` — out of scope here unless trivial; planner's call, note it). - Boundaries must be conservative, identical to the server parser: skip fenced blocks + inline code spans (reuse the existing ref-autolink skipper's tag list), skip inside existing links/autolinks, require the same left boundary (`[^A-Za-z0-9_@-]` or start), trailing-punctuation trim like the server's `.,;:!?")]}` strip. - **Render-valid ≠ mention-valid**: the server drops unresolvable mentions silently; the renderer cannot probe profiles per token. Decision for planner (pick one, note it): (a) link ALL grammar-valid tokens and accept occasional dead profile links (GitHub behaves this way), or (b) none — link only from a participants-supplied allowlist. Recommend (a): deterministic, no extra fetches, matches "valid handles only, conservative boundaries." - Sanitizer: generated anchors are plain `a + href` relative — already in the DOMPurify allowlist, no config change (same reasoning as ref autolinks, `render-md.js` header). - Notification half needs NO code change; this ticket's implementation scope is the renderer + spec text + tests. ## Acceptance criteria - [ ] `web/src/lib/render-md.js` gains a mention-autolink pass beside the `#N` ref-autolink pass, mirroring `identity.ParseMentions` boundaries (code spans/fences/links skipped, same left-boundary + trailing-punct rules) - [ ] Rendered `@username` (and legacy email principals) link to `/{principal}` in issue threads, PR threads, and review bodies (all consumers of `renderBody`) - [ ] Pure parser exported and covered by `node --test` headless tests (valid/invalid handles, code-span skip, email vs username, `@@`, `a@b.com`, boundary cases) — table mirrors `internal/identity/mentions_test.go` - [ ] `docs/features/06_notifications.md` §7 (and `08_ui_sdk.md` renderer row) gain the rendered-mention-link contract - [ ] No server changes: notification fan-out already lands `mentioned`/`team_mention` with tray deep links (verified in evidence above) - [ ] No pinned-contract drift: hash twins and ref-autolink `#N` behavior byte-identical (existing tests stay green)

crueber added the

labels

2026-09-13 13:24:15 +00:00

crueber added this to the v1 milestone

2026-09-13 13:24:15 +00:00

crueber removed the

webhooks

label

2026-09-13 13:24:26 +00:00

crueber commented

2026-09-13 13:25:13 +00:00

Author

Owner

User-requirement pin (from the filing request — verify all three are covered by this ticket's acceptance criteria)

Notification on record: @username in a recorded issue comment or PR body notifies that user — confirmed already implemented server-side (ParseMentions → notify emit); this ticket's job is to NOT regress it while adding rendering.
The bell shows the message: the tray must show the mention message content ('X mentioned you in repo#N') and deep-link to the thread — the investigation says Notifications.jsx already hot-styles mention reasons and deep-links; include a tray-rendering acceptance criterion (mention notification renders with message + link, unread state works) so it's pinned by test, not assumed.
Profile links everywhere: @username links to /:username in rendered comments AND PR bodies — this is the core gap this ticket fixes.

Also confirm the flagged decision points are answered or explicitly left to the planner: edit re-evaluation of mentions, self-mention behavior.

## User-requirement pin (from the filing request — verify all three are covered by this ticket's acceptance criteria) 1. **Notification on record**: `@username` in a recorded issue comment or PR body notifies that user — confirmed already implemented server-side (ParseMentions → notify emit); this ticket's job is to NOT regress it while adding rendering. 2. **The bell shows the message**: the tray must show the mention message content ('X mentioned you in repo#N') and deep-link to the thread — the investigation says Notifications.jsx already hot-styles mention reasons and deep-links; include a tray-rendering acceptance criterion (mention notification renders with message + link, unread state works) so it's pinned by test, not assumed. 3. **Profile links everywhere**: `@username` links to `/:username` in rendered comments AND PR bodies — this is the core gap this ticket fixes. Also confirm the flagged decision points are answered or explicitly left to the planner: edit re-evaluation of mentions, self-mention behavior.

crueber referenced this issue from a commit

2026-09-13 13:32:57 +00:00

Fix #440: @mention profile links in rendered bodies (renderer + spec, no server change)

crueber referenced this issue from a pull request that will close it,

2026-09-13 13:32:59 +00:00

Fix #440: mention profile links #443

crueber commented

2026-09-13 13:33:03 +00:00

Author

Owner

Fix ready for review: #443 (#443) — renderer + spec only, no server changes. linkifyMentions beside the ref pass (@user/@email → /{principal}; @org/team unlinked; decision (a)); 18/18 new tests green, refs byte-identical, vite build green. The 2 smoke.test.js failures are live-server probes failing identically on pristine main (left untouched).

Fix ready for review: #443 (https://git.packden.us/crueber/walhub/pulls/443) — renderer + spec only, no server changes. linkifyMentions beside the ref pass (@user/@email → /{principal}; @org/team unlinked; decision (a)); 18/18 new tests green, refs byte-identical, vite build green. The 2 smoke.test.js failures are live-server probes failing identically on pristine main (left untouched).

crueber commented

2026-09-13 13:36:01 +00:00

Author

Owner

Review: PR #443 (fix/issue-440) — @mention profile links

Verified in scratch worktree at 8b13c66 (node_modules symlinked, worktree removed after; main worktree untouched, still clean). No browser used — node tests + reasoning only, per instructions.

Test results

node --test web/test/unit/mentions-autolink.test.js: 18/18 pass
Full suite minus smoke: 934 pass / 0 fail (includes refs-autolink + markdown — #N behavior byte-identical, criterion 6 holds)
vite build: succeeds (2.24s)
smoke.test.js 2 failures are environmental, not PR-caused: something unrelated is listening on 127.0.0.1:8080 returning 401 for /, so the tests ran instead of skipping. Left the live instance alone.
Own adversarial probes (all confirmed by execution): @@x, a@b.com, x@bob, @bob@x, @.bob, @.. stay plain; @bob./(@bob), trim punct outside link; fences/code/existing-links/URLs skipped; bare jane@example.com keeps mailto with no profile link; @Amy@Example.COM folds the marked mailto shape into /{principal}; @BOB -> href /bob, display BOB.

The 10 checks

Grammar mirror — PASS with one noted superset. Left boundary (lookbehind = server's [^A-Za-z0-9_@-]/^, non-consuming = strictly better), trailing-punct set identical (.,;:!?"')]}}), skip set reuses the same SKIP_RE (a/code/pre/script/style, render-md.js:225), validMentionUsernameis byte-for-byteauth.ValidUsernamesemantics (1..64, no leading dot, not.., post-lowercase charset). @org/team (next==="/") and @bob@x (next==="@"`) correctly stay plain.
Href shape — PASS. Lowercased href + as-typed display matches matchPrincipal (identity/users.go:101, case-insensitive both spellings).
Marked-autolinked @email folding — PASS, verified by execution (bare addresses keep mailto, never a mention).
@org/team NOT linked — PASS, pinned by tests + documented as out of scope.
Decision (a) documented — PASS (code comment + 06 Decisions entry + 06 §7 row + 08 entries).
Ref #N byte-identical — PASS (existing tests green, side-by-side test).
All renderBody consumers inherit — PASS (ThreadTimeline.jsx:83 + all pages via renderBody -> renderMarkdownHtml -> linkifyMentions).
XSS — PASS. href charset excludes "<> and is escAttr'd; display text charset excludes <> &"', so no breakout (probed @bob"><img...> -> href="/bob", payload stays inert text pre-sanitizer).
Sanitizer untouched — PASS (PURIFY_CONFIG unchanged; href-only relative anchors already allowed).
No server changes — PASS (0 .go files; docs + web only; no new deps).

One follow-up (NOT a blocker, do NOT fix in this branch)

Pre-existing server gap, predates this PR: mentionTok (identity/mentions.go) matches only email-shaped or org/team tokens — a bare @bob never parses server-side, so it renders as a link (correct per this ticket) but emits no notification. The issue's notify evidence only probed the email spelling (bob@example.com), and doc 06 §3 says @<principal> (which per ValidPrincipal includes bare usernames). So doc §3 > server impl. Fixing it means extending mentionTok + cap-counting semantics + Go tests — a scoped server change that violates this ticket's 'No server changes' rule. Recommend merging this PR as-is and filing a server-side follow-up (extend mentionTok bare-username alternative or narrow §3 to email+team).

No pushes made — nothing in the branch needed fixing.

MERGE RECOMMENDATION: ready to merge.

## Review: PR #443 (fix/issue-440) — @mention profile links Verified in scratch worktree at 8b13c66 (node_modules symlinked, worktree removed after; main worktree untouched, still clean). No browser used — node tests + reasoning only, per instructions. ### Test results - `node --test web/test/unit/mentions-autolink.test.js`: **18/18 pass** - Full suite minus smoke: **934 pass / 0 fail** (includes refs-autolink + markdown — #N behavior byte-identical, criterion 6 holds) - `vite build`: **succeeds** (2.24s) - smoke.test.js 2 failures are **environmental, not PR-caused**: something unrelated is listening on 127.0.0.1:8080 returning 401 for /, so the tests ran instead of skipping. Left the live instance alone. - Own adversarial probes (all confirmed by execution): `@@x`, `a@b.com`, `x@bob`, `@bob@x`, `@.bob`, `@..` stay plain; `@bob.`/`(@bob),` trim punct outside link; fences/`code`/existing-links/URLs skipped; bare `jane@example.com` keeps mailto with no profile link; `@Amy@Example.COM` folds the marked mailto shape into `/{principal}`; `@BOB` -> href `/bob`, display `BOB`. ### The 10 checks 1. **Grammar mirror — PASS with one noted superset.** Left boundary (lookbehind = server's `[^A-Za-z0-9_@-]`/^`, non-consuming = strictly better), trailing-punct set identical (`.,;:!?"\')]}}`), skip set reuses the same `SKIP_RE` (`a/code/pre/script/style`, render-md.js:225), `validMentionUsername` is byte-for-byte `auth.ValidUsername` semantics (1..64, no leading dot, not `..`, post-lowercase charset). `@org/team` (`next==="/"`) and `@bob@x` (`next==="@"`) correctly stay plain. 2. **Href shape — PASS.** Lowercased href + as-typed display matches `matchPrincipal` (identity/users.go:101, case-insensitive both spellings). 3. **Marked-autolinked @email folding — PASS**, verified by execution (bare addresses keep mailto, never a mention). 4. **@org/team NOT linked — PASS**, pinned by tests + documented as out of scope. 5. **Decision (a) documented — PASS** (code comment + 06 Decisions entry + 06 §7 row + 08 entries). 6. **Ref #N byte-identical — PASS** (existing tests green, side-by-side test). 7. **All renderBody consumers inherit — PASS** (ThreadTimeline.jsx:83 + all pages via renderBody -> renderMarkdownHtml -> linkifyMentions). 8. **XSS — PASS.** href charset excludes `"<>` and is escAttr'd; display text charset excludes `<> &"'`, so no breakout (probed `@bob"><img...>` -> `href="/bob"`, payload stays inert text pre-sanitizer). 9. **Sanitizer untouched — PASS** (PURIFY_CONFIG unchanged; href-only relative anchors already allowed). 10. **No server changes — PASS** (0 .go files; docs + web only; no new deps). ### One follow-up (NOT a blocker, do NOT fix in this branch) **Pre-existing server gap, predates this PR:** `mentionTok` (identity/mentions.go) matches only email-shaped or `org/team` tokens — a bare `@bob` never parses server-side, so it renders as a link (correct per this ticket) but emits **no notification**. The issue's notify evidence only probed the email spelling (`bob@example.com`), and doc 06 §3 says `@<principal>` (which per ValidPrincipal includes bare usernames). So doc §3 > server impl. Fixing it means extending `mentionTok` + cap-counting semantics + Go tests — a scoped server change that violates this ticket's 'No server changes' rule. Recommend merging this PR as-is and filing a server-side follow-up (extend `mentionTok` bare-username alternative or narrow §3 to email+team). No pushes made — nothing in the branch needed fixing. **MERGE RECOMMENDATION: ready to merge.**

crueber referenced this issue

2026-09-13 13:36:09 +00:00

mentionTok never matches bare @username — renders-as-link but no notification #444

crueber closed this issue

2026-09-13 13:36:18 +00:00

crueber commented

2026-09-13 13:36:19 +00:00

Author

Owner

Fixed by PR #443 (review clean — all 10 checks pass, mirror exact, #N byte-identical; bare-@username notify gap filed as #444), merged. Closing.

crueber referenced this issue from a commit

2026-09-13 13:36:20 +00:00