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

New issue

Closed

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

crueber commented

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

Owner

Follow-up flagged by the #440 review (PR #443 findings). The renderer (PR #443) links bare @username tokens, but server mentionTok (internal/identity/mentions.go) never matches bare @bob — so a bare-username mention renders as a profile link yet emits NO notification. Either extend mentionTok to bare usernames (scoped server change with tests mirroring mentions_test.go) or narrow the renderer to email-shaped tokens only. Decide and note it.

crueber referenced this issue

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

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

crueber commented

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

Author

Owner

This must allow bare usernames.

crueber referenced this issue from a commit

2026-09-13 13:50:10 +00:00

Fix #444: mentionTok matches bare @usernames (render/notify parity)

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

2026-09-13 13:50:12 +00:00

Fix #444: mentionTok bare usernames #452

crueber commented

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

Author

Owner

Fix up at #452 (branch fix/issue-444): decision (a) — extended mentionTok to bare @usernames on both server parsers (identity + issues-local) with renderer-mirrored guards; emitters unchanged in shape. -race green, coverage holds, node suites green. No merge — awaiting review.

crueber commented

2026-09-13 13:54:42 +00:00

Author

Owner

Review of PR #452 (fix/issue-444, ec53eb0) — verified in scratch worktree /tmp/pr452 (removed afterward). Main worktree left untouched (still clean; only pre-existing untracked .opencode/). No browser: no browser-facing change (server grammar + docs only; renderer untouched) — parity established by tests + direct probes below. No live instance/docker touched. No new deps (go.mod untouched).

(1) Two-parser finding: BOTH fixes genuinely needed, wiring correct. issues/service.go:1184 reads user mentions from the issues-local ParseMentions (refs.go:185) with a ValidPrincipal shape filter, and teams from identity.ParseMentions (service.go:1189). pulls/service.go:1027 and review/review.go:323 read both from identity.ParseMentions. Fixing only identity would have left issue-comment @bob silent; fixing only issues would have left PR/review @bob silent. Emitters unchanged in shape (actor exclusion, ValidPrincipal filter, team pass-through all intact). No finding.

(2) Grammar parity with web/src/lib/render-md.js linkifyMentionText — verified cell-by-cell with a 19-body probe harness driving BOTH parsers plus node linkifyMentionText on 17 inputs. All agree: @bob/@BOB notify AND link (href lowercased, display as typed); @@x, a@b.com, x@bob, x-@bob (the '-' the issues parser newly gains — old class [^A-Za-z0-9_@] → [^A-Za-z0-9_@-], now identical to identity + renderer lookbehind), trailing punct (@bob./(@zed)/@amy,), side-by-side @bob + @bob@example.com (email-first alternation holds whole), @bob/!, @bob@x, @.bob/@.. all plain on both sides; fenced+inline code skipped both sides; bare jane@example.com mailto-only both sides. The @a@b.com@c corner behaves exactly as documented (renders plain, parses as email a@b.com on both server parsers). No undocumented class found where server notifies but renderer doesn't link or vice versa. No finding.

(3) False-positive risk: LOW, contract intact. @-prefix + left boundary required — ordinary words never match. Shape gate (ValidPrincipal → auth.ValidUsername / mail parse) at parser (identity) and emitter (issues/pulls/review service.go), then the notify profile probe (emit.go:416 validPrincipal: 404 → drop) + team expansion silent-drop (emit.go:381 addTeam) + actor exclusion. Unresolvable bare names (@home, common words) render as dead profile links per the #440 decision-(a) contract and drop silently from fan-out — accepted trade-off, documented. Law 9 n/a confirmed (no auth/config surface). No finding.

(4) Team expansion unchanged: team alternative + ValidOrg/ValidSlug branch byte-identical; addTeam/validPrincipal untouched; @acme/backend → teams=[acme/backend] (identity), users=[] (issues) → issues emitter picks it up via the identity call. Probe-confirmed. No finding.

(5) Residuals documented honestly: all three doc residuals reproduced in probes (team notifies-never-renders; markdown-link interiors notify while renderer skips — see @bob → users=[bob] both parsers; @a@b.com@c renders plain / parses as email). Pre-existing shapes the bare extension inherits, none introduced. One additional pre-existing (not PR-introduced, not in doc, non-blocking): unpaired-backtick handling differs between stripMentionCode (blanks to EOL) and issues stripCode (literal) — only affects malformed input, out of scope. No action.

(6) Verification (scratch worktree): go test -race green on identity, issues, pulls, review, notify; coverage identity 95.4% / issues 96.3% (gate ≥95% holds; PR description says 95.7% for identity — I measured 95.4% non-race, trivial delta, gate holds either way); gofmt clean; go vet clean (all five packages); go build ./... clean; node suites green — mentions-autolink + markdown + md-urls + refs-autolink + blob-md = 78/78 pass. AGENTS.md laws: no new backend/npm deps, git-as-subprocess untouched, concurrency untouched, bucket/wire formats untouched, docs updated in same change (law 12 ✓), no silent-waiting/notification-spam regression.

No fixes pushed — nothing to fix. MERGE RECOMMENDATION: ready to merge.

Review of PR #452 (fix/issue-444, ec53eb0) — verified in scratch worktree /tmp/pr452 (removed afterward). Main worktree left untouched (still clean; only pre-existing untracked .opencode/). No browser: no browser-facing change (server grammar + docs only; renderer untouched) — parity established by tests + direct probes below. No live instance/docker touched. No new deps (go.mod untouched). (1) Two-parser finding: BOTH fixes genuinely needed, wiring correct. issues/service.go:1184 reads user mentions from the issues-local ParseMentions (refs.go:185) with a ValidPrincipal shape filter, and teams from identity.ParseMentions (service.go:1189). pulls/service.go:1027 and review/review.go:323 read both from identity.ParseMentions. Fixing only identity would have left issue-comment @bob silent; fixing only issues would have left PR/review @bob silent. Emitters unchanged in shape (actor exclusion, ValidPrincipal filter, team pass-through all intact). No finding. (2) Grammar parity with web/src/lib/render-md.js linkifyMentionText — verified cell-by-cell with a 19-body probe harness driving BOTH parsers plus node linkifyMentionText on 17 inputs. All agree: @bob/@BOB notify AND link (href lowercased, display as typed); @@x, a@b.com, x@bob, x-@bob (the '-' the issues parser newly gains — old class [^A-Za-z0-9_@] → [^A-Za-z0-9_@-], now identical to identity + renderer lookbehind), trailing punct (@bob./(@zed)/@amy,), side-by-side @bob + @bob@example.com (email-first alternation holds whole), @bob/!, @bob@x, @.bob/@.. all plain on both sides; fenced+inline code skipped both sides; bare jane@example.com mailto-only both sides. The @a@b.com@c corner behaves exactly as documented (renders plain, parses as email a@b.com on both server parsers). No undocumented class found where server notifies but renderer doesn't link or vice versa. No finding. (3) False-positive risk: LOW, contract intact. @-prefix + left boundary required — ordinary words never match. Shape gate (ValidPrincipal → auth.ValidUsername / mail parse) at parser (identity) and emitter (issues/pulls/review service.go), then the notify profile probe (emit.go:416 validPrincipal: 404 → drop) + team expansion silent-drop (emit.go:381 addTeam) + actor exclusion. Unresolvable bare names (@home, common words) render as dead profile links per the #440 decision-(a) contract and drop silently from fan-out — accepted trade-off, documented. Law 9 n/a confirmed (no auth/config surface). No finding. (4) Team expansion unchanged: team alternative + ValidOrg/ValidSlug branch byte-identical; addTeam/validPrincipal untouched; @acme/backend → teams=[acme/backend] (identity), users=[] (issues) → issues emitter picks it up via the identity call. Probe-confirmed. No finding. (5) Residuals documented honestly: all three doc residuals reproduced in probes (team notifies-never-renders; markdown-link interiors notify while renderer skips — [see @bob](http://x) → users=[bob] both parsers; @a@b.com@c renders plain / parses as email). Pre-existing shapes the bare extension inherits, none introduced. One additional pre-existing (not PR-introduced, not in doc, non-blocking): unpaired-backtick handling differs between stripMentionCode (blanks to EOL) and issues stripCode (literal) — only affects malformed input, out of scope. No action. (6) Verification (scratch worktree): go test -race green on identity, issues, pulls, review, notify; coverage identity 95.4% / issues 96.3% (gate ≥95% holds; PR description says 95.7% for identity — I measured 95.4% non-race, trivial delta, gate holds either way); gofmt clean; go vet clean (all five packages); go build ./... clean; node suites green — mentions-autolink + markdown + md-urls + refs-autolink + blob-md = 78/78 pass. AGENTS.md laws: no new backend/npm deps, git-as-subprocess untouched, concurrency untouched, bucket/wire formats untouched, docs updated in same change (law 12 ✓), no silent-waiting/notification-spam regression. No fixes pushed — nothing to fix. MERGE RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-13 13:54:58 +00:00

crueber commented

2026-09-13 13:54:58 +00:00

Author

Owner

Fixed by PR #452 (review clean — both parsers verified, cell-by-cell parity probed, false-positive risk low), merged. Closing.

crueber referenced this issue from a commit

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

Merge pull request 'Fix #444: mentionTok bare usernames' (#452) from fix/issue-444 into main