feat(@mcp/node): mcpNodeHandler mount helper (handleHttp-backed)#2072
feat(@mcp/node): mcpNodeHandler mount helper (handleHttp-backed)#2072felixweinberger wants to merge 2 commits into
Conversation
felixweinberger
added
the
v2-stateless
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
|
@claude review |
1 similar comment
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
No correctness issues — the implementation is a trivial composition of toNodeHttpHandler(handleHttp(...)). Deferring to a maintainer because, contrary to the description's "Public API unchanged", this adds mcpNodeHandler as a new public export and is the first adapter to establish the mcp<Framework>Handler pattern (express/fastify/hono don't have it yet), so the shape and name are worth a human signing off before they propagate across the other packages.
Extended reasoning...
Overview
Adds mcpNodeHandler(mcp, options) to @modelcontextprotocol/node — a one-line wrapper that returns toNodeHttpHandler(handleHttp(mcp, options)) — plus JSDoc and two vitest cases (stateless initialize, SessionCompat lifecycle). index.ts does export *, so this is new public surface. No other framework adapter (express, fastify, hono) currently exports an equivalent, so this PR sets the precedent.
Security risks
None. No auth, parsing, or session logic is introduced; the wrapper just threads through to existing handleHttp / toNodeHttpHandler, which already handle req.auth / req.body.
Level of scrutiny
Implementation-wise this is mechanical and low-risk. The reason it shouldn't be auto-approved is policy, not correctness: the repo's review conventions put the burden of proof on additive API ("helpers users can write themselves belong in a cookbook, not the SDK"; "every new export is intentional"). A one-line composition of two already-public functions is squarely in that gray zone, and since this is the first adapter to ship the mcp*Handler shape, the naming/signature will be copied to the sibling packages — that's a maintainer call.
Other factors
- PR description says "Public API unchanged" / "Documentation update", which under-describes the change (covered by an inline nit re: missing changeset).
- Part of the v2-stateless stack with an explicit "Hold until R/S/F have soaked" note — another reason a human should gate the merge timing.
- Tests look adequate for the wrapper itself; the heavy lifting is already covered by
handleHttp's own suite.
| * ```ts | ||
| * import http from 'node:http'; | ||
| * import { McpServer, SessionCompat } from '@modelcontextprotocol/server'; | ||
| * import { mcpNodeHandler } from '@modelcontextprotocol/node'; | ||
| * | ||
| * const mcp = new McpServer({ name: 's', version: '1.0.0' }); | ||
| * http.createServer(mcpNodeHandler(mcp, { session: new SessionCompat() })).listen(3000); | ||
| * ``` |
There was a problem hiding this comment.
🟡 nit: Consider moving this snippet into streamableHttp.examples.ts (e.g. #region mcpNodeHandler_basicUsage) and referencing it via @example + ts source="./streamableHttp.examples.ts#mcpNodeHandler_basicUsage" so it's type-checked by pnpm sync:snippets. The companion file already exists for this source file and NodeStreamableHTTPServerTransport below uses the pattern; since mcpNodeHandler is the new canonical entry point it'd be nice for its example not to drift. (The adjacent toNodeHttpHandler has the same pre-existing inline pattern, so feel free to defer or fix both together.)
Extended reasoning...
What this is about
Per CLAUDE.md §"JSDoc @example Code Snippets" (lines 44-48), example snippets in JSDoc should pull from companion .examples.ts files via ts source="./file.examples.ts#regionName" fences, with regions named exportedName_variant. Those snippets are then validated by pnpm sync:snippets (which runs as part of lint:all), so they can't silently drift out of sync with the actual API.
The new mcpNodeHandler JSDoc embeds an inline, untyped ts fence (streamableHttp.ts:61-68) with no source= attribute and no corresponding #region in packages/middleware/node/src/streamableHttp.examples.ts. The snippet references McpServer, SessionCompat, and mcpNodeHandler — none of which are exercised by the type-checker for this block.
Step-by-step
packages/middleware/node/src/streamableHttp.examples.tsalready exists and currently contains three regions:NodeStreamableHTTPServerTransport_stateful,_stateless, and_express.- In the same source file,
NodeStreamableHTTPServerTransport's JSDoc uses the convention exactly:@exampleblocks with - The newly added
mcpNodeHandlerJSDoc instead uses a bare description-body fence with nosource=, sopnpm sync:snippetsignores it andtscnever sees it. - Consequence: if
SessionCompatis renamed, ormcpNodeHandler's signature changes, or thesessionoption key changes, this snippet will silently go stale while CI stays green.
Addressing the counter-argument
It's fair to note that CLAUDE.md scopes the rule to "@example tags" specifically, and this snippet is a plain fence in the description body rather than an @example-tagged block. There are also ~12 plain ts fences without source= elsewhere in the repo — including the immediately-preceding toNodeHttpHandler in this very file — so the new code is following adjacent precedent rather than introducing a novel deviation. That's why this is filed as a nit, not a blocker.
That said, three things tip it toward worth-mentioning here:
- The companion file is already in place for this exact source file, so the marginal cost is one
#regionblock plus changing the fence to@example+source=— no new infra needed. - Sibling exports in the same file already follow the convention, so adopting it here increases local consistency rather than decreasing it.
mcpNodeHandleris being positioned as the new canonical Node entry point (the PR description says the legacy transport class "become[s] removable later"). Canonical-entry-point examples are the ones most likely to be copy-pasted by users and most damaging when they drift, so they're the highest-value snippets to type-check.
Suggested fix
Add to streamableHttp.examples.ts:
import http from 'node:http';
import { McpServer, SessionCompat } from '@modelcontextprotocol/server';
import { mcpNodeHandler } from './streamableHttp.js';
function mcpNodeHandler_basicUsage() {
//#region mcpNodeHandler_basicUsage
const mcp = new McpServer({ name: 's', version: '1.0.0' });
http.createServer(mcpNodeHandler(mcp, { session: new SessionCompat() })).listen(3000);
//#endregion mcpNodeHandler_basicUsage
}Then in streamableHttp.ts, replace the bare fence with:
* @example
* ```ts source="./streamableHttp.examples.ts#mcpNodeHandler_basicUsage"
* ...
* ```
…and run pnpm sync:snippets. Optionally do the same for toNodeHttpHandler while you're here, but that one is pre-existing and not required for this PR.
| export function mcpNodeHandler( | ||
| mcp: Dispatchable, | ||
| options?: HandleHttpOptions | ||
| ): (req: IncomingMessage & { auth?: AuthInfo; body?: unknown }, res: ServerResponse, next?: (err?: unknown) => void) => Promise<void> { | ||
| return toNodeHttpHandler(handleHttp(mcp, options)); | ||
| } |
There was a problem hiding this comment.
🟡 This adds mcpNodeHandler as a new public export of @modelcontextprotocol/node (re-exported via export * in index.ts), but no .changeset/*.md covers it — and the PR description's "Public API unchanged" / "Documentation update" checkboxes under-describe the change. Consider adding a minor changeset so the release notes document the new entry point (or note here if you're intentionally batching one changeset for the whole R/S/F stack).
Extended reasoning...
What this is
mcpNodeHandler is a brand-new exported function in packages/middleware/node/src/streamableHttp.ts, and packages/middleware/node/src/index.ts does export * from './streamableHttp.js', so it becomes part of the public API surface of @modelcontextprotocol/node. The PR description says "Public API unchanged" and ticks only "Documentation update", but a new exported symbol is new public surface.
Verification
- Repo uses Changesets:
.changeset/config.jsonand.changeset/pre.jsonexist alongside ~50+.mdentries. grep -r 'mcpNodeHandler\|handleHttp\|Dispatchable' .changeset/→ no matches.- The existing
@modelcontextprotocol/nodechangesets (rich-hounds-report,quick-islands-occur,funky-baths-attack,shy-times-learn,add-hono-peer-dep,hono-peer-optional,brave-lions-glow, …) all describe unrelated fixes; none mention this helper. - The PR diff touches only
streamableHttp.tsand its test file — no.changeset/*.mdis added.
Step-by-step proof
packages/middleware/node/src/index.tsline 1:export * from './streamableHttp.js';streamableHttp.tsnow containsexport function mcpNodeHandler(...)(lines 72–77 in the diff).- Therefore
import { mcpNodeHandler } from '@modelcontextprotocol/node'is a new public import path. ls .changeset/+ grep show no entry referencingmcpNodeHandler(or any of the v2-stateless stack symbols it depends on:handleHttp,SessionCompat,Dispatchable).- The repo is in changesets pre mode (
pre.jsonpresent,2.0.0-alpha), but pre mode still requires changeset entries to drive the version bump and CHANGELOG generation — without one, the nextchangeset versionrun will produce release notes that omit this new entry point.
Why existing changesets don't cover it
Each existing @modelcontextprotocol/node changeset is scoped to a specific prior change (Hono peer-dep handling, transport bugfixes, etc.). None is a catch-all that would absorb a new export.
Impact
Process/release-notes only — no runtime effect. Consumers reading the published CHANGELOG won't see mcpNodeHandler listed, and the package may get a patch bump (or no bump) instead of the minor that semver expects for additive API.
Caveat / possible intentional deferral
The PR is part of a stacked series ("Depends on R4. Hold until R/S/F have soaked"), and none of the upstream stack PRs (handleHttp, SessionCompat, BackchannelCompat, Dispatchable) appear to have changesets either. So this may be a deliberate plan to land one umbrella changeset at the end of the stack. If so, a note in the PR description would help reviewers; if not, each PR conventionally carries its own changeset for the surface it adds.
Fix
npx changesetthen select @modelcontextprotocol/node → minor, with a summary like:
Add
mcpNodeHandler(mcp, options)— mounts anMcpServeras a Node.js(req, res)handler by composinghandleHttpwithtoNodeHttpHandler.
Also consider updating the PR description: this is a New feature (non-breaking), not a Documentation update, and the public API is changed (additively).
felixweinberger
force-pushed
the
fweinberger/r4-backchannel
branch
from
3eebc7f to
2358111
Compare
felixweinberger
force-pushed
the
fweinberger/m1-node
branch
from
08b83ee to
d24cdb1
Compare
|
|
@claude review |
| export function mcpNodeHandler( | ||
| mcp: Dispatchable, | ||
| options?: HandleHttpOptions | ||
| ): (req: IncomingMessage & { auth?: AuthInfo; body?: unknown }, res: ServerResponse, next?: (err?: unknown) => void) => Promise<void> { | ||
| return toNodeHttpHandler(handleHttp(mcp, options)); | ||
| } |
There was a problem hiding this comment.
🟡 nit: Per REVIEW.md's "Tests & docs" checklist, a new public entry point should get prose docs and an examples/ entry, but grep -r mcpNodeHandler docs/ examples/ (and likewise handleHttp) returns nothing — all ~18 server examples still mount via NodeStreamableHTTPServerTransport. Consider porting one runnable example (e.g. simpleStatelessStreamableHttp.ts) and adding a line to the server/migration guide; or, if you're intentionally batching docs at the end of the R/S/F stack, a note in the PR description would let reviewers stop flagging it. (Distinct from the .examples.ts snippet and changeset comments above — this is about docs/*.md prose and examples/*.ts runnable code.)
Extended reasoning...
What this is about
REVIEW.md's Tests & docs checklist says: "New feature: verify prose documentation is added (not just JSDoc), and assess whether examples/ needs a new or updated example." This PR adds mcpNodeHandler as a new public export of @modelcontextprotocol/node and positions it as the canonical Node entry point (PR description: the legacy transport class "become[s] removable later"), but neither the prose docs nor the runnable examples reference it.
Step-by-step verification
grep -rn mcpNodeHandler docs/→ 0 matches. No server guide, migration guide, or transport doc mentions the new helper.grep -rn mcpNodeHandler examples/→ 0 matches.examples/server/src/contains ~18 Streamable-HTTP examples (simpleStatelessStreamableHttp.ts,simpleStreamableHttp.ts,jsonResponseStreamableHttp.ts, etc.) and every one of them still constructs aNodeStreamableHTTPServerTransportand callsserver.connect(transport).- The same holds for the underlying primitives:
grep -rn 'handleHttp\|toNodeHttpHandler' docs/ examples/→ 0 matches. So this isn't a one-symbol oversight; the entirehandleHttp→toNodeHttpHandler→mcpNodeHandlerstack currently has no prose docs or runnable examples — the JSDoc fence at streamableHttp.ts:61-68 is the only user-facing reference. packages/middleware/node/src/index.tsdoesexport * from './streamableHttp.js', somcpNodeHandleris public API surface, not an internal helper.
Why existing artifacts don't cover this
This is distinct from the two doc-related comments already on the PR:
- Inline comment #3226634390 is about moving the JSDoc fence into
streamableHttp.examples.tssopnpm sync:snippetstype-checks it. That's about snippet validation, not user-facing docs. - Inline comment #3226634398 / changeset-bot is about the missing
.changeset/*.md. That's about release notes, not the docs site or examples directory.
Neither addresses docs/*.md prose or a runnable examples/server/src/*.ts file, which are what REVIEW.md's checklist item targets.
Impact
Documentation-only; no runtime effect. Users who land on the docs site or browse examples/ will continue to be steered toward NodeStreamableHTTPServerTransport + server.connect(), which is the pattern this stack is trying to retire. Since the JSDoc snippet is the only reference, adoption of the new path depends entirely on IDE hover / typedoc rather than the curated guides.
Suggested fix
- Examples: port one minimal example, e.g. rewrite
examples/server/src/simpleStatelessStreamableHttp.ts(or add a siblingexamples/server/src/mcpNodeHandler.ts) to:import http from 'node:http'; import { McpServer, SessionCompat } from '@modelcontextprotocol/server'; import { mcpNodeHandler } from '@modelcontextprotocol/node'; const mcp = new McpServer({ name: 'example', version: '1.0.0' }); // …registerTool… http.createServer(mcpNodeHandler(mcp, { session: new SessionCompat() })).listen(3000);
- Docs: add a short section to the server / Streamable-HTTP guide (and the v2 migration notes if they exist) showing
mcpNodeHandleras the preferred Node mount and pointing back tohandleHttpfor web-standard runtimes.
Caveat — possibly intentional
The PR says "Hold until R/S/F have soaked", and the underlying handleHttp/SessionCompat primitives are equally undocumented, so it's plausible the plan is to land one docs/examples sweep at the end of the stack rather than per-PR. If so, this is a one-line dismissal ("batching docs at end of stack") and worth noting in the PR description so the checklist item is visibly deferred rather than missed. Filed as a nit for that reason.
felixweinberger
force-pushed
the
fweinberger/r4-backchannel
branch
from
2358111 to
8493aac
Compare
felixweinberger
force-pushed
the
fweinberger/m1-node
branch
from
d24cdb1 to
7c501d5
Compare
|
@claude review |
| * import { mcpNodeHandler } from '@modelcontextprotocol/node'; | ||
| * | ||
| * const mcp = new McpServer({ name: 's', version: '1.0.0' }); | ||
| * http.createServer(mcpNodeHandler(mcp, { session: new SessionCompat() })).listen(3000); |
There was a problem hiding this comment.
🟡 nit: .listen(3000) with no host arg binds to ::/0.0.0.0, which is the configuration the sibling adapters (createMcpExpressApp/Hono/Fastify) explicitly default away from — they bind 127.0.0.1 and console.warn on 0.0.0.0 without allowedHosts. Since @modelcontextprotocol/node ships no host-validation middleware and HandleHttpOptions has no allowedHosts, consider .listen(3000, '127.0.0.1') and/or a pointer to the express adapter / DNS-rebinding middleware for users who need to bind publicly. (Distinct from the three doc nits above — this is about the example's security posture, not snippet validation, changesets, or prose docs.)
Extended reasoning...
What this is about
The new mcpNodeHandler JSDoc example (streamableHttp.ts:67) ends with:
http.createServer(mcpNodeHandler(mcp, { session: new SessionCompat() })).listen(3000);Node's server.listen(port) with no host argument binds to the unspecified address (:: if IPv6 is available, otherwise 0.0.0.0), exposing the MCP server on every network interface. There is no Host-header validation in this path: @modelcontextprotocol/node ships only index.ts + streamableHttp{,.examples}.ts with no hostHeaderValidation equivalent, and HandleHttpOptions (= ShttpHandlerOptions in shttpHandler.ts) has no allowedHosts/allowedOrigins field. So a user who copy-pastes this snippet verbatim gets an MCP server reachable from the LAN with no DNS-rebinding guard.
Why this is a repo concern, not a generic style preference
The three sibling adapter packages go out of their way to prevent exactly this configuration:
packages/middleware/express/src/express.ts:62-85—createMcpExpressAppdefaultshostto'127.0.0.1', auto-applieslocalhostHostValidation()for loopback binds, and emitsconsole.warn(...)when bound to'0.0.0.0'/'::'withoutallowedHosts.packages/middleware/hono/src/hono.ts:42, 71-87— same defaults and same warning.createMcpFastifyApp— same behaviour, covered by its tests.packages/server/src/server/middleware/hostHeaderValidation.tsexportsvalidateHostHeader/localhostAllowedHostnames, so DNS-rebinding protection is codified repo infrastructure.
In other words, the canonical-entry-point example for the Node adapter demonstrates a configuration that the repo's own Express/Hono/Fastify adapters would refuse to start without a warning.
Step-by-step
- User copies the snippet from the
mcpNodeHandlerJSDoc and runs it. http.createServer(handler).listen(3000)→ Node binds to::/0.0.0.0(per Node docs, "If host is omitted, the server will accept connections on the unspecified IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address (0.0.0.0) otherwise").mcpNodeHandler→toNodeHttpHandler(handleHttp(mcp, options)). NeithertoNodeHttpHandlernorhandleHttpinspects theHostheader;HandleHttpOptionshas noallowedHostsknob.- A page on
evil.exampleresolvesevil.exampleto the user's LAN IP (DNS rebinding) and issuesfetch('http://evil.example:3000/', { method:'POST', body: <tools/call> }). The request arrives withHost: evil.example:3000; nothing rejects it; the tool call executes. - By contrast, the same user starting from
createMcpExpressApp({ port: 3000 })would be bound to127.0.0.1withlocalhostHostValidation()applied, and would have to opt out explicitly to reach the same exposure.
Why nothing catches this
It's documentation, so the toolchain doesn't see it: the inline fence isn't type-checked (already noted in comment #3226634390) and there's no lint for .listen() host arguments. The adjacent toNodeHttpHandler example sidesteps the issue by mounting inside an existing app (app.all('/mcp', ...)), where users can add middleware in front.
Counterpoint considered
Repo precedent is not uniformly '127.0.0.1' — packages/middleware/node/README.md:54 and docs/server.md:517 both already show .listen(3000) with no host, and ~14 .listen() calls in examples/ omit a host too. So this snippet follows local precedent. The reason it's still worth a one-line tweak here is that (a) those existing examples mostly mount inside Express where users have a middleware slot, whereas this one is raw http.createServer with no middleware seam, and (b) the PR positions mcpNodeHandler as the new canonical Node mount (the legacy transport "become[s] removable later"), so this is the snippet most likely to be copy-pasted verbatim.
Severity & fix
nit — doc-only, no runtime defect in mcpNodeHandler itself. Suggested change:
http.createServer(mcpNodeHandler(mcp, { session: new SessionCompat() })).listen(3000, '127.0.0.1');Optionally add a sentence steering users who need to bind on all interfaces toward createMcpExpressApp (which has allowedHosts) or the hostHeaderValidation middleware. Distinct from the three existing PR comments (snippet validation #3226634390, changeset #3226634398, prose docs #3227466352) — none mention bind address or DNS rebinding.
felixweinberger
force-pushed
the
fweinberger/r4-backchannel
branch
from
8493aac to
716039a
Compare
felixweinberger
force-pushed
the
fweinberger/m1-node
branch
from
7c501d5 to
5bf5af6
Compare
|
@claude review |
| export function mcpNodeHandler( | ||
| mcp: Dispatchable, | ||
| options?: HandleHttpOptions | ||
| ): (req: IncomingMessage & { auth?: AuthInfo; body?: unknown }, res: ServerResponse, next?: (err?: unknown) => void) => Promise<void> { | ||
| return toNodeHttpHandler(handleHttp(mcp, options)); | ||
| } |
There was a problem hiding this comment.
🟡 nit: packages/middleware/node/README.md has an explicit ## Exports section (lines 15-18) that enumerates the package's public surface, and it still lists only NodeStreamableHTTPServerTransport + StreamableHTTPServerTransportOptions — mcpNodeHandler is now exported via export * in index.ts but isn't in the list, so the npmjs.com landing page will under-report the API and keep steering users to the legacy transport. Add - ``mcpNodeHandler``` (and optionally the pre-existing-missing toNodeHttpHandler) to that list. (Distinct from #3227466352, which is scoped to docs/andexamples/` — this is the package's own README.)
Extended reasoning...
What this is about
packages/middleware/node/README.md contains an explicit, enumerated ## Exports section:
## Exports
- `NodeStreamableHTTPServerTransport`
- `StreamableHTTPServerTransportOptions` (type alias for `WebStandardStreamableHTTPServerTransportOptions`)This PR adds export function mcpNodeHandler(...) to streamableHttp.ts, which flows through packages/middleware/node/src/index.ts (export * from './streamableHttp.js') and so becomes part of the package's public API. The README's enumerated list is not updated, so after merge it factually under-reports the export set.
This is what the package README renders as on npmjs.com — it's the first thing a user evaluating @modelcontextprotocol/node sees, and right now it advertises only the legacy transport class while this PR positions mcpNodeHandler as the new canonical entry point (PR description: the legacy transport "become[s] removable later").
Step-by-step proof
packages/middleware/node/src/index.tsline 1:export * from './streamableHttp.js';streamableHttp.ts(this PR, lines 72-77):export function mcpNodeHandler(mcp: Dispatchable, options?: HandleHttpOptions): ...- Therefore
import { mcpNodeHandler } from '@modelcontextprotocol/node'is new public surface. packages/middleware/node/README.mdlines 15-18:## Exportssection lists exactly two items, neither of which ismcpNodeHandler.- The PR diff touches only
streamableHttp.tsandstreamableHttp.test.ts—README.mdis not modified. - Result: the enumerated Exports list now contradicts the implementation. REVIEW.md "Tests & docs" checklist: "flag prose that now contradicts the implementation".
Why nothing catches this
The README is plain markdown with no source= reference, so neither pnpm sync:snippets nor tsc validates it. The two usage snippets in the README (Express and http.createServer) both demonstrate NodeStreamableHTTPServerTransport + server.connect(), so the omission is internally consistent within the file — there's just nothing pointing at the new path.
Distinct from existing PR comments
- #3226634390 — about moving the JSDoc fence into
streamableHttp.examples.tsfor type-checking. Different artifact (JSDoc snippet, not README). - #3226634398 / changeset-bot — about the missing
.changeset/*.md. Different artifact (release notes, not README). - #3227466352 — explicitly scoped to
docs/*.mdandexamples/*.ts(its grep verification isgrep -r mcpNodeHandler docs/ examples/and its suggested fix is to port a runnable example + add a server-guide section). It never mentions the package README, which is a separate surface — it renders on npmjs.com, not the docs site. - #3228208910 — about the
.listen(3000)bind address in the JSDoc example. Unrelated.
Pre-existing note
toNodeHttpHandler is also a public export missing from this list, but that omission predates this PR (it was already exported and already absent from the README on main). Mentioned only because fixing both in one line is cheap; the new omission introduced by this PR is mcpNodeHandler.
Impact
Documentation-only; no runtime effect. Users landing on npmjs.com/package/@modelcontextprotocol/node will see an Exports list and two usage snippets that all point at the legacy NodeStreamableHTTPServerTransport + connect() pattern, with no hint that mcpNodeHandler exists. Adoption of the new path then depends entirely on IDE autocomplete / typedoc.
Fix
Add to packages/middleware/node/README.md under ## Exports:
- `mcpNodeHandler`
- `toNodeHttpHandler`Optionally add a third ## Usage snippet showing the one-liner mount (matching the JSDoc example), since both existing README snippets demonstrate the pattern this stack is retiring.
1 participant
@modelcontextprotocol/nodeadapter switches its internals tohandleHttp(server)+toNodeHttpHandler. Public API unchanged.Motivation and Context
The framework adapters are the highest-traffic SHTTP path; routing them through
handleHttpexercises the new entry under existing usage and lets the legacy transport class become removable later.How Has This Been Tested?
Existing
@modelcontextprotocol/nodetests pass unchanged.Breaking Changes
None.
Types of changes
Checklist
Additional context
Depends on R4. Hold until R/S/F have soaked. Review guide: https://gist.github.com/felixweinberger/5a48e0f14d5aced39ed6a91b61940711.