diff --git a/.codex/skills/protocol-breaking-changes/SKILL.md b/.codex/skills/protocol-breaking-changes/SKILL.md new file mode 100644 index 0000000000..d438f64442 --- /dev/null +++ b/.codex/skills/protocol-breaking-changes/SKILL.md @@ -0,0 +1,21 @@ +--- +name: protocol-breaking-changes +description: Test app-server protocol edits for stable client breakage, triage compatibility-lint failures, and record intentional request-schema breaks. Use for changes or CI failures involving the app-server wire format. +--- + +# Protocol breaking changes + +Regenerate the stable schema and compare it with the merge base: + +```sh +just app-server-schema-lint +``` + +This uses `origin/main` by default. Set `CODEX_SCHEMA_BASE_REF` to compare with another fetched base. + +Treat removed methods or fields, newly required fields, and narrower types, enums, unions, or constraints as breaking. +The command exits non-zero for every detected breakage, including recorded ones. + +For a failure, read its method, path, before value, and after value. Inspect the generating protocol type, especially optional or nullable inputs made mandatory. Fix accidental narrowing and rerun. + +For an intentional break, agents must show the human its method, path, before/after values, and proposed justification, then get explicit permission to change the protocol and log. After approval, append the emitted `[[breakages]]` block to `codex-rs/app-server-protocol/stable-api-breakages.toml`. Discuss only wire-format rationale and client-visible compatibility action. Never include internal details, plans, codenames, timelines, customer identities, or other confidential information. Never edit, reorder, or delete existing entries. Rerun the lint to verify the entry. diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 41a1e69229..df96de010e 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -5,6 +5,9 @@ /codex-rs/prompts/ @openai/codex-core-agent-team /codex-rs/utils/path-uri/ @openai/codex-core-agent-team +# Keep stable app-server known breakages reviewed by protocol owners. +/codex-rs/app-server-protocol/stable-api-breakages.toml @openai/codex-core-agent-team + # Keep macOS AKV signing changes reviewed by Codex maintainers. /.github/actions/setup-akv-pkcs11-codesigning/ @openai/codex-core-agent-team /.github/scripts/macos-signing/ @openai/codex-core-agent-team diff --git a/codex-rs/app-server-protocol/stable-api-breakages.toml b/codex-rs/app-server-protocol/stable-api-breakages.toml new file mode 100644 index 0000000000..83c24482e8 --- /dev/null +++ b/codex-rs/app-server-protocol/stable-api-breakages.toml @@ -0,0 +1,3 @@ +# See `.codex/skills/protocol-breaking-changes/SKILL.md` for the canonical workflow. +# Existing entries are append-only. +version = 1 diff --git a/justfile b/justfile index 03f87391ac..cc5b9f5163 100644 --- a/justfile +++ b/justfile @@ -148,6 +148,10 @@ write-config-schema: write-app-server-schema *args: cargo run -p codex-app-server-protocol --bin write_schema_fixtures -- {args} +# Check the generated stable client request schema against the merge base. +app-server-schema-lint: write-app-server-schema + {{ python }} ../scripts/app_server_schema_lint.py + [no-cd] write-hooks-schema: cargo run --manifest-path {{ justfile_directory() }}/codex-rs/Cargo.toml -p codex-hooks --bin write_hooks_schema_fixtures diff --git a/scripts/app_server_schema_lint.py b/scripts/app_server_schema_lint.py new file mode 100644 index 0000000000..92783c2fc0 --- /dev/null +++ b/scripts/app_server_schema_lint.py @@ -0,0 +1,107 @@ +#!/usr/bin/env python3 + +import json +import os +import subprocess +import sys +from collections.abc import Callable +from collections.abc import Mapping +from pathlib import Path +from typing import Any + + +# TODO(anp): Extend this lint workflow to cover server-sent replies. +SCHEMA_PATH = "codex-rs/app-server-protocol/schema/json/ClientRequest.json" +KNOWN_BREAKAGES_PATH = "codex-rs/app-server-protocol/stable-api-breakages.toml" +DEFAULT_BASE_REF = "origin/main" +EMPTY_KNOWN_BREAKAGES = "version = 1\n" +SCHEMA_EVOLUTION_TARGET = "//codex-rs/schema-evolution:codex-schema-evolution" + +GitRunner = Callable[..., str] + + +def run_git(root: Path, *args: str) -> str: + process = subprocess.run( + ["git", "-C", str(root), *args], + check=False, + capture_output=True, + encoding="utf-8", + ) + if process.returncode != 0: + detail = process.stderr.strip() or "no error output" + raise RuntimeError(f"git {' '.join(args)} failed: {detail}") + return process.stdout + + +def find_repo_root() -> Path: + process = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], + check=False, + capture_output=True, + encoding="utf-8", + ) + if process.returncode != 0: + detail = process.stderr.strip() or "no error output" + raise RuntimeError(f"git rev-parse failed: {detail}") + return Path(process.stdout.strip()) + + +def schema_base_ref(environ: Mapping[str, str] = os.environ) -> str: + return environ.get("CODEX_SCHEMA_BASE_REF", "").strip() or DEFAULT_BASE_REF + + +def build_lint_input( + root: Path, + base_ref: str, + *, + git: GitRunner = run_git, +) -> dict[str, Any]: + base = git(root, "merge-base", "HEAD", base_ref).strip() + if not base: + raise RuntimeError(f"git merge-base HEAD {base_ref} returned no revision") + + baseline_log_path = git( + root, + "ls-tree", + "--name-only", + base, + "--", + KNOWN_BREAKAGES_PATH, + ).strip() + before_known_breakages = EMPTY_KNOWN_BREAKAGES + if baseline_log_path == KNOWN_BREAKAGES_PATH: + before_known_breakages = git(root, "show", f"{base}:{KNOWN_BREAKAGES_PATH}") + + return { + "before": json.loads(git(root, "show", f"{base}:{SCHEMA_PATH}")), + "after": json.loads((root / SCHEMA_PATH).read_text(encoding="utf-8")), + "beforeKnownBreakages": before_known_breakages, + "afterKnownBreakages": (root / KNOWN_BREAKAGES_PATH).read_text( + encoding="utf-8" + ), + } + + +def run_schema_evolution(root: Path, payload: Mapping[str, Any]) -> int: + process = subprocess.run( + ["bazel", "run", SCHEMA_EVOLUTION_TARGET], + check=False, + cwd=root, + input=json.dumps(payload, separators=(",", ":")) + "\n", + encoding="utf-8", + ) + return process.returncode + + +def main() -> int: + try: + root = find_repo_root() + payload = build_lint_input(root, schema_base_ref()) + return run_schema_evolution(root, payload) + except (OSError, RuntimeError, ValueError) as error: + print(f"app-server schema lint failed: {error}", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/test_app_server_schema_lint.py b/scripts/test_app_server_schema_lint.py new file mode 100644 index 0000000000..6da237d772 --- /dev/null +++ b/scripts/test_app_server_schema_lint.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 + +import json +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +import app_server_schema_lint as lint + + +class AppServerSchemaLintTest(unittest.TestCase): + def test_builds_input_from_merge_base_and_current_files(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + root = Path(temp_dir) + write_current_files(root) + outputs = { + ("merge-base", "HEAD", "base-ref"): "abc123\n", + ( + "ls-tree", + "--name-only", + "abc123", + "--", + lint.KNOWN_BREAKAGES_PATH, + ): f"{lint.KNOWN_BREAKAGES_PATH}\n", + ( + "show", + f"abc123:{lint.KNOWN_BREAKAGES_PATH}", + ): "version = 1\n# baseline\n", + ( + "show", + f"abc123:{lint.SCHEMA_PATH}", + ): '{"oneOf": [{"before": true}]}', + } + + payload = lint.build_lint_input( + root, + "base-ref", + git=lambda _root, *args: outputs[args], + ) + + self.assertEqual( + payload, + { + "before": {"oneOf": [{"before": True}]}, + "after": {"oneOf": [{"after": True}]}, + "beforeKnownBreakages": "version = 1\n# baseline\n", + "afterKnownBreakages": "version = 1\n# current\n", + }, + ) + + def test_missing_baseline_log_uses_empty_log(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + root = Path(temp_dir) + write_current_files(root) + outputs = { + ("merge-base", "HEAD", "origin/main"): "abc123\n", + ( + "ls-tree", + "--name-only", + "abc123", + "--", + lint.KNOWN_BREAKAGES_PATH, + ): "", + ( + "show", + f"abc123:{lint.SCHEMA_PATH}", + ): '{"oneOf": []}', + } + + payload = lint.build_lint_input( + root, + "origin/main", + git=lambda _root, *args: outputs[args], + ) + + self.assertEqual(payload["beforeKnownBreakages"], lint.EMPTY_KNOWN_BREAKAGES) + + def test_base_ref_defaults_and_trims_override(self) -> None: + self.assertEqual(lint.schema_base_ref({}), "origin/main") + self.assertEqual( + lint.schema_base_ref({"CODEX_SCHEMA_BASE_REF": " feature/base "}), + "feature/base", + ) + + def test_bazel_exit_code_and_payload_are_preserved(self) -> None: + payload = {"before": {}, "after": {}} + with mock.patch.object(lint.subprocess, "run") as run: + run.return_value = subprocess.CompletedProcess([], 23) + + returncode = lint.run_schema_evolution(Path("/repo"), payload) + + self.assertEqual(returncode, 23) + run.assert_called_once_with( + ["bazel", "run", lint.SCHEMA_EVOLUTION_TARGET], + check=False, + cwd=Path("/repo"), + input=json.dumps(payload, separators=(",", ":")) + "\n", + encoding="utf-8", + ) + + +def write_current_files(root: Path) -> None: + schema = root / lint.SCHEMA_PATH + schema.parent.mkdir(parents=True) + schema.write_text('{"oneOf": [{"after": true}]}', encoding="utf-8") + log = root / lint.KNOWN_BREAKAGES_PATH + log.write_text("version = 1\n# current\n", encoding="utf-8") + + +if __name__ == "__main__": + unittest.main()