feat: markdown @mention + external-link transforms (closes #81)#91
Merged
Conversation
Two transforms the spec declares but the pipeline doesn't yet apply — external-link target/rel rewriting and @mention resolution. Plan covers both Mdast (@mention) and HAST (external-link) plugin implementations, the API-side Fastify decorator that closes over siteHost + a state-bound resolver, and a new CFP_SITE_HOST env var. Closes #81. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the two custom transforms declared in
specs/behaviors/markdown-rendering.md:
- remarkMentions (Mdast): walks text nodes inside paragraphs / list
items / table cells / blockquotes; matches `@<slug>` with a word-start
check on the prior char (so `alice@chris.example` stays literal); for
matched slugs whose resolveMention returns true, splits the text node
into [prefix-text, link, suffix-text] and interpolates a link node
pointing at /members/<slug>. Inline + fenced code are tagged as
distinct Mdast nodes already, so walking only `text` nodes skips them
automatically.
- rehypeExternalLinks (HAST): walks anchor elements after sanitization;
parses href via a tight regex-based hostOf() helper (avoids dragging
DOM / @types/node into the shared package); anchors with a host
different from siteHost get target="_blank" rel="noopener nofollow".
Relative URLs and mailto:/tel:/etc. are always internal.
renderMarkdown(source, opts?) signature now takes an optional
RenderMarkdownOptions { siteHost?, resolveMention? }. No-opts call
preserves existing behavior exactly. Tests: 16 new cases covering both
transforms; 69/69 pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New markdown plugin (apps/api/src/plugins/markdown.ts) calls setRenderMarkdown(...) on a module-level binding in services/serializers/common.ts, closing over CFP_SITE_HOST + a state-bound resolveMention. Every serializer now imports renderMarkdown from common.ts (not @cfp/shared directly), so all *Html fields in API responses get the spec'd transforms applied. Module-level binding is justified over per-call threading: serializers are pure functions without a Fastify reference, and threading the decorator through every call site would have meant updating every serializer signature for what's a single per-process binding. Plugin re-runs at every buildApp() so tests stay isolated; the default fallback is the bare @cfp/shared renderer so callers without the plugin (scripts, unit tests of serializers) still work. Adds CFP_SITE_HOST env (default codeforphilly.org); preview.test.ts gains 2 integration cases (foreign-link rewrite + seeded @chris mention resolution). Renames the unused renderField helper out of existence — its only caller was deleted in PR #57. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The new env var that the markdown renderer's external-link transform uses. Defaults baked into the base configmap (codeforphilly.org); the sandbox overlay patches it to next-v2.codeforphilly.org so local-link heuristics match the live host. Env table in deploy.md gets a documented row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closeout: flip to done, tick validations (all green: 16 new shared tests, 2 new integration tests, 244/244 API tests, lint + type-check clean), fill Notes (boot-installed renderer trade-off, regex-based hostOf vs URL constructor, dead-code renderField cleanup) and Follow-ups (mention caching, image proxying, configurable mention paths). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI failure on PR #91: ENOTEMPTY rmdir on `.git/objects/` during the seedRawToml cleanup. Linux ext4 + git background pack work can race a synchronous rm() right after `git push`. macOS APFS doesn't expose this ordering. Adds maxRetries: 5, retryDelay: 100 to the three post-seed working-tree cleanups (seedRawToml + the test-repo / test-full-repo bootstrappers). Retries are no-ops on systems that succeed first try. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The two custom transforms declared in
specs/behaviors/markdown-rendering.mdthat the pipeline didn't yet apply:How it's wired
`@cfp/shared`'s `renderMarkdown(source, opts?)` takes an optional `{ siteHost?, resolveMention? }`. No-args calls preserve existing behavior — every pre-existing test passes unchanged.
API serializers route through a new module-level renderer in `apps/api/src/services/serializers/common.ts` that the new `markdownPlugin` installs at boot via `setRenderMarkdown(...)`. The plugin closes over `CFP_SITE_HOST` (new env, defaulting to `codeforphilly.org`; sandbox overlay patches to `next-v2.codeforphilly.org`) plus a resolver that consults the live state Map.
Threaded-decorator alternative was rejected because serializers are pure functions without a Fastify reference — boot-installed module binding matches the per-process runtime shape and keeps test isolation via per-`buildApp()` re-binding.
Test plan
Closes #81.
🤖 Generated with Claude Code