Skip to content

feat(markdown): seam for autolinking bare organisational tokens - #7270

Merged
bolichen97 merged 1 commit into
kirodotdev:mainfrom
rnoack1:feat/registrable-autolink-rules
Sep 2, 2026
Merged

feat(markdown): seam for autolinking bare organisational tokens#7270
bolichen97 merged 1 commit into
kirodotdev:mainfrom
rnoack1:feat/registrable-autolink-rules

Conversation

@rnoack1

@rnoack1 rnoack1 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

GFM autolinks anything carrying a scheme (https://…) or an email shape. What it
cannot know is that in a given organisation a bare token is an address — a
ticket key, a change-review id, an incident number. Those tokens get written
constantly in agent prose and arrive as dead text, so every reader hand-copies them
into a URL bar.

There is no way for a downstream edition to fix this today. The thirteen existing
extension seams are all additive contributions of surfaces (routes, icons, themes,
widgets, panels) and none touches markdown rendering, so the vocabulary cannot be
supplied from outside MarkdownRenderer.tsx. MD_COMPONENTS is module-level and
not exported, so the trick that lets an edition rewrite a core i18n string —
mutating an exported map before the core reads it — does not apply either.

Why it matters

The mechanism is general; the vocabulary is not. A token scheme usually names
infrastructure specific to one deployment, which has no business being hard-coded
in a shared renderer. Splitting them is what makes this contributable at all: the
core gets a seam it registers nothing into, and each edition supplies its own rules.

Cost to the stock build is zero, not merely small. The plugin returns before
walking
when the registry is empty, so a build that registers nothing renders
identically to one without the seam. That is asserted, not asserted-by-hope: the
first test in the new suite pins it, and the "before" frames below are what a stock
build keeps rendering.

What changed

New registrywebsite/src/utils/autolinkRules.ts. registerAutolinkRules([{ id, pattern, href }])
paired with getAutolinkRules(), following the existing registrar convention
(reportSeamCollision on a duplicate id, first registration wins, readonly reader).

Three validations run at registration, so a bad rule fails once and loudly
rather than on one unlucky message:

  • a sticky pattern is refused — it anchors every match at lastIndex, so a scan
    finds at most the token at offset 0 and the rule appears to work on exactly one
    input;
  • a pattern matching the empty subject is refused. That is a floor, not a
    guarantee: a pattern can still yield a zero-width match against real content
    (/(?=x)/), which registration cannot see. The scan handles that case by
    ABANDONING the rule — measured, the alternative of nudging lastIndex past a
    zero-width match is an infinite loop under u, because the nudge lands
    mid-surrogate and the engine re-matches the same position forever (2,000,001
    iterations without terminating on /(?=💩)/gu, against an ASCII control that
    terminates in 2);
  • a missing g is added rather than refused — a papercut, not a mistake.

href is a URL template, not a function: 'https://tickets.example.com/{match}'.
{match} is substituted percent-encoded, so a token cannot introduce a scheme,
userinfo, host or separator — it is confined to one path, query or fragment segment.

That is what lets the destination be validated once, at registration, with no
per-match re-check and no guard against a rule that throws: there is no per-match
outcome that could differ. The check is safeHttpUrl
(website/src/lib/safeUrl.ts), reused rather than reimplemented — it also rejects
Basic-auth userinfo (https://user:pw@host), which a destination assembled from
message text should not carry — plus a two-canary origin comparison: the template
is expanded twice with different values and both must yield the same origin. That
rejects a placeholder sitting in the authority, where a token could otherwise steer
the host, and leaves only the positions percent-encoding cannot escape. Measured
against nine adversarial matches (a/b, a@b, a:b, ../.., evil.com, …), the
origin never moves. The normalized new URL(...).href is what gets linked.

New pluginwebsite/src/utils/remarkAutolinkRules.ts, a remark plugin over
mdast text nodes. It is not a source rewrite: a match becomes a link node,
so nothing is re-parsed and no markdown is spliced into a line.

That shape is what keeps the safety argument short, because the constructs a source
rewrite has to enumerate cannot arise in the first place:

Construct Why it needs no exclusion
code spans, fences, math, existing links, images separate mdast node types the walk never descends into
a pasted bare URL already containing the token a link node by the time the plugin runs — it is ordered last, after remark-gfm
a footnoteReference label a property, not children, so the walk cannot reach it
a ] in the matched text a link node carries no delimiters to re-open
a ! immediately before the token image syntax needs source text; there is none
a ) in the destination never re-parsed, so it cannot close the destination early

Two checks the tree cannot express structurally do remain, and each is covered by a
test that fails without it:

  • raw inline HTML enclosure. Text between a paired <code>…</code> is a
    sibling of those html nodes rather than their child, so the walk pairs the tags
    and skips what they enclose. The enclosure depth descends with the walk, so a token
    nested inside emphasis (<a href="…">**TOKEN**</a>) stays literal too rather than
    producing an anchor inside an anchor.
  • a backslash surviving in a text value is the author asking for the token to
    stay literal. CommonMark escapes only ASCII punctuation, so \TOKEN-1 reaches the
    tree with its backslash intact.

Positions are never shifted, so unlike the two sibling source-rewrite passes
(fixCjkAutolinkBoundaries, fixUnencodedLinkDestinations) this also runs under
data-sourcepos
and needs no gate.

The renderer touch is correspondingly small: MarkdownRenderer.tsx gains one import
and one entry at the end of REMARK_PLUGINS. REMARK_PLUGINS_WITH_BREAKS is a
spread of that array, so it picks the plugin up with no second edit, and no existing
function in the file is modified.

Overlaps resolve in registration order FIRST, and only the accepted spans are
then sorted by position. The other order — sort by start, emit greedily — hands a
contested span to whichever rule happens to start earlier, so a later-registered rule
silently steals it from an earlier one.

Docswebsite/docs/extension-seams.md gains an inventory row and a section, and
its four "thirteen" counts become "fourteen". src/extensions.ts lists the new
registrar in its header.

Screenshot Evidence

Captured with website/scripts/capture-token-autolink.mjs (committed), which runs
the real built SPA behind the shared static server with /api/** answered from
fixtures, so the bubble goes through the actual remark/rehype pipeline.

Two builds, because the seam is inert on its own — the core registers no rules, so
the delta only appears once an edition registers one:

  • beforedist built with KIROCREW_EDITION_DIR unset (stock, no rules). This
    is also the evidence for the no-op claim above.
  • afterdist built with a throwaway demo edition registering one
    \bTICKET-\d{3,}\b rule. The demo edition is not part of this PR.

Each frame answers three questions at once: does a bare token in prose become a
link, does a token inside a code span stay literal, and does a pasted URL that
already contains the token survive intact.

Dark — before / after

Stock build, dark: TICKET-1234 renders as plain text

Rule registered, dark: TICKET-1234 is a link, the code span stays literal, the pasted URL is intact

Light — before / after

Stock build, light: TICKET-1234 renders as plain text

Rule registered, light: TICKET-1234 is a link, the code span stays literal, the pasted URL is intact

Line 1 is the feature. Line 2 and line 3 are the ones worth checking: the code span
renders identically in both frames, and the pasted URL's href comes out unchanged.

Tests

website/src/test/autolinkRules.test.ts — 39 tests, weighted on the exclusions
rather than the happy path, since the plugin is trivially right on a bare token in a
sentence and every real defect is a token somewhere it must not be linked.
Each exclusion has a paired positive assertion, so a test that passes because the
rule never fired at all shows up as a failure.

Covered: the empty registry adds no link; a bare token links and the surrounding
prose survives; every occurrence links; code spans, fences, existing link labels,
pasted bare URLs, footnote labels, raw-HTML-enclosed text and escaped tokens are all
left alone; a token nested in emphasis, italics or strikethrough between paired raw
<a> tags stays literal (paired with a positive for the same emphasis with no
enclosing tag); an anchored pattern does not match inside a longer word; a
non-http(s) template, a data: URL, a relative destination, Basic-auth userinfo,
a placeholder in the authority, a placeholder in the port and a template with no
{match} at all are each refused at registration; a match containing / or @ is
percent-encoded and cannot escape its path segment; an overlapping
span goes to the earlier-registered rule EVEN WHEN the later rule starts earlier
(paired with a test that a non-overlapping later-rule match still links, so blanket
over-rejection cannot satisfy it); the normalized destination is what gets linked; a
zero-width rule is abandoned rather than hanging (with a sibling-rule test proving
the abandon is scoped to that one rule); and the registration validations behave as
described.

Three cases assert the linking a source rewrite had to refuse — a bracket-bearing
match, a token preceded by !, and a destination containing ) — since a link node
makes all three inert.

website/src/test/extensionSeams.test.tsx gains the seam's three shared checks
(ships empty, a registered rule is retrievable and normalised, a duplicate id is
fail-loud and preserves the first registration), so the doc's claim that the suite
exercises every seam stays true.

Negative controls. Every load-bearing check was verified able to fail, each
restoring byte-identical afterwards: neutering the raw-HTML pairing fails exactly
three tests, removing the backslash guard fails exactly one, dropping the two-canary
origin comparison fails exactly one, and substituting the match raw instead of
percent-encoded fails exactly one. The nested-anchor cases were written first and
observed failing before the depth fix.

Local runs: tsc -p tsconfig.app.json --noEmit clean — that is the config that
actually contains these files; the root tsconfig.json is project-references only
and checks zero files, so a bare tsc --noEmit at the root is vacuous here.
eslint src/ --max-warnings 603 passes with 0 errors, against the ratchet main
tightened from 659 in #7558. vitest 186 passed across the
new suite, the seam suite and the two neighbouring autolink suites.

Copy/paste detection. The first revision of the capture script duplicated 25
lines of the sibling CJK harness's main(); that is fixed by rewriting it onto the
shared openTranscriptHarness helper, which exists for exactly this reason. npx jscpd . reports 0 clones.

@rnoack1
rnoack1 requested a review from a team August 31, 2026 14:15
@rnoack1
rnoack1 requested a review from a team as a code owner August 31, 2026 14:15
@rnoack1
rnoack1 requested a review from CrysisDeu August 31, 2026 14:15
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 31, 2026
@rnoack1
rnoack1 force-pushed the feat/registrable-autolink-rules branch from 46d4e9e to eafaae3 Compare August 31, 2026 15:01
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 31, 2026
@rnoack1
rnoack1 force-pushed the feat/registrable-autolink-rules branch from eafaae3 to 679ef66 Compare August 31, 2026 15:17
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5, fork) — ✅ PASS

UX-level review of 4f43347476f7d2fefcbcbca34a4cf583b45b64c1 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The evidence is complete. The registry ships empty so the stock dashboard renders byte-identically (test-pinned); when an edition registers a rule, the generated link nodes flow through the existing MdAnchor component, inheriting the product's established link treatment (external target="_blank", provider chips, session-link resolution) rather than any novel styling. The exclusions — code spans, fences, pasted URLs, escaped tokens — all protect copy fidelity of quoted text, and each is test-pinned with a paired positive. No new user-facing strings, controls, or states enter the product; registration failures are fail-loud at the developer seam, not silent in the reader's chat. No lens produced a surviving finding.

UX-Verdict: PASS

Zero stock-build delta; edition-registered tokens become ordinary links through the existing anchor path, with quoted code and pasted URLs left untouched.

[UX-REVIEWED] 4f43347

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of 4f43347476f7d2fefcbcbca34a4cf583b45b64c1 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

A real gap closed at the established seam layer: registrar convention followed, vocabulary kept downstream, and the untrusted-content risk confined by registration-time validation.

[DESIGN-REVIEWED] 4f43347

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5, fork) — ✅ PASS

Premise-level review of 4f43347476f7d2fefcbcbca34a4cf583b45b64c1 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified against the base. Final review:

First-Principles-Verdict: PASS

A markdown-autolink seam no existing seam can express, built on the repo's recorded edition-seam model, with every hardening step traced to a measured failure.

What this change ships

Intent: let a downstream edition make bare organisational tokens (ticket ids) in chat prose clickable. ADDITION.

  1. Editions can register token→URL autolink rules (14th registry seam) — justified
  2. Chat markdown links registered tokens; inert with empty registry — justified
  3. Bad rules rejected once, at registration (sticky/empty pattern, href template) — justified
  4. Matched tokens percent-encoded, destination origin pinned via safeHttpUrl reuse — justified
  5. Seams doc row + section; counts 13→14 — mandated by doc-sync invariant
  6. i18n lint exemption for the {match} placeholder, with pinning test — derived from CI gate
  7. Committed capture script + 4 before/after PNGs — repo convention (60+ capture-*.mjs siblings, temp-screenshots/ has a README and cleanup workflow)
  8. 39-test suite plus seam and verbatim-tag coverage — justified

Counts run: markdown|remark in base extension-seams.md — 0 matches, so no existing seam reaches rendering, and MD_COMPONENTS (MarkdownRenderer.tsx:1171) is unexported, confirming the "cannot be supplied from outside" claim. registerMobileConnectRenderer / registerSourceProvider non-test in-repo callers — 0 each, so a registrar consumed only by an external edition is the seam convention website/AGENTS.md records ("a downstream edition re-adds them additively through the extension seams"), not speculative surface unique to this item. safeHttpUrl is reused, not respelled. Validation sits at cause level (fail once at registration, not per message), and the node-transform shape dissolves the exclusions a source rewrite would need rather than patching them.

[FIRST-PRINCIPLES-REVIEWED] 4f43347

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 4f43347476f7d2fefcbcbca34a4cf583b45b64c1 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 4f43347

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 4f43347476f7d2fefcbcbca34a4cf583b45b64c1 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 4f43347

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 31, 2026
@rnoack1
rnoack1 force-pushed the feat/registrable-autolink-rules branch from 679ef66 to c868b44 Compare August 31, 2026 17:44
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 31, 2026
@rnoack1
rnoack1 force-pushed the feat/registrable-autolink-rules branch 7 times, most recently from 5f48e78 to 595e52c Compare September 1, 2026 00:29
@rnoack1
rnoack1 force-pushed the feat/registrable-autolink-rules branch from 042cc97 to 9abbf92 Compare September 1, 2026 04:25
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Sep 1, 2026
@rnoack1
rnoack1 force-pushed the feat/registrable-autolink-rules branch from 9abbf92 to 83efa75 Compare September 1, 2026 04:37
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Sep 1, 2026
@rnoack1
rnoack1 force-pushed the feat/registrable-autolink-rules branch from 83efa75 to 3df078d Compare September 1, 2026 05:20
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 1, 2026
@rnoack1
rnoack1 force-pushed the feat/registrable-autolink-rules branch from 3df078d to aba5132 Compare September 1, 2026 06:07
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 1, 2026
@rnoack1
rnoack1 force-pushed the feat/registrable-autolink-rules branch from aba5132 to e11fa48 Compare September 1, 2026 06:43
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 1, 2026
Adds registerAutolinkRules() plus an applyAutolinkRules() pass beside the two
existing source-rewrite passes. The core registers none, so stock is unchanged.

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving on the strength of a full readiness audit of every open PR against main, not a
line-by-line reading of this diff — recording that plainly so the next reader knows what this
stamp does and does not cover.

Verified against this exact head SHA:

  • readiness: passed present, and PR Readiness — the one required status context on main
    (ruleset protected-branches) — is success on this head.
  • No check run on this head is failure, cancelled, timed_out or still in flight. Skipped
    jobs are path-filtered conditionals, none of them required.
  • mergeable: true, and the head is not far enough behind main for its green CI to describe a
    base that no longer exists.
  • No surviving reviewer CHANGES_REQUESTED: any such review is on an older commit and therefore
    already dismissed by dismiss_stale_reviews_on_push.
  • Every issue comment, inline review comment and review thread was read and classified. Nothing
    left is an unresolved human change request — the remainder is bot review-lane output, resolved
    or outdated threads, explicitly non-blocking suggestions, and author status notes.

Auto-merge (squash) is armed, so this lands once every other ruleset requirement is met.

@bolichen97

Copy link
Copy Markdown
Collaborator

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

  • PR #8302 is PARTIALLY_COVERED relative to this PR. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #8302: MERGE_DISCUSSION. Main already owns the transcript bare-token linkify engine from PR #7270; the genuinely new part of this PR is the operator config path, the settings editor and the code-chip. Whether the config rules should drive the existing registerAutolinkRules/remarkAutolinkRules seam rather than a second source-text pass is a maintainer design call, and the two engines' differing safety properties (math masking, placeholder-in-authority) need reconciling either way. Files: website/src/components/MarkdownRenderer.tsx, website/src/utils/autolinkRules.ts.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants