diff --git a/.github/workflows/local-ci.yml b/.github/workflows/local-ci.yml index 0c5fab515dab..cbc1aeba66f8 100644 --- a/.github/workflows/local-ci.yml +++ b/.github/workflows/local-ci.yml @@ -30,6 +30,24 @@ jobs: - name: Check fork surface if: hashFiles('local/scripts/check_fork_surface.py') != '' run: python3 local/scripts/check_fork_surface.py + # TODO: switch to `uses: randlee/sc-lint@v1` (operation: setup, pinned + # released version) once sc-lint ships prefix patterns + SCB-FEATURE-001 + # (randlee/sc-lint#603). Until then, build the PR commit from git. + - name: Cache sc-lint + id: sc-lint-cache + uses: actions/cache@v4 + with: + path: | + ~/.cargo/bin/sc-lint + ~/.cargo/bin/sc-lint-boundary + key: sc-lint-bc19f2b-${{ runner.os }} + - name: Install sc-lint + if: steps.sc-lint-cache.outputs.cache-hit != 'true' + run: cargo install --locked --git https://github.com/ehrvs/sc-lint --rev bc19f2b sc-lint sc-lint-boundary + - name: sc-lint boundary (fork independence) + if: hashFiles('codex-rs/sc-lint.toml') != '' + working-directory: codex-rs + run: sc-lint lint sc-boundary --rule-filter dependencies python: runs-on: ubuntu-latest diff --git a/codex-rs/boundaries/codex-local-cli/codex-local-cli.toml b/codex-rs/boundaries/codex-local-cli/codex-local-cli.toml new file mode 100644 index 000000000000..7ba2bbe72069 --- /dev/null +++ b/codex-rs/boundaries/codex-local-cli/codex-local-cli.toml @@ -0,0 +1,37 @@ +boundary_id = "BOUNDARY-CodexLocalCli" +owner_package = "codex-local-cli" +owner_crate_path = "codex_local_cli" +name = "CodexLocalCli" + +[public] +facade = "Homes" + +[implementation] +type = "Homes" +module = "codex_local_cli" +visibility = "public" +constructor = "none" + +[composition] +roots = ["Homes"] + +[dependencies] +allowed_dependents = [] +allowed_dependencies = ["codex-*"] +allowed_feature_dependencies = [] +forbidden_edges = [] + +[references] +scope = "outside_owner_crate" +forbidden = [] + +[testing] +allowed_test_double_paths = [] +forbidden_test_bypasses = [] + +[enforcement] +lint_rules = [] +review_gates = ["fork_independence"] + +[status] +state = "concrete_landed" diff --git a/codex-rs/boundaries/codex-local-models/codex-local-models.toml b/codex-rs/boundaries/codex-local-models/codex-local-models.toml new file mode 100644 index 000000000000..e26975abc9f1 --- /dev/null +++ b/codex-rs/boundaries/codex-local-models/codex-local-models.toml @@ -0,0 +1,37 @@ +boundary_id = "BOUNDARY-CodexLocalModels" +owner_package = "codex-local-models" +owner_crate_path = "codex_local_models" +name = "CodexLocalModels" + +[public] +facade = "Engine" + +[implementation] +type = "Engine" +module = "codex_local_models" +visibility = "public" +constructor = "none" + +[composition] +roots = ["Engine"] + +[dependencies] +allowed_dependents = ["codex-local-cli"] +allowed_dependencies = ["codex-http-client", "codex-login", "codex-model-provider", "codex-model-provider-info", "codex-models-manager", "codex-protocol"] +allowed_feature_dependencies = [] +forbidden_edges = [] + +[references] +scope = "outside_owner_crate" +forbidden = [] + +[testing] +allowed_test_double_paths = [] +forbidden_test_bypasses = [] + +[enforcement] +lint_rules = [] +review_gates = ["fork_independence"] + +[status] +state = "concrete_landed" diff --git a/codex-rs/boundaries/codex-local-resume/codex-local-resume.toml b/codex-rs/boundaries/codex-local-resume/codex-local-resume.toml new file mode 100644 index 000000000000..daf1312eeffe --- /dev/null +++ b/codex-rs/boundaries/codex-local-resume/codex-local-resume.toml @@ -0,0 +1,37 @@ +boundary_id = "BOUNDARY-CodexLocalResume" +owner_package = "codex-local-resume" +owner_crate_path = "codex_local_resume" +name = "CodexLocalResume" + +[public] +facade = "sanitize_history" + +[implementation] +type = "sanitize_history" +module = "codex_local_resume" +visibility = "public" +constructor = "none" + +[composition] +roots = ["sanitize_history"] + +[dependencies] +allowed_dependents = ["codex-local-cli"] +allowed_dependencies = ["codex-core", "codex-protocol", "codex-tui"] +allowed_feature_dependencies = [] +forbidden_edges = [] + +[references] +scope = "outside_owner_crate" +forbidden = [] + +[testing] +allowed_test_double_paths = [] +forbidden_test_bypasses = [] + +[enforcement] +lint_rules = [] +review_gates = ["fork_independence"] + +[status] +state = "concrete_landed" diff --git a/codex-rs/boundaries/planning.toml b/codex-rs/boundaries/planning.toml new file mode 100644 index 000000000000..7d2e086363b9 --- /dev/null +++ b/codex-rs/boundaries/planning.toml @@ -0,0 +1,2 @@ +[planning] +current_sprint = "1.1" diff --git a/codex-rs/sc-lint.toml b/codex-rs/sc-lint.toml new file mode 100644 index 000000000000..6ed47ca44aa1 --- /dev/null +++ b/codex-rs/sc-lint.toml @@ -0,0 +1,9 @@ +# Fork-owned. Enforces crate/feature independence between codex-rs/local/* and +# upstream crates (see boundaries/). Only the dependency rules run, so +# upstream manifests are not linted. +[tool.sc-lint] +minimum_version = "0.4.0" + +[[tool.sc-lint.lint]] +name = "fork-boundary" +command = ["sc-lint", "lint", "sc-boundary", "--rule-filter", "dependencies"] diff --git a/local/docs/plan-deltas.md b/local/docs/plan-deltas.md index 1c0ddeb78af1..030056819161 100644 --- a/local/docs/plan-deltas.md +++ b/local/docs/plan-deltas.md @@ -3,7 +3,7 @@ Where the build differs from the approved plan. - **GitHub home:** PixelatedMountainStream/codex, with stacked PRs on base `codex-local/main`. The Radiant-Vision-Systems fork is deferred, because org members can't create public repos. -- **Boundary check:** sc-lint boundary records were replaced by a `cargo metadata` check in `check_fork_surface.py`. sc-lint has no wildcard mode and would need a static list of 161 crates. +- **Boundary check:** sc-lint enforces crate and feature independence between `codex-rs/local/*` and upstream (`codex-rs/sc-lint.toml`, records in `codex-rs/boundaries/`). It relies on trailing-`*` prefix patterns and SCB-FEATURE-001 from randlee/sc-lint#603, so CI installs a pinned git revision until a release exists. `check_fork_surface.py` only checks that every `codex-local-*` crate has a record. - **New hook H8:** in `core/src/tools/router.rs` (splits flattened `ns.name` tool names on a registry miss). - **H1:** uses `include!` of upstream `cli/src/main.rs` under `cfg(codex_local)`. `codex-local-cli`'s dependencies must mirror upstream cli's. - **H4:** also drops the provider filter for named lookups when registered. diff --git a/local/fork-surface.toml b/local/fork-surface.toml index 67b8e0e55f2e..1278b5a0add0 100644 --- a/local/fork-surface.toml +++ b/local/fork-surface.toml @@ -11,7 +11,8 @@ # the budget (e.g. Cargo.lock; verify by regeneration). # ignore list of globs OPTIONAL extra fork-owned globs, appended to the # built-in set, which is always ignored: local/**, -# codex-rs/local/**, boundaries/**, .claude/**, +# codex-rs/local/**, boundaries/**, +# codex-rs/boundaries/**, codex-rs/sc-lint.toml, .claude/**, # .codex/skills/**, .github/workflows/local-*.yml, # .github/workflows/upstream-sync.yml # [budget] diff --git a/local/scripts/check_fork_surface.py b/local/scripts/check_fork_surface.py index 838f6a11325f..6ef18a5aa8b6 100755 --- a/local/scripts/check_fork_surface.py +++ b/local/scripts/check_fork_surface.py @@ -4,6 +4,9 @@ Prints one JSON object {ok, upstream, files[], total_lines, violations[]} and exits 0 when ok, 1 on violations, 2 on a tool error (bad config, missing tag). The policy schema is documented at the top of local/fork-surface.toml. +Crate/feature dependency edges are not checked here: sc-lint owns them +(codex-rs/sc-lint.toml + codex-rs/boundaries/). This script only verifies each +codex-local-* crate has a boundary record. """ import argparse import json @@ -18,6 +21,8 @@ "codex-rs/local/**", "local/**", "boundaries/**", + "codex-rs/boundaries/**", + "codex-rs/sc-lint.toml", ".claude/**", ".codex/skills/**", ".github/workflows/local-*.yml", @@ -63,35 +68,22 @@ def resolve_ref(root, tag): raise RuntimeError(f"upstream ref not found: {tag} (also tried upstream/{tag})") -def local_dependency_violations(root): - """No crate outside codex-local-* may depend on codex-local-*, and only - codex-local-cli may depend on another local crate (it is the only root). +def boundary_record_violations(root): + """Every codex-local-* crate under codex-rs/local/ needs a boundary record + directory codex-rs/boundaries// with at least one .toml file. - sc-lint has no wildcard/default-allow mode (allowed_dependencies is - mandatory, so local crates could not depend on arbitrary upstream crates), - so this rule is enforced here, generically, with no crate lists. + The dependency/feature edges themselves are enforced by sc-lint + (codex-rs/sc-lint.toml, run in local-ci); this only guards coverage. """ - ws = Path(root) / "codex-rs" - if not (ws / "Cargo.toml").exists(): - return [] - r = subprocess.run(["cargo", "metadata", "--no-deps", "--format-version", "1"], - cwd=ws, capture_output=True, text=True) - if r.returncode: - raise RuntimeError("cargo metadata: " + r.stderr.strip()[-300:]) + local = Path(root) / "codex-rs" / "local" out = [] - for pkg in json.loads(r.stdout)["packages"]: - name = pkg["name"] - for dep in pkg["dependencies"]: - if not dep["name"].startswith("codex-local-"): - continue - if not name.startswith("codex-local-"): - out.append({"kind": "upstream_depends_on_local", "crate": name, - "dependency": dep["name"]}) - elif name != "codex-local-cli" and dep["name"] != name: - out.append({"kind": "local_not_callable_from_cli_only", "crate": name, - "dependency": dep["name"]}) - elif name == "codex-local-cli" and dep["name"] == name: - continue + for manifest in sorted(local.glob("*/Cargo.toml")): + name = tomllib.loads(manifest.read_text()).get("package", {}).get("name", "") + if not name.startswith("codex-local-"): + continue + rec = Path(root) / "codex-rs" / "boundaries" / name + if not any(rec.glob("*.toml")): + out.append({"kind": "missing_boundary_record", "crate": name}) return out @@ -123,7 +115,7 @@ def check(root): violations.append({"kind": "over_line_budget", "lines": total, "max": max_lines}) if max_files is not None and len(files) > max_files: violations.append({"kind": "over_file_budget", "files": len(files), "max": max_files}) - violations += local_dependency_violations(root) + violations += boundary_record_violations(root) return { "ok": not violations, "upstream": tag, diff --git a/local/scripts/sync_upstream.py b/local/scripts/sync_upstream.py index f6dd267504e2..72d73f8387fc 100755 --- a/local/scripts/sync_upstream.py +++ b/local/scripts/sync_upstream.py @@ -120,9 +120,11 @@ def classify(path, allow, generated): def unmerged(root): - out = git(root, "diff", "--name-only", "--diff-filter=U").stdout + # index stage entries are the authoritative record of unmerged paths + out = git(root, "ls-files", "-u", "-z").stdout + paths = sorted({e.split("\t", 1)[1] for e in out.split("\0") if "\t" in e}) allow, generated = load_policy(root) - return [{"path": p, "kind": classify(p, allow, generated)} for p in out.splitlines() if p] + return [{"path": p, "kind": classify(p, allow, generated)} for p in paths] def conflict_result(step, root): diff --git a/local/tests/test_sync_upstream.py b/local/tests/test_sync_upstream.py index 4b5a9b3ab2a0..97b2ef27ffff 100644 --- a/local/tests/test_sync_upstream.py +++ b/local/tests/test_sync_upstream.py @@ -1,5 +1,6 @@ import contextlib import io +import os import json import subprocess import sys @@ -14,8 +15,18 @@ KEYS = {"ok", "step", "conflicts", "build_errors", "test_failures", "surface_violations", "next_action"} -def git(cwd, *a): - subprocess.run(["git", "-c", "user.name=t", "-c", "user.email=t@t", *a], cwd=cwd, check=True, capture_output=True) +HERMETIC = ["-c", "user.name=t", "-c", "user.email=t@t", "-c", "init.defaultBranch=main", + "-c", "commit.gpgsign=false", "-c", "tag.gpgsign=false"] + + +def _env(): + env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} + env.update(GIT_CONFIG_GLOBAL=os.devnull, GIT_CONFIG_NOSYSTEM="1") + return env + + +def git(cwd, *a, check=True): + return subprocess.run(["git", *HERMETIC, *a], cwd=cwd, check=check, capture_output=True, text=True, env=_env()) def run_cli(root, *args): @@ -78,7 +89,8 @@ def test_conflicts_clean_and_escalate(self): git(self.work, "add", "z.rs"); git(self.work, "commit", "-qm", "1") git(self.work, "checkout", "-q", "-b", "m2", "m1~1"); (self.work / "z.rs").write_text("2") git(self.work, "add", "z.rs"); git(self.work, "commit", "-qm", "2") - subprocess.run(["git", "merge", "m1"], cwd=self.work, capture_output=True) + git(self.work, "merge", "m1", check=False) # expected to conflict + self.assertTrue(git(self.work, "ls-files", "-u").stdout.strip(), "setup produced no conflict") code, out = run_cli(self.work, "conflicts") self.assertEqual(code, 2) self.assertEqual(out["conflicts"], [{"path": "z.rs", "kind": "other"}])