text/markdown accept header support - #5131
Conversation
📝 WalkthroughWalkthroughThe frontend adds content-negotiated Markdown rewrites, a dedicated Markdown route and renderer, typed document builders, YAML serialization, a notebook builder, and proxy exclusion for Markdown paths. ChangesMarkdown delivery
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front_end/src/proxy.ts`:
- Around line 213-219: Update the Proxy matcher configuration around source to
add a missing-header condition that excludes requests negotiated with Accept:
text/markdown, allowing canonical Markdown requests to bypass Proxy before the
beforeFiles rewrite. Preserve existing exclusions, and add integration coverage
for canonical Markdown requests with authentication both enabled and disabled.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b0a9ca46-0f4c-4d11-b735-b4aeb64551f5
⛔ Files ignored due to path filters (1)
front_end/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
front_end/next.config.mjsfront_end/package.jsonfront_end/src/agent_markdown/respond.tsfront_end/src/agent_markdown/routes.mjsfront_end/src/agent_markdown/types.tsfront_end/src/app/(md)/md/[type]/[[...args]]/route.tsfront_end/src/proxy.ts
| // | ||
| // `md` is excluded because markdown responses are anonymous and public: | ||
| // no token verification, CSP nonce or experiment enrollment is needed. | ||
| // agent_markdown/respond.ts re-checks the access gates enforced above, | ||
| // which is what keeps that safe. | ||
| source: | ||
| "/((?!api|_next/static|_next/image|favicon.ico|sitemap.xml|robots.txt|csp-report|questions/embed|experiments/embed|opengraph-image-|twitter-image-|app-version|.*\\..*).*)", | ||
| "/((?!api|md|_next/static|_next/image|favicon.ico|sitemap.xml|robots.txt|csp-report|questions/embed|experiments/embed|opengraph-image-|twitter-image-|app-version|.*\\..*).*)", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)front_end/(src/proxy.ts|next.config.mjs|.*embed|markdown|readme|md)' || true
echo "== proxy outline =="
if [ -f front_end/src/proxy.ts ]; then
wc -l front_end/src/proxy.ts
ast-grep outline front_end/src/proxy.ts --match Proxy --view expanded || true
sed -n '1,280p' front_end/src/proxy.ts
fi
echo "== next config =="
if [ -f front_end/next.config.mjs ]; then
wc -l front_end/next.config.mjs
sed -n '1,260p' front_end/next.config.mjs
fi
echo "== markdown accept/header checks =="
rg -n "Accept|text/markdown|markdown|next-mdx-remote|BeforeFiles|rewrite|missing|md/" front_end -S || trueRepository: Metaculus/metaculus
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== agent_markdown files outline =="
for f in front_end/src/agent_markdown/routes.mjs front_end/src/agent_markdown/respond.ts front_end/src/app/\(md\)/md/\[type\]/\[\[...args\]\]/route.ts; do
if [ -f "$f" ]; then
echo "-- $f --"
wc -l "$f"
sed -n '1,220p' "$f"
fi
done
echo "== Markdown rewrite generation =="
python3 - <<'PY'
from pathlib import Path
path = Path("front_end/src/agent_markdown/routes.mjs")
text = path.read_text()
print("contains text/markdown value:", '".*text/markdown.*"' in text)
print("uses missing header:", "missing:" in text)
print("uses beforeFiles:", "beforeFiles:" in text)
print("\nfirst 140 lines:")
PY
sed -n '1,140p' front_end/src/agent_markdown/routes.mjs
echo "== package Next versions =="
node - <<'JS'
const fs = require('fs');
for (const name of ['package.json','bun.lock']) {
const p = `front_end/${name}`;
if (fs.existsSync(p)) {
const txt = fs.readFileSync(p, 'utf8');
const line = (txt.split('\n').find(l => l.includes('"next"') || l.includes("'next'")) || "").split(/\n/)[0];
console.log(name, line);
}
}
JS
echo "== check Next docs on rewrites beforeFiles (static text from installed package if present) =="
for d in front_end/node_modules/next dist/node_modules/next; do
[ -d "$d" ] || continue
echo "-- $d --"
rg -n "beforeFiles|re-ordered|rewrites|middleware|headers|before-rewrites|after-rewrites" "$d" -S --max-count 30 || true
done
echo "== deterministic matcher model based on source comments =="
python3 - <<'PY'
import re
source = r"/((?!api|md|_next/static|_next/image|favicon.ico|sitemap.xml|robots.txt|csp-report|questions/embed|experiments/embed|opengraph-image-|twitter-image-|app-version|.*\..*).*)"
# A request path starts with /md exactly in this pattern unless later missing/header conditions apply.
paths = ["/md/notebook/1/", "/notebooks/1/", "/api/foo", "/favicon.ico", "/questions/embed/1/"]
pat = re.compile(source.lstrip("/") + "$")
# Direct route with path starting with /md.
print("direct /md/notebook/1/ directly matches matcher?", bool(pat.match("/md/notebook/1/")))
# Rewritten canonical Markdown request pathname before rewrite = "/notebooks/1/"; after rewrite = "/md/...".
print("canonical /notebooks/1/ starts /md?", "/notebooks/1/".startswith("/md"))
PYRepository: Metaculus/metaculus
Length of output: 10164
🌐 Web query:
Next.js middleware runs before filesystem routes beforeFiles rewrites API reference
💡 Result:
In Next.js, middleware (or its successor, proxy) runs before filesystem routes and beforeFiles rewrites in the request execution lifecycle [1][2][3][4]. The standard execution order for incoming requests is as follows [1][3][4]: 1. headers from next.config.js 2. redirects from next.config.js 3. Middleware (or proxy) (rewrites, redirects, etc.) 4. beforeFiles rewrites from next.config.js 5. Filesystem routes (public/, _next/static/, pages/, app/, etc.) 6. afterFiles rewrites from next.config.js 7. Dynamic Routes (/blog/[slug]) 8. fallback rewrites from next.config.js Because middleware executes before beforeFiles rewrites, it can intercept and modify requests before any filesystem-based routing or configuration-based rewrites are applied [1][3]. This allows middleware to dynamically handle routing logic, such as authentication, A/B testing, or URL masking, with higher priority than standard next.config.js rewrites [1][5][6].
Citations:
- 1: https://nextjs.org/docs/14/app/building-your-application/routing/middleware
- 2: https://nextjs.org/docs/app/api-reference/file-conventions/proxy
- 3: https://nextjs.org/docs/15/app/api-reference/file-conventions/middleware
- 4: https://github.com/vercel/next.js/blob/v16.2.1/docs/01-app/03-api-reference/03-file-conventions/proxy.mdx
- 5: Rewrites in middleware override next.config rewrites vercel/next.js#70365
- 6: Rewrites in middleware override next.config rewrites vercel/next.js#67245
🌐 Web query:
Next.js proxy matcher missing header regex value behavior
💡 Result:
In Next.js, when using the missing (or has) configuration within a matcher or rewrite, the value field is treated as a regular expression string [1][2]. If you provide a value for a missing condition, Next.js checks if the specified header, cookie, or query parameter is either entirely absent OR present but does not match the provided regex pattern [2]. Key behaviors to note: 1. Regex Matching: The value is evaluated as a regular expression [1]. For example, if you define a missing condition with a value, the condition evaluates to "true" (and thus the rewrite or matcher applies) if the header is missing OR if the header exists but its value fails to match the provided regex [2]. 2. Purpose: This functionality allows for sophisticated conditional logic, such as ensuring a route only matches when a specific header is absent or when a header does not contain a specific sensitive token or format [2]. 3. Implementation: You can use regex features like named capture groups if you need to extract values [2], though in a missing context, it primarily serves as a negative match filter. It is important to ensure your regex is correctly escaped for a string context if necessary, and keep in mind that these matchers are statically analyzed at build time [3]. If you encounter issues where a configuration appears valid but fails, ensure you are not relying on experimental or removed properties (such as an explicit regexp field in some older middleware documentation, which may not be supported) [4]. Always prefer the source, has, and missing structure defined in the official API reference [5][2].
Citations:
- 1: Allow matching against user-agent in rewrites that match headers vercel/next.js#48271
- 2: https://nextjs.org/docs/app/api-reference/config/next-config-js/rewrites
- 3: https://nextjslaunchpad.com/article/nextjs-middleware-proxy-complete-guide-edge-logic-security-migration
- 4: middleware matcher is not working as documented vercel/next.js#84307
- 5: https://nextjs.org/docs/app/api-reference/file-conventions/proxy
Exclude negotiated canonical Markdown requests from Proxy.
proxy runs before the beforeFiles Markdown Accept: text/markdown rewrite. A canonical request such as /notebooks/<id>/ therefore still performs token verification, CSP setup, and experiment enrollment before reaching the /md/<type>/... Markdown route.
Add a missing header condition so the same Accept: text/markdown negotiation is skipped by Proxy. Require an integration test for canonical Markdown requests with both authentication enabled and disabled.
Proposed matcher change
source:
"/((?!api|md|_next/static|_next/image|favicon.ico|sitemap.xml|robots.txt|csp-report|questions/embed|experiments/embed|opengraph-image-|twitter-image-|app-version|.*\\..*).*)",
+ missing: [
+ { type: "header", key: "accept", value: ".*text/markdown.*" },
+ ],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // | |
| // `md` is excluded because markdown responses are anonymous and public: | |
| // no token verification, CSP nonce or experiment enrollment is needed. | |
| // agent_markdown/respond.ts re-checks the access gates enforced above, | |
| // which is what keeps that safe. | |
| source: | |
| "/((?!api|_next/static|_next/image|favicon.ico|sitemap.xml|robots.txt|csp-report|questions/embed|experiments/embed|opengraph-image-|twitter-image-|app-version|.*\\..*).*)", | |
| "/((?!api|md|_next/static|_next/image|favicon.ico|sitemap.xml|robots.txt|csp-report|questions/embed|experiments/embed|opengraph-image-|twitter-image-|app-version|.*\\..*).*)", | |
| // | |
| // `md` is excluded because markdown responses are anonymous and public: | |
| // no token verification, CSP nonce or experiment enrollment is needed. | |
| // agent_markdown/respond.ts re-checks the access gates enforced above, | |
| // which is what keeps that safe. | |
| source: | |
| "/((?!api|md|_next/static|_next/image|favicon.ico|sitemap.xml|robots.txt|csp-report|questions/embed|experiments/embed|opengraph-image-|twitter-image-|app-version|.*\\..*).*)", | |
| missing: [ | |
| { type: "header", key: "accept", value: ".*text/markdown.*" }, | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@front_end/src/proxy.ts` around lines 213 - 219, Update the Proxy matcher
configuration around source to add a missing-header condition that excludes
requests negotiated with Accept: text/markdown, allowing canonical Markdown
requests to bypass Proxy before the beforeFiles rewrite. Preserve existing
exclusions, and add integration coverage for canonical Markdown requests with
authentication both enabled and disabled.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front_end/src/app/`(main)/notebooks/[id]/[[...slug]]/markdown.ts:
- Around line 12-16: Update the error handling around ServerPostsApi.getPost in
the markdown loader to return { status: "not_found" } only for the API’s
confirmed missing-post condition; re-throw transport, server, and other
unexpected failures so the response layer preserves its 500/error-logging path.
Add tests covering both confirmed not-found and upstream failure cases.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 183dd9e3-395d-4e6f-a5f3-ccb011b09ba7
📒 Files selected for processing (1)
front_end/src/app/(main)/notebooks/[id]/[[...slug]]/markdown.ts
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front_end/src/agent_markdown/frontmatter.ts`:
- Around line 27-31: Update the response construction in respond.ts around the
document.body/frontmatter assembly so the "\n\n" separator is added only when
frontmatter serialization is non-empty; otherwise return document.body directly.
Preserve frontmatter.stringify behavior in frontmatter.ts and ensure documents
without entries do not begin with blank lines.
In `@front_end/src/app/`(main)/notebooks/[id]/[[...slug]]/markdown.ts:
- Around line 16-22: Update builder.build to parse and validate id before
calling ServerPostsApi.getPost: accept only positive safe integers, and return {
status: "not_found" } for missing, non-numeric, fractional, unsafe, or
non-positive values. Pass the validated numeric ID to getPost, and add a test
confirming invalid IDs do not invoke ServerPostsApi.getPost.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fff90238-ff5e-4089-8b72-328e424a3317
📒 Files selected for processing (7)
front_end/src/agent_markdown/frontmatter.tsfront_end/src/agent_markdown/respond.tsfront_end/src/agent_markdown/routes.mjsfront_end/src/agent_markdown/types.tsfront_end/src/app/(main)/notebooks/[id]/[[...slug]]/markdown.tsfront_end/src/app/(md)/md/[type]/[[...args]]/route.tsfront_end/src/proxy.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- front_end/src/proxy.ts
- front_end/src/app/(md)/md/[type]/[[...args]]/route.ts
- front_end/src/agent_markdown/routes.mjs
- front_end/src/agent_markdown/types.ts
- front_end/src/agent_markdown/respond.ts
| if (present.length === 0) return ""; | ||
|
|
||
| const body = stringify(Object.fromEntries(present)).trimEnd(); | ||
|
|
||
| return ["---", body, "---"].join("\n"); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Avoid leading blank lines when frontmatter is empty.
When no entries are present, this function returns "". front_end/src/agent_markdown/respond.ts:49 always appends \n\n, so the response starts with two blank lines instead of document.body. Add the separator only when the serialized frontmatter is non-empty.
Proposed fix in the response layer
- return `${serializeFrontmatter(document.frontmatter)}\n\n${document.body}`;
+ const frontmatter = serializeFrontmatter(document.frontmatter);
+ return frontmatter ? `${frontmatter}\n\n${document.body}` : document.body;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@front_end/src/agent_markdown/frontmatter.ts` around lines 27 - 31, Update the
response construction in respond.ts around the document.body/frontmatter
assembly so the "\n\n" separator is added only when frontmatter serialization is
non-empty; otherwise return document.body directly. Preserve
frontmatter.stringify behavior in frontmatter.ts and ensure documents without
entries do not begin with blank lines.
| export const builder: MarkdownBuilder<Params> = { | ||
| async build({ id }): Promise<MarkdownBuildResult> { | ||
| if (!id) return { status: "not_found" }; | ||
|
|
||
| let post; | ||
| try { | ||
| post = await ServerPostsApi.getPost(Number(id), false); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject malformed IDs before the API call.
if (!id) checks only that the parameter is present. Values such as "abc" or "1.5" pass, and Number(id) produces NaN or a non-integer value for ServerPostsApi.getPost. Return { status: "not_found" } unless the parsed value matches the post-ID format, such as a positive safe integer. Add a test that verifies the API is not called for an invalid ID.
Proposed validation
async build({ id }): Promise<MarkdownBuildResult> {
- if (!id) return { status: "not_found" };
+ const postId = Number(id);
+ if (!Number.isSafeInteger(postId) || postId <= 0) {
+ return { status: "not_found" };
+ }
let post;
try {
- post = await ServerPostsApi.getPost(Number(id), false);
+ post = await ServerPostsApi.getPost(postId, false);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const builder: MarkdownBuilder<Params> = { | |
| async build({ id }): Promise<MarkdownBuildResult> { | |
| if (!id) return { status: "not_found" }; | |
| let post; | |
| try { | |
| post = await ServerPostsApi.getPost(Number(id), false); | |
| export const builder: MarkdownBuilder<Params> = { | |
| async build({ id }): Promise<MarkdownBuildResult> { | |
| const postId = Number(id); | |
| if (!Number.isSafeInteger(postId) || postId <= 0) { | |
| return { status: "not_found" }; | |
| } | |
| let post; | |
| try { | |
| post = await ServerPostsApi.getPost(postId, false); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@front_end/src/app/`(main)/notebooks/[id]/[[...slug]]/markdown.ts around lines
16 - 22, Update builder.build to parse and validate id before calling
ServerPostsApi.getPost: accept only positive safe integers, and return { status:
"not_found" } for missing, non-numeric, fractional, unsafe, or non-positive
values. Pass the validated numeric ID to getPost, and add a test confirming
invalid IDs do not invoke ServerPostsApi.getPost.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes