From 81f6d094a5be4540682afd209c5b3ab860939a8c Mon Sep 17 00:00:00 2001 From: Ryan Beckett Date: Fri, 16 Oct 2026 11:02:31 -0700 Subject: [PATCH 1/2] feat(router)!: add a versioned JSON Schema for routing tables at version 1 Routing tables are the router's second input contract, but their format was defined only by routing_table.py and contracts.py. Add a hand-written draft 2020-12 schema at containers/router/schemas/routing-table.v1.schema.json, frozen once published, so the table format is an explicit, versioned contract. Start the published numbering at 1: the loader, bundled tables, synthetic fixtures and openapi.yaml move from schema_version 5 to 1. The loader now also rejects a boolean schema_version, which Literal[1] would otherwise accept because True == 1. tests/test_table_schema.py checks that: - the schema is valid and every bundled, synthetic and contract-corpus table conforms; - its version, profiles, label-key and model-identity patterns and closed objects match the runtime models and enums; - the schema and RoutingTable.from_bytes reject the same single-field defects, and the cross-reference rules that only the loader enforces stay documented in the schema; - published schema files keep their pinned SHA-256. Include schemas/ in the source distribution and document the schema and its versioning policy in the router README. BREAKING CHANGE: the router accepts only routing tables with schema_version 1. Tables with schema_version 5 are rejected and must be regenerated with the new version. Closes #10124 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- containers/router/README.md | 24 +- containers/router/openapi.yaml | 4 +- containers/router/pyproject.toml | 1 + containers/router/routing/cost-balanced.json | 2 +- containers/router/routing/cost-economy.json | 2 +- containers/router/routing/cost-robust.json | 2 +- .../router/routing/cost-speed-balanced.json | 2 +- .../router/routing/cost-speed-economy.json | 2 +- .../router/routing/cost-speed-robust.json | 2 +- .../schemas/routing-table.v1.schema.json | 75 ++++ .../router/src/gh_aw_router/routing_table.py | 12 +- containers/router/tests/contract_corpus.py | 2 +- containers/router/tests/test_package.py | 1 + containers/router/tests/test_routing.py | 6 +- containers/router/tests/test_table_schema.py | 367 ++++++++++++++++++ 15 files changed, 488 insertions(+), 16 deletions(-) create mode 100644 containers/router/schemas/routing-table.v1.schema.json create mode 100644 containers/router/tests/test_table_schema.py diff --git a/containers/router/README.md b/containers/router/README.md index 410566f3b..584114ca4 100644 --- a/containers/router/README.md +++ b/containers/router/README.md @@ -213,9 +213,22 @@ order, so they differ only in their routing order. Select a directory or a singl `--routing-tables` or `GH_AW_ROUTER_ROUTING_TABLES`, and mount or publish generated tables read-only. -Only schema 5 is supported. A schema-5 table declares one fixed `cost` or `cost-speed` profile -and carries no default-effort aliases. This format marker remains independent because tables -can be supplied through a file or mount without replacing the router image. +Only `schema_version` 1 is supported, and the loader rejects any other value. A table declares +one fixed `cost` or `cost-speed` profile and carries no default-effort aliases. This format +marker remains independent because tables can be supplied through a file or mount without +replacing the router image. + +[schemas/routing-table.v1.schema.json](schemas/routing-table.v1.schema.json) specifies the +table file format as a JSON Schema. The loader also enforces rules that relate parts of a table +to each other, which the schema can't express: every label cell appears in exactly one ranking +group, every ranking and the classifier order are permutations of the first group's choices, +no two groups have the same ranking, and a model listed with explicit efforts isn't also listed +without one. The schema requires `schema_version`, which the loader defaults to 1 when it's +missing. + +A published schema file never changes. A format change adds a new schema file, such as +`routing-table.v2.schema.json`, and bumps `schema_version`. Every table must validate against +the schema for its `schema_version`. ## Container @@ -350,6 +363,7 @@ configured to run both groups separately. - `src/gh_aw_router/` contains request contracts, classification, table lookup, and CLI and HTTP adapters. - `tests/` mirrors the runtime modules, with shared synthetic fixtures in `conftest.py`. - `routing/` contains generated routing tables from training. +- `schemas/` contains the versioned JSON Schema for routing table files. - `examples/` contains executable planning requests. - The repository root's `.github/workflows/test-router.yml` and `.github/dependabot.yml` contain CI and dependency-update configuration. @@ -369,6 +383,10 @@ modes from published profiles and includes the classifier-output schema used by Update it alongside contract changes and extend the schema/runtime parity tests in `tests/test_openapi.py`. Do not add a second generated public contract. +The hand-written schema in `schemas/` is the authoritative routing-table contract. Change the +table format only by adding a new schema version, and update the parity tests in +`tests/test_table_schema.py` alongside it. + Use the shared synthetic table and request fixtures in `tests/conftest.py` for generic routing, service, and transport tests. Reserve the bundled-table `service` fixture for release-data and example integration checks so ranking refreshes do not affect unrelated behavior tests. diff --git a/containers/router/openapi.yaml b/containers/router/openapi.yaml index 31e46bf13..82dca4ea1 100644 --- a/containers/router/openapi.yaml +++ b/containers/router/openapi.yaml @@ -15,8 +15,8 @@ info: economy, balanced, robust, or auto. Auto uses the supplied classification's mode, falling back to balanced when classification is absent or its mode is unknown. /capabilities lists the fixed profiles the loaded tables serve. - Schema-5 tables each carry one fixed profile. - Older schemas are rejected. Unserved profiles return invalid_request without fallback. + Routing tables (schema_version 1) each carry one fixed profile. + Other schema versions are rejected. Unserved profiles return invalid_request without fallback. Planning requests contain only decision inputs. Execution identity and outcome correlation remain with the caller; the router stores no task state. Every application error uses the Error envelope, including unknown paths, diff --git a/containers/router/pyproject.toml b/containers/router/pyproject.toml index c24211f45..634444434 100644 --- a/containers/router/pyproject.toml +++ b/containers/router/pyproject.toml @@ -43,6 +43,7 @@ source-include = [ "openapi.yaml", "requirements.lock", "routing/**", + "schemas/**", "tests/**", "uv.lock", ] diff --git a/containers/router/routing/cost-balanced.json b/containers/router/routing/cost-balanced.json index f81e14392..38b0d0a00 100644 --- a/containers/router/routing/cost-balanced.json +++ b/containers/router/routing/cost-balanced.json @@ -1,5 +1,5 @@ { - "schema_version": 5, + "schema_version": 1, "profile": { "goal": "cost", "mode": "balanced" diff --git a/containers/router/routing/cost-economy.json b/containers/router/routing/cost-economy.json index 10c5b023c..32720cf5e 100644 --- a/containers/router/routing/cost-economy.json +++ b/containers/router/routing/cost-economy.json @@ -1,5 +1,5 @@ { - "schema_version": 5, + "schema_version": 1, "profile": { "goal": "cost", "mode": "economy" diff --git a/containers/router/routing/cost-robust.json b/containers/router/routing/cost-robust.json index d4db06e82..fe78b1144 100644 --- a/containers/router/routing/cost-robust.json +++ b/containers/router/routing/cost-robust.json @@ -1,5 +1,5 @@ { - "schema_version": 5, + "schema_version": 1, "profile": { "goal": "cost", "mode": "robust" diff --git a/containers/router/routing/cost-speed-balanced.json b/containers/router/routing/cost-speed-balanced.json index ad32aa006..2ed3f8583 100644 --- a/containers/router/routing/cost-speed-balanced.json +++ b/containers/router/routing/cost-speed-balanced.json @@ -1,5 +1,5 @@ { - "schema_version": 5, + "schema_version": 1, "profile": { "goal": "cost-speed", "mode": "balanced" diff --git a/containers/router/routing/cost-speed-economy.json b/containers/router/routing/cost-speed-economy.json index 0c8fe81b9..c8accb4df 100644 --- a/containers/router/routing/cost-speed-economy.json +++ b/containers/router/routing/cost-speed-economy.json @@ -1,5 +1,5 @@ { - "schema_version": 5, + "schema_version": 1, "profile": { "goal": "cost-speed", "mode": "economy" diff --git a/containers/router/routing/cost-speed-robust.json b/containers/router/routing/cost-speed-robust.json index 7a6928600..e65afe231 100644 --- a/containers/router/routing/cost-speed-robust.json +++ b/containers/router/routing/cost-speed-robust.json @@ -1,5 +1,5 @@ { - "schema_version": 5, + "schema_version": 1, "profile": { "goal": "cost-speed", "mode": "robust" diff --git a/containers/router/schemas/routing-table.v1.schema.json b/containers/router/schemas/routing-table.v1.schema.json new file mode 100644 index 000000000..5607b8b00 --- /dev/null +++ b/containers/router/schemas/routing-table.v1.schema.json @@ -0,0 +1,75 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://raw.githubusercontent.com/github/gh-aw-firewall/main/containers/router/schemas/routing-table.v1.schema.json", + "title": "gh-aw-router routing table, schema version 1", + "description": "One routing table file for one fixed profile. A document that validates against this schema can still be rejected by the router, which also enforces these cross-reference rules: (1) every label cell, 210 in total, appears in exactly one ranking group's applies_to; (2) the first ranking group's choices define the model catalogue, and every other group's choices and classification_choices are permutations of it; (3) no two ranking groups have the same choices, so cells with the same ranking share one group; (4) a model listed with an explicit effort is not also listed without one. Tables loaded together must also share the same repository, catalogue and classification_choices, and serve each profile at most once. This file is frozen once published: a format change adds a new schema file and a new schema_version.", + "$comment": "Every pattern ends with $(?!\\n) so that validators using Python regular expressions, where $ also matches before a final newline, behave like ECMA-262.", + "type": "object", + "additionalProperties": false, + "required": ["schema_version", "profile", "repository", "classification_choices", "rankings"], + "properties": { + "schema_version": { + "description": "Table format version. Every table includes it.", + "const": 1 + }, + "profile": { + "description": "The fixed goal and mode this table serves. The request mode auto is not a table profile.", + "type": "object", + "additionalProperties": false, + "required": ["goal", "mode"], + "properties": { + "goal": {"enum": ["cost", "cost-speed"]}, + "mode": {"enum": ["economy", "balanced", "robust"]} + } + }, + "repository": { + "description": "The repository the rankings were generated for, or global. No leading or trailing whitespace.", + "type": "string", + "minLength": 1, + "pattern": "^(?!\\s)[\\s\\S]*(? tuple[RoutingObjective, ...]: """Expose the sole profile through the shared compiler/serving interface.""" return (self.profile,) + @field_validator("schema_version", mode="before") + @classmethod + def _reject_boolean_version(cls, value: object) -> object: + """Reject JSON true, which Literal[1] would otherwise accept because True == 1.""" + if isinstance(value, bool): + raise ValueError("schema_version must be an integer") + return value + @dataclass(frozen=True) class RoutingTable: diff --git a/containers/router/tests/contract_corpus.py b/containers/router/tests/contract_corpus.py index 069b2f953..eb788cd80 100644 --- a/containers/router/tests/contract_corpus.py +++ b/containers/router/tests/contract_corpus.py @@ -36,7 +36,7 @@ def synthetic_table_document( choices: tuple[str, str] = ("provider/fast", "provider/reasoning:medium"), ) -> dict[str, Any]: return { - "schema_version": 5, + "schema_version": 1, "profile": {"goal": "cost", "mode": "balanced"}, "repository": "global", "classification_choices": list(choices), diff --git a/containers/router/tests/test_package.py b/containers/router/tests/test_package.py index 5dfe29492..54610252c 100644 --- a/containers/router/tests/test_package.py +++ b/containers/router/tests/test_package.py @@ -322,6 +322,7 @@ def release_artifacts(tmp_path_factory: pytest.TempPathFactory) -> tuple[Path, P "tests/fixtures/routing-contract/route.json", "routing/cost-balanced.json", "routing/cost-speed-robust.json", + "schemas/routing-table.v1.schema.json", ): assert (extracted / filename).is_file(), filename assert not (extracted / "docs").exists() diff --git a/containers/router/tests/test_routing.py b/containers/router/tests/test_routing.py index 98412f8ab..640a7f0da 100644 --- a/containers/router/tests/test_routing.py +++ b/containers/router/tests/test_routing.py @@ -237,8 +237,10 @@ def test_table_rejects_auto_profile(table_document: dict[str, Any]) -> None: _routing_table(table_document) -@pytest.mark.parametrize("schema_version", [3, 4]) -def test_table_rejects_legacy_schemas(table_document: dict[str, Any], schema_version: int) -> None: +@pytest.mark.parametrize("schema_version", [0, 2, 3, 4, 5]) +def test_table_rejects_other_schema_versions( + table_document: dict[str, Any], schema_version: int +) -> None: document = table_document document["schema_version"] = schema_version with pytest.raises(ValueError, match="schema_version"): diff --git a/containers/router/tests/test_table_schema.py b/containers/router/tests/test_table_schema.py new file mode 100644 index 000000000..cb56ffa69 --- /dev/null +++ b/containers/router/tests/test_table_schema.py @@ -0,0 +1,367 @@ +"""Keep the published routing-table schema, the table loader and the bundled tables in step.""" + +from __future__ import annotations + +import functools +import hashlib +import json +import re +from collections.abc import Callable +from pathlib import Path +from typing import Any, get_args + +import pytest +from contract_corpus import contract_tables, synthetic_table_document +from jsonschema import Draft202012Validator +from jsonschema.protocols import Validator +from pydantic import BaseModel + +from gh_aw_router.contracts import ( + ReasoningEffort, + RoutingObjective, + TaskComplexity, + TaskScope, + TaskType, +) +from gh_aw_router.routing_table import ( + PROFILES, + RankingGroup, + RoutingTable, + TableDocument, + all_labels, + label_key, + profile_filename, +) + +PROJECT_ROOT = Path(__file__).resolve().parents[1] +SCHEMA_DIRECTORY = PROJECT_ROOT / "schemas" +CURRENT_VERSION = get_args(TableDocument.model_fields["schema_version"].annotation)[0] +# A published schema never changes. A format change adds a new file and pins its hash here. +PUBLISHED_SCHEMAS = { + 1: "e522d397a8b595dba741189dbafbe00ebc369e0f196bf02280a5d85f008e5bcd", +} +PATTERN_END = r"$(?!\n)" + +type Mutation = Callable[[dict[str, Any]], None] + + +def _schema_name(version: int) -> str: + return f"routing-table.v{version}.schema.json" + + +@functools.cache +def _schema() -> dict[str, Any]: + return json.loads((SCHEMA_DIRECTORY / _schema_name(CURRENT_VERSION)).read_bytes()) + + +def _validator() -> Validator: + return Draft202012Validator(_schema()) + + +def _definition_validator(name: str) -> Validator: + return Draft202012Validator({"$ref": f"#/$defs/{name}", "$defs": _schema()["$defs"]}) + + +def _node(document: dict[str, Any], path: tuple[str | int, ...]) -> Any: # noqa: ANN401 + node: Any = document + for key in path: + node = node[key] + return node + + +def _loader_accepts(document: dict[str, Any]) -> bool: + try: + RoutingTable.from_bytes(json.dumps(document).encode()) + except ValueError: + return False + return True + + +def _alternation(enum: type[Any]) -> str: + return "(" + "|".join(item.value for item in enum) + ")" + + +@functools.cache +def _router_tables() -> dict[str, bytes]: + tables = { + f"routing/{path.name}": path.read_bytes() + for path in sorted((PROJECT_ROOT / "routing").glob("*.json")) + } + tables["synthetic"] = json.dumps(synthetic_table_document()).encode() + tables.update({f"contract/{name}": data for name, data in contract_tables().items()}) + return tables + + +def test_schema_is_a_valid_draft_2020_12_schema() -> None: + Draft202012Validator.check_schema(_schema()) + assert _schema()["$schema"] == "https://json-schema.org/draft/2020-12/schema" + + +def test_router_tables_include_every_bundled_profile() -> None: + bundled = {name for name in _router_tables() if name.startswith("routing/")} + assert bundled == {f"routing/{profile_filename(profile)}" for profile in PROFILES} + + +@pytest.mark.parametrize("name", sorted(_router_tables())) +def test_every_router_table_conforms_to_the_schema(name: str) -> None: + data = _router_tables()[name] + _validator().validate(json.loads(data)) + RoutingTable.from_bytes(data) + + +def test_schema_constants_match_the_runtime() -> None: + properties = _schema()["properties"] + assert properties["schema_version"]["const"] == CURRENT_VERSION + profile = properties["profile"]["properties"] + goals, modes = profile["goal"]["enum"], profile["mode"]["enum"] + assert len(goals) == len(set(goals)) + assert len(modes) == len(set(modes)) + assert {(goal, mode) for goal in goals for mode in modes} == { + (objective.goal.value, objective.mode.value) for objective in PROFILES + } + + +def test_schema_patterns_are_built_from_the_runtime_enums() -> None: + definitions = _schema()["$defs"] + labels = "/".join(_alternation(enum) for enum in (TaskType, TaskScope, TaskComplexity)) + assert definitions["labelKey"]["pattern"] == f"^{labels}{PATTERN_END}" + assert definitions["modelIdentity"]["pattern"] == ( + rf"^[^/:\s][^/:]*/[^/:]*[^/:\s](:{_alternation(ReasoningEffort)})?{PATTERN_END}" + ) + + +@pytest.mark.parametrize( + ("model", "pointer"), + [ + (TableDocument, ()), + (RoutingObjective, ("properties", "profile")), + (RankingGroup, ("$defs", "rankingGroup")), + ], + ids=["TableDocument", "profile", "rankingGroup"], +) +def test_schema_objects_match_the_runtime_fields( + model: type[BaseModel], pointer: tuple[str, ...] +) -> None: + node = _node(_schema(), pointer) + runtime_required = {name for name, field in model.model_fields.items() if field.is_required()} + assert model.model_config.get("extra") == "forbid" + assert node["additionalProperties"] is False + assert set(node["properties"]) == set(model.model_fields) + # Every table includes the version, even though the loader defaults it. + expected_required = runtime_required | ({"schema_version"} if model is TableDocument else set()) + assert set(node["required"]) == expected_required + + +def test_schema_nests_the_same_objects_as_the_runtime() -> None: + fields = TableDocument.model_fields + assert fields["profile"].annotation is RoutingObjective + assert get_args(fields["rankings"].annotation)[0] is RankingGroup + + +def test_label_key_pattern_accepts_every_runtime_cell() -> None: + keys = [label_key(labels) for labels in all_labels()] + validator = _definition_validator("labelKey") + assert len(set(keys)) == len(TaskType) * len(TaskScope) * len(TaskComplexity) + assert [key for key in keys if not validator.is_valid(key)] == [] + + +@pytest.mark.parametrize( + "key", + [ + "fix/easy/local", + "local/fix/easy", + "invalid/local/easy", + "fix/local/invalid", + "Fix/local/easy", + "fix/local", + "fix/local/easy/extra", + " fix/local/easy", + "fix/local/easy\n", + ], +) +def test_label_key_pattern_rejects_other_keys(key: str) -> None: + assert not _definition_validator("labelKey").is_valid(key) + + +@pytest.mark.parametrize( + ("identity", "valid"), + [ + *((f"provider/model:{effort.value}", True) for effort in ReasoningEffort), + ("provider/model", True), + ("provider/my model", True), + ("provider /model", True), + ("p/m", True), + ("provider", False), + ("provider/", False), + ("/model", False), + ("a/b/c", False), + (" provider/model", False), + ("provider/model ", False), + ("\u00a0provider/model", False), + ("provider/model :medium", False), + ("provider/model:", False), + ("provider/model:turbo", False), + ("provider/model:MEDIUM", False), + ("provider/model:medium:high", False), + ("provider/model\n", False), + ("provider/model:medium\n", False), + ], +) +def test_schema_and_loader_agree_on_model_identities(identity: str, valid: bool) -> None: + document = synthetic_table_document() + document["classification_choices"] = [identity] + document["rankings"][0]["choices"] = [identity] + assert _definition_validator("modelIdentity").is_valid(identity) is valid + assert _validator().is_valid(document) is valid + assert _loader_accepts(document) is valid + + +def _set(*path: str | int, value: object) -> Mutation: + def mutate(document: dict[str, Any]) -> None: + _node(document, path[:-1])[path[-1]] = value + + return mutate + + +def _append(*path: str | int, value: object) -> Mutation: + def mutate(document: dict[str, Any]) -> None: + _node(document, path).append(value) + + return mutate + + +def _delete(*path: str | int) -> Mutation: + def mutate(document: dict[str, Any]) -> None: + del _node(document, path[:-1])[path[-1]] + + return mutate + + +def _duplicate_first_cell(document: dict[str, Any]) -> None: + group = document["rankings"][0] + group["applies_to"].append(group["applies_to"][0]) + + +def _new_group(choices: Callable[[list[str]], list[str]]) -> Mutation: + """Move the last label cell of the first group into a new group with derived choices.""" + + def mutate(document: dict[str, Any]) -> None: + group = document["rankings"][0] + cell = group["applies_to"].pop() + document["rankings"].append({"applies_to": [cell], "choices": choices(group["choices"])}) + + return mutate + + +def _assign_first_cell_twice(document: dict[str, Any]) -> None: + group = document["rankings"][0] + document["rankings"].append( + {"applies_to": [group["applies_to"][0]], "choices": group["choices"][::-1]} + ) + + +REJECTED_BY_BOTH: dict[str, Mutation] = { + "unknown top-level field": _set("default_efforts", value={"provider/reasoning": "medium"}), + "unknown profile field": _set("profile", "overrides", value=None), + "unknown ranking group field": _set("rankings", 0, "weight", value=1), + "missing profile": _delete("profile"), + "missing profile goal": _delete("profile", "goal"), + "missing repository": _delete("repository"), + "missing classification_choices": _delete("classification_choices"), + "missing rankings": _delete("rankings"), + "missing applies_to": _delete("rankings", 0, "applies_to"), + "missing choices": _delete("rankings", 0, "choices"), + "schema_version 0": _set("schema_version", value=0), + "schema_version 2": _set("schema_version", value=2), + "schema_version 5": _set("schema_version", value=5), + "schema_version as a string": _set("schema_version", value="1"), + "schema_version as a boolean": _set("schema_version", value=True), + "auto profile": _set("profile", "mode", value="auto"), + "unknown mode": _set("profile", "mode", value="custom"), + "unknown goal": _set("profile", "goal", value="speed"), + "empty repository": _set("repository", value=""), + "repository with leading whitespace": _set("repository", value=" global"), + "repository with trailing whitespace": _set("repository", value="global "), + "repository with a trailing newline": _set("repository", value="global\n"), + "repository not a string": _set("repository", value=1), + "unknown label key": _append("rankings", 0, "applies_to", value="invalid/local/easy"), + "label key in another order": _append("rankings", 0, "applies_to", value="local/fix/easy"), + "label key with a trailing newline": _append( + "rankings", 0, "applies_to", value="fix/local/easy\n" + ), + "cell repeated in one group": _duplicate_first_cell, + "choice repeated in one group": _append("rankings", 0, "choices", value="provider/fast"), + "unknown effort": _set("rankings", 0, "choices", 1, value="provider/reasoning:turbo"), + "non-canonical identity": _set("rankings", 0, "choices", 0, value=" provider/fast"), + "empty rankings": _set("rankings", value=[]), + "empty applies_to": _set("rankings", 0, "applies_to", value=[]), + "empty choices": _set("rankings", 0, "choices", value=[]), + "empty classification_choices": _set("classification_choices", value=[]), + "rankings not an array": _set("rankings", value={}), + "applies_to not an array": _set("rankings", 0, "applies_to", value="fix/local/easy"), +} + + +@pytest.mark.parametrize("change", REJECTED_BY_BOTH) +def test_schema_and_loader_reject_the_same_defects(change: str) -> None: + document = synthetic_table_document() + REJECTED_BY_BOTH[change](document) + assert not _validator().is_valid(document) + assert not _loader_accepts(document) + + +# Rules that relate parts of a table to each other. The schema's description lists them. +LOADER_ONLY: dict[str, tuple[Mutation, str]] = { + "missing label cell": ( + _delete("rankings", 0, "applies_to", -1), + "every label combination", + ), + "cell assigned to two groups": (_assign_first_cell_twice, "more than once"), + "ranking not a permutation": ( + _new_group(lambda choices: choices[:1]), + "full ordered pair list", + ), + "classification order not a permutation": ( + _set("classification_choices", value=["provider/fast"]), + "full ordered pair list", + ), + "identical groups": (_new_group(list), "identical rankings must share one group"), + "bare alias next to explicit efforts": ( + _append("rankings", 0, "choices", value="provider/reasoning"), + "bare default aliases", + ), +} + + +@pytest.mark.parametrize("change", LOADER_ONLY) +def test_cross_reference_rules_are_enforced_only_by_the_loader(change: str) -> None: + mutate, message = LOADER_ONLY[change] + document = synthetic_table_document() + mutate(document) + _validator().validate(document) + with pytest.raises(ValueError, match=message): + RoutingTable.from_bytes(json.dumps(document).encode()) + + +def test_schema_requires_the_version_that_the_loader_defaults() -> None: + document = synthetic_table_document() + del document["schema_version"] + assert not _validator().is_valid(document) + assert _loader_accepts(document) + + +def test_published_schemas_are_frozen() -> None: + expected = {_schema_name(version) for version in PUBLISHED_SCHEMAS} + assert {path.name for path in SCHEMA_DIRECTORY.iterdir()} == expected + assert CURRENT_VERSION in PUBLISHED_SCHEMAS + for version, digest in PUBLISHED_SCHEMAS.items(): + name = _schema_name(version) + data = (SCHEMA_DIRECTORY / name).read_bytes() + assert hashlib.sha256(data).hexdigest() == digest, ( + f"{name} is published and must not change. Add {_schema_name(version + 1)} and " + "bump TableDocument.schema_version for a format change." + ) + schema = json.loads(data) + assert schema["$id"].endswith(f"/{name}") + assert re.search(rf"\bschema version {version}$", schema["title"]) + assert schema["properties"]["schema_version"]["const"] == version From 40a17bf7d2c274da3a5d40ca8f9a1d67c0e114f7 Mon Sep 17 00:00:00 2001 From: Ryan Beckett Date: Fri, 16 Oct 2026 12:17:55 -0700 Subject: [PATCH 2/2] fix(router): make table schema patterns portable across regex engines Replace \s and lookarounds in the routing-table schema with an explicit list of the whitespace characters the loader strips, plus a trimmed definition that rejects outer whitespace. ECMA-262, Python re and RE2 validators now agree with the loader. Derive the contract archive's routing_table_schema from the loader instead of hard-coding it, and check that every embedded table matches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../schemas/routing-table.v1.schema.json | 16 +++- containers/router/tests/contract_corpus.py | 13 ++- containers/router/tests/test_package.py | 9 +- containers/router/tests/test_table_schema.py | 85 ++++++++++++++++++- 4 files changed, 110 insertions(+), 13 deletions(-) diff --git a/containers/router/schemas/routing-table.v1.schema.json b/containers/router/schemas/routing-table.v1.schema.json index 5607b8b00..eace4a17c 100644 --- a/containers/router/schemas/routing-table.v1.schema.json +++ b/containers/router/schemas/routing-table.v1.schema.json @@ -3,7 +3,7 @@ "$id": "https://raw.githubusercontent.com/github/gh-aw-firewall/main/containers/router/schemas/routing-table.v1.schema.json", "title": "gh-aw-router routing table, schema version 1", "description": "One routing table file for one fixed profile. A document that validates against this schema can still be rejected by the router, which also enforces these cross-reference rules: (1) every label cell, 210 in total, appears in exactly one ranking group's applies_to; (2) the first ranking group's choices define the model catalogue, and every other group's choices and classification_choices are permutations of it; (3) no two ranking groups have the same choices, so cells with the same ranking share one group; (4) a model listed with an explicit effort is not also listed without one. Tables loaded together must also share the same repository, catalogue and classification_choices, and serve each profile at most once. This file is frozen once published: a format change adds a new schema file and a new schema_version.", - "$comment": "Every pattern ends with $(?!\\n) so that validators using Python regular expressions, where $ also matches before a final newline, behave like ECMA-262.", + "$comment": "Patterns use only literal characters, explicit character classes, groups, alternation, quantifiers, ^ and $, with no \\s or lookaround, so ECMA-262, Python re and RE2 validators agree. Whitespace is listed explicitly, as literal characters written with JSON \\u escapes, and is the set that Python's str.strip() removes, which is what the router checks. In Python, $ also matches before a final newline, so every pattern-checked string also references trimmed, which rejects a trailing newline in every engine.", "type": "object", "additionalProperties": false, "required": ["schema_version", "profile", "repository", "classification_choices", "rankings"], @@ -26,7 +26,7 @@ "description": "The repository the rankings were generated for, or global. No leading or trailing whitespace.", "type": "string", "minLength": 1, - "pattern": "^(?!\\s)[\\s\\S]*(? bytes: @@ -36,7 +43,7 @@ def synthetic_table_document( choices: tuple[str, str] = ("provider/fast", "provider/reasoning:medium"), ) -> dict[str, Any]: return { - "schema_version": 1, + "schema_version": TABLE_SCHEMA_VERSION, "profile": {"goal": "cost", "mode": "balanced"}, "repository": "global", "classification_choices": list(choices), @@ -197,7 +204,7 @@ def export_archive( { "archive_format": 1, "router_version": __version__, - "routing_table_schema": 5, + "routing_table_schema": TABLE_SCHEMA_VERSION, "source": source, "integration_contract_revision": revision.group(1), "files": { diff --git a/containers/router/tests/test_package.py b/containers/router/tests/test_package.py index 54610252c..99d9ee98b 100644 --- a/containers/router/tests/test_package.py +++ b/containers/router/tests/test_package.py @@ -31,7 +31,7 @@ import gh_aw_router import gh_aw_router.cli as cli from gh_aw_router.http import create_app -from gh_aw_router.routing_table import DEFAULT_TABLE_DIRECTORY +from gh_aw_router.routing_table import DEFAULT_TABLE_DIRECTORY, TableDocument from gh_aw_router.service import GhAwRouterService PROJECT_ROOT = Path(__file__).resolve().parents[1] @@ -173,7 +173,12 @@ def test_contract_archive_is_reproducible_and_replays_reviewed_cases( manifest = json.loads(files.pop("manifest.json")) assert manifest["archive_format"] == 1 assert manifest["router_version"] == gh_aw_router.__version__ - assert manifest["routing_table_schema"] == 5 + assert manifest["routing_table_schema"] == TableDocument.model_fields["schema_version"].default + assert { + json.loads(data)["schema_version"] + for name, data in files.items() + if name.startswith("tables/") + } == {manifest["routing_table_schema"]} assert manifest["source"]["development"] is True assert manifest["integration_contract_revision"] == "test/v1" assert manifest["files"] == { diff --git a/containers/router/tests/test_table_schema.py b/containers/router/tests/test_table_schema.py index cb56ffa69..6aef533f6 100644 --- a/containers/router/tests/test_table_schema.py +++ b/containers/router/tests/test_table_schema.py @@ -38,9 +38,10 @@ CURRENT_VERSION = get_args(TableDocument.model_fields["schema_version"].annotation)[0] # A published schema never changes. A format change adds a new file and pins its hash here. PUBLISHED_SCHEMAS = { - 1: "e522d397a8b595dba741189dbafbe00ebc369e0f196bf02280a5d85f008e5bcd", + 1: "c76de15239db5d6ee9bfc3fa4f4b797887efe04f40334e2ca9e8afe5377461cd", } -PATTERN_END = r"$(?!\n)" +# The loader trims with str.strip(), which removes exactly the characters str.isspace() accepts. +WHITESPACE = "".join(chr(code) for code in range(0x110000) if chr(code).isspace()) type Mutation = Callable[[dict[str, Any]], None] @@ -81,6 +82,29 @@ def _alternation(enum: type[Any]) -> str: return "(" + "|".join(item.value for item in enum) + ")" +def _character_class_body(characters: str) -> str: + """List characters as literals, collapsing runs of three or more into ranges.""" + runs: list[list[int]] = [] + for code in map(ord, characters): + if runs and code == runs[-1][1] + 1: + runs[-1][1] = code + else: + runs.append([code, code]) + return "".join( + chr(first) + ("-" + chr(last) if last - first > 1 else chr(last) if last > first else "") + for first, last in runs + ) + + +def _patterns(node: object) -> list[str]: + if isinstance(node, dict): + found = [node["pattern"]] if isinstance(node.get("pattern"), str) else [] + return found + [pattern for value in node.values() for pattern in _patterns(value)] + if isinstance(node, list): + return [pattern for value in node for pattern in _patterns(value)] + return [] + + @functools.cache def _router_tables() -> dict[str, bytes]: tables = { @@ -124,10 +148,63 @@ def test_schema_constants_match_the_runtime() -> None: def test_schema_patterns_are_built_from_the_runtime_enums() -> None: definitions = _schema()["$defs"] labels = "/".join(_alternation(enum) for enum in (TaskType, TaskScope, TaskComplexity)) - assert definitions["labelKey"]["pattern"] == f"^{labels}{PATTERN_END}" + space = _character_class_body(WHITESPACE) + assert definitions["labelKey"]["pattern"] == f"^{labels}$" assert definitions["modelIdentity"]["pattern"] == ( - rf"^[^/:\s][^/:]*/[^/:]*[^/:\s](:{_alternation(ReasoningEffort)})?{PATTERN_END}" + f"^[^/:{space}][^/:]*/[^/:]*[^/:{space}](:{_alternation(ReasoningEffort)})?$" ) + assert definitions["trimmed"] == { + "description": definitions["trimmed"]["description"], + "not": {"pattern": f"^[{space}]|[{space}]$"}, + } + + +def test_schema_patterns_use_only_syntax_that_every_engine_shares() -> None: + # No escapes (\s differs between Python and ECMA-262, \u is not RE2) and no lookaround. + patterns = _patterns(_schema()) + assert len(patterns) == 3 + for pattern in patterns: + assert "\\" not in pattern, pattern + assert "(?" not in pattern, pattern + re.compile(pattern) + + +def test_pattern_checked_strings_also_reject_outer_whitespace() -> None: + definitions = _schema()["$defs"] + trimmed = {"$ref": "#/$defs/trimmed"} + assert _schema()["properties"]["repository"]["$ref"] == trimmed["$ref"] + for name in ("labelKey", "modelIdentity"): + assert definitions[name]["$ref"] == trimmed["$ref"] + + +# Every character the loader strips, plus format and separator characters it keeps. +EDGE_CHARACTERS = [*WHITESPACE, "\ufeff", "\u200b", "\u180e"] + + +@pytest.mark.parametrize("character", EDGE_CHARACTERS, ids=lambda c: f"U+{ord(c):04X}") +def test_schema_and_loader_agree_on_outer_whitespace(character: str) -> None: + candidates: list[tuple[str, str]] = [ + ("repository", f"{character}global"), + ("repository", f"global{character}"), + ("identity", f"{character}provider/fast"), + ("identity", f"provider/fast{character}"), + ("identity", f"provider/reasoning{character}:medium"), + ] + for field, value in candidates: + document = synthetic_table_document() + if field == "repository": + document["repository"] = value + else: + original = "provider/reasoning:medium" if ":" in value else "provider/fast" + document["classification_choices"] = [ + value if choice == original else choice + for choice in document["classification_choices"] + ] + for group in document["rankings"]: + group["choices"] = [ + value if choice == original else choice for choice in group["choices"] + ] + assert _validator().is_valid(document) is _loader_accepts(document), (field, value) @pytest.mark.parametrize(