Make registered router patterns immutable

clone() shared the same RouterPathPattern objects through
#activeEntries(), and Template.match was a writable property, so a
caller could compile a pattern, register it, clone the router, then
mutate the pattern/template and change routing in both routers.  This
is the snapshot boundary FederationBuilder.build() relies on.

Rather than make the router own a private copy of every compiled route
expression, freeze the compiled artifact itself so a single shared
instance is safe to hand out:

- Router.compile() returns an Object.freeze()d RouterPathPattern.
- Template freezes itself and its token stream
  (tokens/vars/VarSpec deep-frozen); expand/match/toString are
  readonly and the instance is frozen, so the entry points cannot be
  reassigned.
- Token and VarSpec are readonly in the type surface; tokenize()
  replaces the trailing literal instead of mutating it in place.
- Router.compile().variables is an ImmutableSet that throws on
  mutation.

Add Router.clone() isolation and immutability tests, and a
FederationBuilder snapshot test asserting build() does not keep
observing later builder mutations and that two builds do not share
mutable route state.

#758 (comment)
#758 (comment)
#758 (comment)

Assisted-by: Claude Code:claude-opus-4-7

Author: 2chanhaeng

packages/fedify/src/federation/builder.test.ts

@@ -160,6 +160,31 @@ test("FederationBuilder", async (t) => {
     },
   );
 
+  await t.step("should snapshot router state on build", async () => {
+    const builder = createFederationBuilder<void>();
+    const kv = new MemoryKvStore();
+    const noteRouteName = `object:${Note.typeId.href}`;
+
+    builder.setActorDispatcher("/users/{identifier}", () => null);
+    const federation1 = await builder.build({ kv });
+    const impl1 = federation1 as FederationImpl<void>;
+
+    builder.setObjectDispatcher(Note, "/notes/{id}", () => null);
+    assertEquals(impl1.router.route("/notes/1"), null);
+
+    const federation2 = await builder.build({ kv });
+    const impl2 = federation2 as FederationImpl<void>;
+    assertEquals(impl2.router.route("/notes/1")?.name, noteRouteName);
+
+    impl1.router.add("/leaked/{id}", "leaked");
+    assertEquals(impl1.router.route("/leaked/1")?.name, "leaked");
+    assertEquals(impl2.router.route("/leaked/1"), null);
+
+    const federation3 = await builder.build({ kv });
+    const impl3 = federation3 as FederationImpl<void>;
+    assertEquals(impl3.router.route("/leaked/1"), null);
+  });
+
   await t.step("should build with default options", async () => {
     const builder = createFederationBuilder<void>();
     const kv = new MemoryKvStore();

packages/uri-template/src/router/router.test.ts

@@ -1,5 +1,6 @@
 import { test } from "@fedify/fixture";
 import { deepEqual, equal, throws } from "node:assert/strict";
+import type Template from "../template/template.ts";
 import {
   createRouterAddTest,
   createRouterBuildTest,
@@ -64,11 +65,22 @@ const createCountingPattern = (
 ): RouterPathPattern => {
   const pattern = Router.compile(path);
   const match = pattern.template.match;
-  pattern.template.match = (uri: string): ExpandContext | null => {
-    calls.set(path, (calls.get(path) ?? 0) + 1);
-    return match(uri);
+  const template = {
+    get tokens(): typeof pattern.template.tokens {
+      return pattern.template.tokens;
+    },
+    expand: pattern.template.expand,
+    match: (uri: string): ExpandContext | null => {
+      calls.set(path, (calls.get(path) ?? 0) + 1);
+      return match(uri);
+    },
+    toString: pattern.template.toString,
+  } as unknown as Template;
+  return {
+    path: pattern.path,
+    template,
+    variables: pattern.variables,
   };
-  return pattern;
 };
 
 test("Router indexes shared dynamic prefixes before template matching", () => {
@@ -219,6 +231,93 @@ test("Router accepts pre-parsed RouterPathPattern", async (t) => {
   });
 });
 
+test("Router.compile() returns immutable path patterns", async (t) => {
+  const pattern = Router.compile("/items/{id}");
+  const router = new Router([[pattern, "item"]]);
+
+  await t.step("blocks Template entry point reassignment", () => {
+    throws(() => {
+      (pattern.template as {
+        match: (uri: string) => ExpandContext | null;
+      }).match = () => null;
+    }, TypeError);
+    throws(() => {
+      (pattern.template as {
+        expand: (context: ExpandContext) => string;
+      }).expand = () => "/changed";
+    }, TypeError);
+  });
+
+  await t.step("blocks RouterPathPattern wrapper reassignment", () => {
+    const otherTemplate = Router.compile("/other/{id}").template;
+    throws(() => {
+      (pattern as { path: Path }).path = "/other/{id}";
+    }, TypeError);
+    throws(() => {
+      (pattern as { template: typeof otherTemplate }).template = otherTemplate;
+    }, TypeError);
+  });
+
+  await t.step("blocks variables mutation", () => {
+    throws(() => {
+      (pattern.variables as Set<string>).add("unexpected");
+    }, TypeError);
+    equal(pattern.variables.has("unexpected"), false);
+    equal(pattern.variables.size, 1);
+  });
+
+  await t.step("keeps registered routing behavior unchanged", () => {
+    deepEqual(router.route("/items/9"), {
+      name: "item",
+      template: "/items/{id}",
+      values: { id: "9" },
+    });
+    equal(router.build("item", { id: "9" }), "/items/9");
+  });
+});
+
+test("Router.clone() isolates route sets with shared pre-parsed patterns", () => {
+  const pattern = Router.compile("/items/{id}");
+  const original = new Router([[pattern, "item"]]);
+  const clone = original.clone();
+  const itemRoute = {
+    name: "item",
+    template: "/items/{id}",
+    values: { id: "9" },
+  };
+
+  deepEqual(original.route("/items/9"), itemRoute);
+  deepEqual(clone.route("/items/9"), itemRoute);
+
+  original.add("/posts/{postId}", "post");
+  equal(clone.route("/posts/3"), null);
+  deepEqual(original.route("/posts/3"), {
+    name: "post",
+    template: "/posts/{postId}",
+    values: { postId: "3" },
+  });
+
+  clone.add("/users/{userId}", "user");
+  equal(original.route("/users/5"), null);
+  deepEqual(clone.route("/users/5"), {
+    name: "user",
+    template: "/users/{userId}",
+    values: { userId: "5" },
+  });
+
+  original.add("/people/{id}", "item");
+  equal(original.route("/items/9"), null);
+  deepEqual(original.route("/people/9"), {
+    name: "item",
+    template: "/people/{id}",
+    values: { id: "9" },
+  });
+  equal(original.build("item", { id: "9" }), "/people/9");
+  deepEqual(clone.route("/items/9"), itemRoute);
+  equal(clone.route("/people/9"), null);
+  equal(clone.build("item", { id: "9" }), "/items/9");
+});
+
 test("Router trailing slash retry accepts empty root path", () => {
   const router = new Router([["", "root"]], {
     trailingSlashInsensitive: true,

packages/uri-template/src/router/router.ts

@@ -36,6 +36,9 @@ export interface RouterRouteResult {
 
 /**
  * Parsed path template ready to be registered in a {@link Router}.
+ *
+ * Instances returned by {@link Router.compile} are immutable and may be shared
+ * safely between routers and router clones.
  */
 export interface RouterPathPattern {
   /**
@@ -143,11 +146,11 @@ export default class Router {
     }
 
     const template = Template.parse(path);
-    return {
+    return Object.freeze({
       path,
       template,
       variables: collectVariables(template.tokens),
-    };
+    });
   }
 
   /**
@@ -259,9 +262,10 @@ export default class Router {
       ?.pattern.template.expand(values) ?? null) as Path | null;
 
   /**
-   * Creates a shallow clone of the router.  The clone shares the same route
-   * entries, but changes to the route set (adding, removing, or re-registering
-   * routes) do not affect the other router.
+   * Creates a shallow clone of the router.  The clone shares immutable
+   * registered path patterns with the original, but changes to the route set
+   * (adding, removing, or re-registering routes) do not affect the other
+   * router.
    * @returns A new router with the same routes and options as this one.
    */
   clone = (): Router =>
@@ -311,13 +315,32 @@ const isRoutesArgument = (
 const toggleTrailingSlash = (path: Path): Path =>
   path.endsWith("/") ? (path.replace(/\/+$/, "") as Path) : `${path}/`;
 
-const collectVariables = (tokens: readonly Token[]): Set<string> =>
-  new Set(
+const collectVariables = (tokens: readonly Token[]): ImmutableSet<string> =>
+  new ImmutableSet(
     tokens
       .filter(isExpression)
       .flatMap((token) => token.vars.map((varSpec) => varSpec.name)),
   );
 
+class ImmutableSet<T> extends Set<T> implements ReadonlySet<T> {
+  constructor(values?: Iterable<T>) {
+    super();
+    if (values != null) { for (const value of values) super.add(value); }
+  }
+
+  override add(_value: T): this {
+    throw new TypeError("ImmutableSet cannot be mutated.");
+  }
+
+  override delete(_value: T): boolean {
+    throw new TypeError("ImmutableSet cannot be mutated.");
+  }
+
+  override clear(): void {
+    throw new TypeError("ImmutableSet cannot be mutated.");
+  }
+}
+
 const getInitialLiteralPrefix = (tokens: readonly Token[]): string =>
   tokens[0] != null && isLiteral(tokens[0]) ? tokens[0].text : "";
 

packages/uri-template/src/template/expand.ts

@@ -28,7 +28,7 @@ export default function expand(
 }
 
 function expandExpressions(
-  vars: VarSpec[],
+  vars: readonly VarSpec[],
   operator: keyof typeof operatorSpecs,
   context: ExpandContext,
   options: TemplateOptions,

packages/uri-template/src/template/template.ts

@@ -3,6 +3,7 @@ import type {
   Reporter,
   TemplateOptions,
   Token,
+  VarSpec,
 } from "../types.ts";
 import expand from "./expand.ts";
 import match from "./match.ts";
@@ -12,10 +13,10 @@ import tokenize from "./token.ts";
  * Parsed RFC 6570 URI Template that can be expanded repeatedly.
  *
  * This class owns tokenization and delegates expression expansion to the
- * expansion module.
+ * expansion module.  Instances are immutable after construction.
  */
 export default class Template {
-  readonly #tokens: Token[];
+  readonly #tokens: readonly Token[];
   readonly #fullOptions: TemplateOptions;
 
   constructor(
@@ -36,7 +37,8 @@ export default class Template {
     readonly options: Partial<TemplateOptions> = {},
   ) {
     this.#fullOptions = fillOptions(options);
-    this.#tokens = tokenize(uriTemplate, this.#fullOptions);
+    this.#tokens = freezeTokens(tokenize(uriTemplate, this.#fullOptions));
+    Object.freeze(this);
   }
 
   /**
@@ -50,7 +52,7 @@ export default class Template {
   }
 
   /**
-   * Parsed token stream for diagnostics and router integration.
+   * Immutable parsed token stream for diagnostics and router integration.
    */
   get tokens(): readonly Token[] {
     return this.#tokens;
@@ -59,21 +61,40 @@ export default class Template {
   /**
    * Expands this template against a variable context.
    */
-  expand: (context: ExpandContext) => string = (
+  readonly expand: (context: ExpandContext) => string = (
     context: ExpandContext,
   ): string => expand(this.#tokens, context, this.#fullOptions);
 
   /**
    * Matches a URI against this template, returning the variable context if the
    * URI matches or `null` if it does not.
    */
-  match: (uri: string) => ExpandContext | null = (
+  readonly match: (uri: string) => ExpandContext | null = (
     uri: string,
   ): ExpandContext | null => match(this.#tokens, uri, this.#fullOptions);
 
-  toString = (): string => this.uriTemplate;
+  readonly toString = (): string => this.uriTemplate;
 }
 
+const freezeTokens = (tokens: Token[]): readonly Token[] =>
+  Object.freeze(tokens.map(freezeToken));
+
+const freezeToken = (token: Token): Token =>
+  token.kind === "literal"
+    ? Object.freeze({ kind: "literal", text: token.text })
+    : Object.freeze({
+      kind: "expression",
+      operator: token.operator,
+      vars: Object.freeze(token.vars.map(freezeVarSpec)),
+    });
+
+const freezeVarSpec = (varSpec: VarSpec): VarSpec =>
+  Object.freeze({
+    name: varSpec.name,
+    explode: varSpec.explode,
+    ...(varSpec.prefix == null ? {} : { prefix: varSpec.prefix }),
+  });
+
 const defaultReporter: Reporter = () => void 0;
 
 const fillOptions = (

packages/uri-template/src/template/token.ts

@@ -23,7 +23,10 @@ export default function tokenize(
   const appendLiteral = (text: string): void => {
     const previous = tokens.at(-1);
     if (previous?.kind === "literal") {
-      previous.text += text;
+      tokens[tokens.length - 1] = {
+        kind: "literal",
+        text: previous.text + text,
+      };
     } else {
       tokens.push({ kind: "literal", text });
     }

packages/uri-template/src/types.ts

@@ -48,11 +48,11 @@ export type ExpandContext = Record<string, ExpandValue>;
  */
 export interface VarSpec {
   /** Variable name to look up in the expansion context. */
-  name: string;
+  readonly name: string;
   /** Whether the varspec uses the Level 4 explode modifier (`*`). */
-  explode: boolean;
+  readonly explode: boolean;
   /** Prefix length from a Level 4 prefix modifier (`:N`), if present. */
-  prefix?: number;
+  readonly prefix?: number;
 }
 
 /**
@@ -62,8 +62,12 @@ export interface VarSpec {
  * context object.
  */
 export type Token =
-  | { kind: "literal"; text: string }
-  | { kind: "expression"; operator: Operator; vars: VarSpec[] };
+  | { readonly kind: "literal"; readonly text: string }
+  | {
+    readonly kind: "expression";
+    readonly operator: Operator;
+    readonly vars: readonly VarSpec[];
+  };
 
 /**
  * Options controlling URI Template parsing and expansion diagnostics.