crueber/walhub

Watch

Fork 0

Comments/bodies: autolink #N to issues and PRN to pull requests (boundary-delimited, narrow) #340

New issue

Closed

opened 2026-09-11 17:26:34 +00:00 by crueber · 3 comments

crueber commented

2026-09-11 17:26:34 +00:00

Owner

What's requested

In issue comments (and issue/PR bodies), two narrowly-scoped autolinks:

#<integer> → links to that issue in the same repo (e.g. #3 → /{owner}/{repo}/issues/3).
PR<integer> / any capitalization of "PR" (pr3, Pr3, PR3) → links to that pull request (e.g. /{owner}/{repo}/pull/3).

Both match only when bounded by whitespace or punctuation (start/end of text, space, punctuation) — never inside a word, a URL, or an existing markdown link.

Scope rules (deliberately tight, per the request)

# must be followed by digits only — one or more. #3, #123 link; #3.2, #x3, #-3, #3abc do not (the #3abc case links #3? No — the integer run must terminate at the punctuation/whitespace boundary; #3abc is # followed by 3abc, which is not # + integer, so no link).
PR prefix: exactly the two letters PR in any case (pr, Pr, pR, PR) + digits, bounded as above. PR3 links; xPR3, PR3x do not. Note PR3abc does not link (the digits must end at the boundary).
Leading-zero forms (#003, pr007) — link as written, resolved numerically; decide and document (simplest: link them, the href is the numeric value).
Do not link inside existing markdown links or code spans. [see #3](x) and `#3` must pass through untouched. Inside fenced code blocks: untouched.
Do not link headings inside URLs: https://x/#3 must not double-process (the URL autolinker and this rule must not fight — order matters, see below).
Ambiguity note: #3 in a markdown heading (### 3) — heading markers are #s adjacent to a space, so ### never matches #+digit at a boundary; but a heading like # 3 days HAS # + space + 3, which does NOT match (integer must directly follow #). Document the boundary rule in tests.

Where it lives (code evidence)

Comment and body rendering goes through the single markdown pipeline: marked (GFM) → DOMPurify → HTML (web/src/lib/render-md.js:1-45, renderBody/renderMarkdownHtml). The autolink belongs as a marked extension or a post-marked/DOMPurify-safe text-node pass inside renderMarkdownHtml — the headless entry point — so every consumer (comments, issue body, PR body) gets it for free.
Ordering with the URL/relative-link resolution (#182/#185, same file :48-141): the #-reference pass must ignore content inside <a href> (marked may already have autolinked URLs) and inside <code>/<pre>. Cleanest: run the reference linkifier as a marked extension (tokenizer-level, so code spans/fences are already exempt), or as a DOM text-node walk between marked and DOMPurify that skips a/code/pre ancestors. Both are DOMPurify-safe; the extension approach keeps it headless-testable without a DOM — preferred given the module's headless contract.
The commit-message linkifier (linkifyBody, web/src/lib/diff.js:204) is a separate surface — whether commit bodies get #3 autolinks is out of scope for this ticket (flag it as a follow-up decision; the underlying helper should be written so it can be reused there).

Link target + rendering

#3 → <a href="/{owner}/{repo}/issues/3">#3</a>; PR3 → <a href="/{owner}/{repo}/pull/3">PR3</a> — preserve the author's original text exactly (case included: pr3 links as "pr3").
The renderer already receives the repo context (renderBody(src, {owner, repo, ref, dir}) — render-md.js), so building the href needs nothing new.
Dead references: linking #999 when issue 999 doesn't exist — still render the link (GitHub behavior; the 404 is honest and the author intent is clear). No existence check in the renderer (would cost a fetch per reference and the pipeline is synchronous/headless). Note this in the PR.
Sanitizer: generated anchors carry plain href — inside the DOMPurify allowlist already (http/https/mailto/relative kept, render-md.js:44). No allowlist change.

Acceptance criteria

#3 (and multi-digit) in a comment renders as a link to /issues/3; pr3/PR3/Pr3 render as links to /pull/3 — original text and case preserved.
Boundary rules hold: x#3, #3x, #3.2, xPR3, PR3x do NOT link; (#3), #3,, #3., end-of-text, and after whitespace DO.
Existing markdown links, inline code spans, and fenced code blocks containing #3/PR3 pass through unlinked.
URLs containing # fragments are not double-processed.
Works in issue comments, issue body, and PR body (all renderBody consumers) — one implementation, not per-page hacks.
Headless tests (node --test) covering the match table above, including the boundary and code-span cases; the render pipeline's DOMPurify output asserts the exact href shape.
Zero sanitizer changes required (anchors already allowed) — assert in a test.

## What's requested In issue comments (and issue/PR bodies), two narrowly-scoped autolinks: 1. **`#<integer>`** → links to that issue in the same repo (e.g. `#3` → `/{owner}/{repo}/issues/3`). 2. **`PR<integer>` / any capitalization of "PR"** (`pr3`, `Pr3`, `PR3`) → links to that pull request (e.g. `/{owner}/{repo}/pull/3`). Both match **only** when bounded by whitespace or punctuation (start/end of text, space, punctuation) — never inside a word, a URL, or an existing markdown link. ## Scope rules (deliberately tight, per the request) - `#` must be followed by **digits only** — one or more. `#3`, `#123` link; `#3.2`, `#x3`, `#-3`, `#3abc` do not (the `#3abc` case links `#3`? No — the integer run must terminate at the punctuation/whitespace boundary; `#3abc` is `#` followed by `3abc`, which is not `#` + integer, so no link). - `PR` prefix: exactly the two letters `PR` in any case (`pr`, `Pr`, `pR`, `PR`) + digits, bounded as above. `PR3` links; `xPR3`, `PR3x` do not. Note `PR3abc` does not link (the digits must end at the boundary). - Leading-zero forms (`#003`, `pr007`) — link as written, resolved numerically; decide and document (simplest: link them, the href is the numeric value). - **Do not link inside existing markdown links or code spans.** `[see #3](x)` and `` `#3` `` must pass through untouched. Inside fenced code blocks: untouched. - **Do not link headings inside URLs**: `https://x/#3` must not double-process (the URL autolinker and this rule must not fight — order matters, see below). - Ambiguity note: `#3` in a markdown heading (`### 3`) — heading markers are `#`s adjacent to a space, so `###` never matches `#`+digit at a boundary; but a heading like `# 3 days` HAS `#` + space + 3, which does NOT match (integer must directly follow `#`). Document the boundary rule in tests. ## Where it lives (code evidence) - Comment and body rendering goes through the single markdown pipeline: `marked` (GFM) → DOMPurify → HTML (`web/src/lib/render-md.js:1-45`, `renderBody`/`renderMarkdownHtml`). The autolink belongs as a **marked extension or a post-marked/DOMPurify-safe text-node pass inside `renderMarkdownHtml`** — the headless entry point — so every consumer (comments, issue body, PR body) gets it for free. - **Ordering with the URL/relative-link resolution** (#182/#185, same file :48-141): the `#`-reference pass must ignore content inside `<a href>` (marked may already have autolinked URLs) and inside `<code>`/`<pre>`. Cleanest: run the reference linkifier as a marked extension (tokenizer-level, so code spans/fences are already exempt), or as a DOM text-node walk between marked and DOMPurify that skips `a`/`code`/`pre` ancestors. Both are DOMPurify-safe; the extension approach keeps it headless-testable without a DOM — preferred given the module's headless contract. - The commit-message linkifier (`linkifyBody`, `web/src/lib/diff.js:204`) is a separate surface — whether commit bodies get `#3` autolinks is **out of scope** for this ticket (flag it as a follow-up decision; the underlying helper should be written so it can be reused there). ## Link target + rendering - `#3` → `<a href="/{owner}/{repo}/issues/3">#3</a>`; `PR3` → `<a href="/{owner}/{repo}/pull/3">PR3</a>` — preserve the author's original text exactly (case included: `pr3` links as "pr3"). - The renderer already receives the repo context (`renderBody(src, {owner, repo, ref, dir})` — `render-md.js`), so building the href needs nothing new. - **Dead references**: linking `#999` when issue 999 doesn't exist — still render the link (GitHub behavior; the 404 is honest and the author intent is clear). No existence check in the renderer (would cost a fetch per reference and the pipeline is synchronous/headless). Note this in the PR. - Sanitizer: generated anchors carry plain `href` — inside the DOMPurify allowlist already (`http/https/mailto/relative kept`, render-md.js:44). No allowlist change. ## Acceptance criteria - [ ] `#3` (and multi-digit) in a comment renders as a link to `/issues/3`; `pr3`/`PR3`/`Pr3` render as links to `/pull/3` — original text and case preserved. - [ ] Boundary rules hold: `x#3`, `#3x`, `#3.2`, `xPR3`, `PR3x` do NOT link; `(#3)`, `#3,`, `#3.`, end-of-text, and after whitespace DO. - [ ] Existing markdown links, inline code spans, and fenced code blocks containing `#3`/`PR3` pass through unlinked. - [ ] URLs containing `#` fragments are not double-processed. - [ ] Works in issue comments, issue body, and PR body (all `renderBody` consumers) — one implementation, not per-page hacks. - [ ] Headless tests (node --test) covering the match table above, including the boundary and code-span cases; the render pipeline's DOMPurify output asserts the exact href shape. - [ ] Zero sanitizer changes required (anchors already allowed) — assert in a test.

crueber added the

issues

pull requests

labels

2026-09-11 17:26:34 +00:00

crueber added this to the v1 milestone

2026-09-11 17:26:34 +00:00

crueber referenced this issue from a commit

2026-09-11 23:32:29 +00:00

Fix #340: autolink #N/PRN refs in comment and body markdown

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

2026-09-11 23:32:32 +00:00

Fix #340: autolink issue/PR refs #354

crueber commented

2026-09-11 23:32:37 +00:00

Author

Owner

Fixed by PR #354 (#354): #N/PRN autolinks in renderMarkdownHtml, thread pages pass {owner, repo} mdCtx. Notes: dead refs still link (honest 404, no per-ref fetch); leading zeros link as written with numeric hrefs (#003 → /issues/3); commit bodies (diff.js linkifyBody) flagged as follow-up — linkifyRefText is exported reusable for it.

Fixed by PR #354 (https://git.packden.us/crueber/walhub/pulls/354): #N/PRN autolinks in renderMarkdownHtml, thread pages pass {owner, repo} mdCtx. Notes: dead refs still link (honest 404, no per-ref fetch); leading zeros link as written with numeric hrefs (#003 → /issues/3); commit bodies (diff.js linkifyBody) flagged as follow-up — linkifyRefText is exported reusable for it.

crueber commented

2026-09-11 23:35:40 +00:00

Author

Owner

Review: PR #354 (fix/issue-340) — autolink #N/PRN

Reviewed in scratch worktree at origin/fix/issue-340 (main worktree untouched, still clean). No browser used — node tests + reasoning only, per instructions. No docker/system changes.

Match table — all verified adversarially (not just the test file)

Core: web/src/lib/render-md.js:206 (REF_RE = /(?<![A-Za-z0-9_&])(#\d+|[Pp][Rr]\d+)(?![A-Za-z0-9_]|\.[0-9])/g), core linkifyRefText (:213-221), walker linkifyIssueRefs (:228-274), pipeline wiring (:277-279, linkify runs AFTER resolveMarkdownUrls).

Link: #3, #123, PR3/pr3/Pr3/pR3, (#3), #3,, #3., EOL, after whitespace/newline — all link with exact hrefs (/o/r/issues/3, /o/r/pull/3), original text/case preserved.
No link: x#3, #3x, #3abc, #x3, #-3, # 3, xPR3, PR3x, PR3abc, aPR3b, #3_, #3.2 (lookahead \.[0-9] blocks the version run; greedy \d+ can't backtrack past it). #003→href /issues/3 as written; 30-digit runs stay literal (regex strip, no Number() → no 1e+30).
Exempt: `#3` / fences (skipped via <code>/<pre> depth counter), [see #3](x) / [PR3](x) (skipped via <a> depth), https://x.test/a#3 (single anchor, fragment intact — marked-autolinked URL sits inside <a>).
Headings: # 3 days and ### 3 never match (no # survives into the HTML — verified <h1>3 days</h1>/<h3>3</h3>); ##3 links the inner #3 (pinned in test, correct — second # is a punctuation boundary).
Walker sound: quote-aware tag scan (verified title="a>b" doesn't break parsing); attribute values containing #N (title="issue #3") pass through untouched;  untouched (whole comment consumed as one tag); uppercase <A HREF> skipped (case-insensitive SKIP_RE); nested <pre><code> skipped; unclosed non-skip tags still link (correct); & excluded from leading boundary so '/3 entities never corrupt (verified it's #3 → entity intact + ref links).
Ordering vs #182/#185: fresh /{o}/{r}/issues|pull/N hrefs are minted after the resolver runs, and the pass only ADDS anchors — verified file ctx test asserts no /blob/main/o/r/issues mangling. Resolver matrix untouched.
XSS: attacker-controlled ref text is regex-constrained to [#PRpr0-9]; escAttr (:208-210) escapes &<>" in owner/repo-derived hrefs (verified with quote-in-owner probe). Sanitizer config (PURIFY_CONFIG :33-44) byte-identical — zero changes, pinned by test.

Consumers — one finding, fixed

Audited all 7 renderBody call sites: ThreadTimeline (:83 via props.mdCtx — Issue.jsx:423 + Pull.jsx:655 pass {owner, repo} ✓), Blob/Tree/Release (full file ctx — refs link in prose, documented intent ✓), Repos bios ×2 (no ctx → stay plain; out of scope, safe degradation, OK).

FINDING (fixed): IssueNew.jsx preview advertised "(markdown; #N links issues)" but called renderBody(getBody()) without ctx → preview stayed plain until posted. Fix pushed to origin/fix/issue-340 (commit 7530f4f): preview passes { owner: ctx.owner, repo: ctx.name } (same shape as Issue.jsx; no ref/dir so relative URLs stay verbatim), wiring pin added to the test, doc clause updated per law 12.

Laws / docs / hygiene

Law 1: no package.json/lock changes — no new deps. Law 7: N/A (synchronous pure function, no goroutines/channels). Law 8: no seam changes, no upward imports. Law 12: docs/go/12_web_ui.md decision accurate (verified each claim against code); #328 entry intact at line 674 (diff is append-only there).

Test results

node --test web/test/unit/refs-autolink.test.js: 21/21 pass (incl. new IssueNew pin).
Full node --test web/test/unit/*.test.js: 685/688 — the 3 failures are all smoke.test.js live-server probes (WALHUB_TEST_WEB_BASE_URL → 127.0.0.1:8080, 503 with no server running); environmental, unrelated to this diff, same as PR description claims.
vite build + esbuild SDK bundle: green (chunk-size warning is advisory). Note: build deletes tracked web/dist/.keep — restored before commit, not part of the push.

MERGE RECOMMENDATION: ready to merge (after CI passes)

## Review: PR #354 (fix/issue-340) — autolink #N/PRN Reviewed in scratch worktree at `origin/fix/issue-340` (main worktree untouched, still clean). No browser used — node tests + reasoning only, per instructions. No docker/system changes. ### Match table — all verified adversarially (not just the test file) Core: `web/src/lib/render-md.js:206` (`REF_RE = /(?<![A-Za-z0-9_&])(#\d+|[Pp][Rr]\d+)(?![A-Za-z0-9_]|\.[0-9])/g`), core `linkifyRefText` (:213-221), walker `linkifyIssueRefs` (:228-274), pipeline wiring (:277-279, linkify runs AFTER `resolveMarkdownUrls`). - Link: `#3`, `#123`, `PR3/pr3/Pr3/pR3`, `(#3)`, `#3,`, `#3.`, EOL, after whitespace/newline — all link with exact hrefs (`/o/r/issues/3`, `/o/r/pull/3`), original text/case preserved. - No link: `x#3`, `#3x`, `#3abc`, `#x3`, `#-3`, `# 3`, `xPR3`, `PR3x`, `PR3abc`, `aPR3b`, `#3_`, `#3.2` (lookahead `\.[0-9]` blocks the version run; greedy `\d+` can't backtrack past it). `#003`→href `/issues/3` as written; 30-digit runs stay literal (regex strip, no `Number()` → no `1e+30`). - Exempt: `` `#3` `` / fences (skipped via `<code>/<pre>` depth counter), `[see #3](x)` / `[PR3](x)` (skipped via `<a>` depth), `https://x.test/a#3` (single anchor, fragment intact — marked-autolinked URL sits inside `<a>`). - Headings: `# 3 days` and `### 3` never match (no `#` survives into the HTML — verified `<h1>3 days</h1>`/`<h3>3</h3>`); `##3` links the inner `#3` (pinned in test, correct — second `#` is a punctuation boundary). - Walker sound: quote-aware tag scan (verified `title="a>b"` doesn't break parsing); attribute values containing `#N` (`title="issue #3"`) pass through untouched; `` untouched (whole comment consumed as one tag); uppercase `<A HREF>` skipped (case-insensitive `SKIP_RE`); nested `<pre><code>` skipped; unclosed non-skip tags still link (correct); `&` excluded from leading boundary so `'`/`3` entities never corrupt (verified `it's #3` → entity intact + ref links). - Ordering vs #182/#185: fresh `/{o}/{r}/issues|pull/N` hrefs are minted after the resolver runs, and the pass only ADDS anchors — verified `file` ctx test asserts no `/blob/main/o/r/issues` mangling. Resolver matrix untouched. - XSS: attacker-controlled ref text is regex-constrained to `[#PRpr0-9]`; `escAttr` (:208-210) escapes `&<>"` in owner/repo-derived hrefs (verified with quote-in-owner probe). Sanitizer config (`PURIFY_CONFIG` :33-44) byte-identical — zero changes, pinned by test. ### Consumers — one finding, fixed Audited all 7 `renderBody` call sites: ThreadTimeline (:83 via `props.mdCtx` — Issue.jsx:423 + Pull.jsx:655 pass `{owner, repo}` ✓), Blob/Tree/Release (full file ctx — refs link in prose, documented intent ✓), Repos bios ×2 (no ctx → stay plain; out of scope, safe degradation, OK). - FINDING (fixed): IssueNew.jsx preview advertised "(markdown; #N links issues)" but called `renderBody(getBody())` without ctx → preview stayed plain until posted. Fix pushed to `origin/fix/issue-340` (commit `7530f4f`): preview passes `{ owner: ctx.owner, repo: ctx.name }` (same shape as Issue.jsx; no ref/dir so relative URLs stay verbatim), wiring pin added to the test, doc clause updated per law 12. ### Laws / docs / hygiene - Law 1: no `package.json`/lock changes — no new deps. Law 7: N/A (synchronous pure function, no goroutines/channels). Law 8: no seam changes, no upward imports. Law 12: `docs/go/12_web_ui.md` decision accurate (verified each claim against code); #328 entry intact at line 674 (diff is append-only there). ### Test results - `node --test web/test/unit/refs-autolink.test.js`: 21/21 pass (incl. new IssueNew pin). - Full `node --test web/test/unit/*.test.js`: 685/688 — the 3 failures are all `smoke.test.js` live-server probes (`WALHUB_TEST_WEB_BASE_URL` → 127.0.0.1:8080, 503 with no server running); environmental, unrelated to this diff, same as PR description claims. - `vite build` + `esbuild` SDK bundle: green (chunk-size warning is advisory). Note: build deletes tracked `web/dist/.keep` — restored before commit, not part of the push. ### MERGE RECOMMENDATION: ready to merge (after CI passes)

crueber closed this issue

2026-09-11 23:36:05 +00:00

crueber referenced this issue from a commit

2026-09-11 23:36:06 +00:00

Merge pull request 'Fix #340: autolink issue/PR refs' (#354) from fix/issue-340 into main

crueber commented

2026-09-11 23:36:06 +00:00

Author

Owner

Fixed by PR #354 (review clean + one IssueNew preview-ctx fix by reviewer; adversarial match-table fully verified; 685/688 with only env smoke failures), merged. Closing.

crueber referenced this issue

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

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

crueber referenced this issue

2026-09-14 18:25:35 +00:00

Redesign PR conversation page to match the issue page's design language (header chip, markdown bodies, merged state) #521