From e3eba1e96403867c5864f13d40c9d476b3e41a38 Mon Sep 17 00:00:00 2001 From: Herbert Damker <52109189+hdamker@users.noreply.github.com> Date: Thu, 6 Aug 2026 12:21:08 +0200 Subject: [PATCH 1/3] feat(validation): add bundler component-renaming conflict check (P-040) --- documentation/validation/faq.md | 24 ++ validation/engines/python_checks/__init__.py | 6 + .../engines/python_checks/bundling_checks.py | 107 +++++ validation/rules/python-rules.yaml | 28 ++ validation/rules/rule-inventory.yaml | 4 +- .../tests/test_python_checks_bundling.py | 366 ++++++++++++++++++ .../tests/test_rule_metadata_integrity.py | 6 +- 7 files changed, 536 insertions(+), 5 deletions(-) create mode 100644 validation/engines/python_checks/bundling_checks.py create mode 100644 validation/tests/test_python_checks_bundling.py diff --git a/documentation/validation/faq.md b/documentation/validation/faq.md index 550bd54f..539a3fac 100644 --- a/documentation/validation/faq.md +++ b/documentation/validation/faq.md @@ -177,6 +177,30 @@ Do not edit `code/common/info-description-templates.yaml` in the API repository. + +
+[P-040] Why does bundling rename a component I didn't touch? + +Applies to: `[P-040] Component-renaming collision during bundling` + +Two different components — one defined locally, one pulled in from a common file via an +external `$ref` — are competing for the same name with different content. When the spec is +bundled, the local definition keeps the name; the externally-referenced one is renamed to +`-2`, and every use site that resolved to it is silently repointed at the renamed copy. +This is not a bundler quirk to ignore: a reader of the bundled spec sees the wrong schema at +those use sites. + +This only fires for a genuine conflict — same name, **different** content. A local component +that intentionally proxies a common one under the same name, with identical content, is not +flagged. + +**What you do:** rename the *local* definition to a distinct, API-specific name — you can't +rename the common definition, since Commonalities owns it. If the local definition isn't +actually needed (it has no reason to diverge from the common one), delete it and reference the +common component directly instead. + +
+ ## Related - [Validation problem messages](problem-messages.md) — the shape of each message diff --git a/validation/engines/python_checks/__init__.py b/validation/engines/python_checks/__init__.py index 369110b8..adfda94b 100644 --- a/validation/engines/python_checks/__init__.py +++ b/validation/engines/python_checks/__init__.py @@ -6,6 +6,7 @@ from ._types import CheckDescriptor, CheckScope +from .bundling_checks import check_component_renaming_conflict from .error_code_checks import check_conflict_deprecated, check_contextcode_format from .filename_checks import check_filename_kebab_case, check_filename_matches_api_name from .info_description_checks import check_info_description_templates @@ -70,6 +71,11 @@ CheckScope.API, check_info_description_templates, ), + CheckDescriptor( + "check-component-renaming-conflict", + CheckScope.API, + check_component_renaming_conflict, + ), # --- Repo-level checks (run once) --- CheckDescriptor("check-test-directory-exists", CheckScope.REPO, check_test_directory_exists), CheckDescriptor("check-release-plan-semantics", CheckScope.REPO, check_release_plan_semantics), diff --git a/validation/engines/python_checks/bundling_checks.py b/validation/engines/python_checks/bundling_checks.py new file mode 100644 index 00000000..5848da09 --- /dev/null +++ b/validation/engines/python_checks/bundling_checks.py @@ -0,0 +1,107 @@ +"""Bundler component-renaming conflict check. + +Runs Redocly's bundler with ``--component-renaming-conflicts-severity=error`` +against the current API definition to detect a name shared between a local +component and a different-content component pulled in via an external +``$ref``. Redocly silently renames the loser to ``-2``; this only +reports the cases where the two definitions actually differ (a same-content +proxy/alias is never flagged). +""" + +from __future__ import annotations + +import re +import subprocess +import tempfile +from pathlib import Path +from typing import List + +from validation.context import ValidationContext + +from ._types import make_finding + +_ENGINE_RULE = "check-component-renaming-conflict" +_EXECUTION_ERROR_RULE = "component-renaming-conflict-execution-error" +_EXTERNAL_REF_RE = re.compile(r'\$ref\s*:\s*["\']?\.\.') +_CONFLICT_RE = re.compile( + r'\[\d+\]\s+\S+:\d+:\d+\s+at\s+\S+\s*\n+' + r"Two schemas are referenced with the same name but different content\. " + r'Renamed (?P\S+) to \S+-2\.' +) +_TIMEOUT_SECONDS = 60 + + +def check_component_renaming_conflict( + repo_path: Path, context: ValidationContext +) -> List[dict]: + """Detect bundler component-renaming collisions for the current API. + + API-scoped check — the adapter calls this once per API context. + """ + if not context.apis: + return [] + + api = context.apis[0] + spec_file = api.spec_file or f"code/API_definitions/{api.api_name}.yaml" + full_path = repo_path / spec_file + if not full_path.is_file(): + return [] + + content = full_path.read_text(encoding="utf-8") + if not _EXTERNAL_REF_RE.search(content): + return [] + + with tempfile.TemporaryDirectory() as tmp: + try: + result = subprocess.run( + [ + "redocly", "bundle", spec_file, + "--component-renaming-conflicts-severity=error", + "-o", str(Path(tmp) / "bundled.yaml"), + ], + cwd=repo_path, + capture_output=True, + text=True, + timeout=_TIMEOUT_SECONDS, + check=False, + ) + except (FileNotFoundError, OSError, subprocess.TimeoutExpired) as exc: + return [ + make_finding( + engine_rule=_EXECUTION_ERROR_RULE, + level="error", + message=f"Component-renaming-conflict probe could not run: {exc}", + path=spec_file, + line=1, + api_name=api.api_name, + ) + ] + + if result.returncode == 0: + return [] + + output = result.stdout + result.stderr + matches = list(_CONFLICT_RE.finditer(output)) + if not matches: + return [] + + return [ + make_finding( + engine_rule=_ENGINE_RULE, + level="error", + message=( + f"Bundling collides on the name '{m.group('name')}': the local " + f"definition keeps it, and the different-content component " + f"pulled in via an external $ref is silently renamed to " + f"'{m.group('name')}-2' at its use sites. Give the local " + f"'{m.group('name')}' a distinct, API-specific name — the " + f"common definition isn't yours to rename. If it isn't " + f"actually needed locally, reference the common component " + f"directly instead." + ), + path=spec_file, + line=1, + api_name=api.api_name, + ) + for m in matches + ] diff --git a/validation/rules/python-rules.yaml b/validation/rules/python-rules.yaml index 394272df..c8d62dab 100644 --- a/validation/rules/python-rules.yaml +++ b/validation/rules/python-rules.yaml @@ -596,3 +596,31 @@ Prefer block-style YAML for examples. Rewrite invalid JSON-like flow-style YAML mappings or sequences so the file parses with YAML 1.2-conformant parsers. + +# P-040: check-component-renaming-conflict +# Redocly's bundler silently disambiguates when a local component and a +# different-content component pulled in via an external $ref compete for the +# same name: the local definition keeps the name, the external one is +# renamed to -2 and every use site that resolved to it is repointed. +# A same-name-same-content proxy/alias is never flagged. error only at +# public, warn everywhere else — deliberately skips both the "new +# requirement" warn-only ramp and the initial-API (0.x) relaxation: the fix +# has no wire-format/backward-compatibility cost, so there is nothing for +# either convention to protect. See +# private-dev-docs/validation-rules/bundler-renaming-conflict-detection-design.md +# for the full rationale. +- id: P-040 + engine: python + engine_rule: check-component-renaming-conflict + short_title: "Component-renaming collision during bundling" + documentation_url: "https://github.com/camaraproject/tooling/blob/main/documentation/validation/faq.md#p-040-component-renaming-conflict" + conditional_level: + default: error + overrides: + - condition: + target_api_status: [draft, alpha, rc] + level: warn + suggestion: >- + Give the local component a distinct, API-specific name — the common + definition isn't yours to rename. If it isn't actually needed locally, + reference the common component directly instead. diff --git a/validation/rules/rule-inventory.yaml b/validation/rules/rule-inventory.yaml index d99acad3..4a53e26f 100644 --- a/validation/rules/rule-inventory.yaml +++ b/validation/rules/rule-inventory.yaml @@ -14,7 +14,7 @@ version: 1 generated: 2026-04-07 summary: - total_implemented: 146 + total_implemented: 147 total_gap: 0 total_manual: 25 total_pending: 0 @@ -22,7 +22,7 @@ summary: by_engine: spectral: 85 gherkin: 25 - python: 23 + python: 24 yamllint: 13 # --------------------------------------------------------------------------- diff --git a/validation/tests/test_python_checks_bundling.py b/validation/tests/test_python_checks_bundling.py new file mode 100644 index 00000000..4f696b2b --- /dev/null +++ b/validation/tests/test_python_checks_bundling.py @@ -0,0 +1,366 @@ +"""Tests for the bundler component-renaming conflict check (P-040).""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path +from unittest.mock import Mock + +from validation.context import ApiContext, ValidationContext + + +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +_REDOCLY_BIN_DIR = _REPO_ROOT / "validation" / "node_modules" / ".bin" + + +def _make_context(api_name: str | None = None) -> ValidationContext: + apis = () + if api_name is not None: + apis = ( + ApiContext( + api_name=api_name, + target_api_version="0.1.0", + target_api_status="alpha", + target_api_maturity="initial", + api_pattern="request-response", + spec_file=f"code/API_definitions/{api_name}.yaml", + ), + ) + return ValidationContext( + repository="TestRepo", + branch_type="main", + trigger_type="dispatch", + profile="advisory", + stage="enabled", + target_release_type=None, + commonalities_release=None, + commonalities_version=None, + icm_release=None, + base_ref=None, + is_release_review_pr=False, + release_plan_changed=None, + pr_number=None, + apis=apis, + workflow_run_url="", + tooling_ref="", + ) + + +def _write_spec(tmp_path: Path, name: str, content: str) -> Path: + api_dir = tmp_path / "code" / "API_definitions" + api_dir.mkdir(parents=True, exist_ok=True) + path = api_dir / f"{name}.yaml" + path.write_text(content, encoding="utf-8") + return path + + +_NO_EXTERNAL_REF = """\ +openapi: 3.0.3 +info: + title: Sample + version: 0.1.0 +paths: + /ping: + get: + operationId: ping + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: '#/components/schemas/Foo' +components: + schemas: + Foo: + type: string +""" + +_WITH_EXTERNAL_REF = """\ +openapi: 3.0.3 +info: + title: Sample + version: 0.1.0 +paths: + /ping: + get: + operationId: ping + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: '#/components/schemas/Foo' + '500': + description: err + content: + application/json: + schema: + $ref: '../common/common.yaml#/components/schemas/Foo' +components: + schemas: + Foo: + type: string +""" + +_CONFLICT_STDERR_SINGLE = """\ +bundling code/API_definitions/sample.yaml... +[1] code/API_definitions/sample.yaml:21:17 at #/paths/~1ping/get/responses/500/content/application~1json/schema + +Two schemas are referenced with the same name but different content. Renamed Foo to Foo-2. + +Error was generated by the bundler rule. + +❌ Errors encountered while bundling code/API_definitions/sample.yaml: bundle not created (use --force to ignore errors). +""" + +_CONFLICT_STDERR_DOUBLE = """\ +bundling code/API_definitions/sample.yaml... +[1] code/API_definitions/sample.yaml:21:17 at #/paths/~1ping/get/responses/500/content/application~1json/schema + +Two schemas are referenced with the same name but different content. Renamed Foo to Foo-2. + +[2] code/API_definitions/sample.yaml:29:17 at #/paths/~1pong/get/responses/500/content/application~1json/schema + +Two schemas are referenced with the same name but different content. Renamed Bar to Bar-2. + +❌ Errors encountered while bundling code/API_definitions/sample.yaml: bundle not created (use --force to ignore errors). +""" + +_UNRELATED_FAILURE_STDERR = """\ +bundling code/API_definitions/sample.yaml... +[1] code/API_definitions/sample.yaml:12:17 at #/paths/~1ping/get/responses/200/content/application~1json/schema + +Can't resolve $ref + +❌ Errors encountered while bundling code/API_definitions/sample.yaml: bundle not created (use --force to ignore errors). +""" + + +class TestCheckComponentRenamingConflict: + def test_no_api_context_returns_no_findings(self, tmp_path: Path, monkeypatch): + from validation.engines.python_checks import bundling_checks + + run = Mock() + monkeypatch.setattr(bundling_checks.subprocess, "run", run) + + findings = bundling_checks.check_component_renaming_conflict( + tmp_path, _make_context() + ) + + assert findings == [] + run.assert_not_called() + + def test_missing_spec_file_returns_no_findings(self, tmp_path: Path, monkeypatch): + from validation.engines.python_checks import bundling_checks + + run = Mock() + monkeypatch.setattr(bundling_checks.subprocess, "run", run) + + findings = bundling_checks.check_component_renaming_conflict( + tmp_path, _make_context("sample") + ) + + assert findings == [] + run.assert_not_called() + + def test_no_external_ref_skips_subprocess(self, tmp_path: Path, monkeypatch): + from validation.engines.python_checks import bundling_checks + + _write_spec(tmp_path, "sample", _NO_EXTERNAL_REF) + run = Mock() + monkeypatch.setattr(bundling_checks.subprocess, "run", run) + + findings = bundling_checks.check_component_renaming_conflict( + tmp_path, _make_context("sample") + ) + + assert findings == [] + run.assert_not_called() + + def test_clean_bundle_returns_no_findings(self, tmp_path: Path, monkeypatch): + from validation.engines.python_checks import bundling_checks + + _write_spec(tmp_path, "sample", _WITH_EXTERNAL_REF) + monkeypatch.setattr( + bundling_checks.subprocess, + "run", + Mock(return_value=subprocess.CompletedProcess([], 0, stdout="", stderr="")), + ) + + findings = bundling_checks.check_component_renaming_conflict( + tmp_path, _make_context("sample") + ) + + assert findings == [] + + def test_single_conflict_produces_one_finding(self, tmp_path: Path, monkeypatch): + from validation.engines.python_checks import bundling_checks + + _write_spec(tmp_path, "sample", _WITH_EXTERNAL_REF) + monkeypatch.setattr( + bundling_checks.subprocess, + "run", + Mock( + return_value=subprocess.CompletedProcess( + [], 1, stdout="", stderr=_CONFLICT_STDERR_SINGLE + ) + ), + ) + + findings = bundling_checks.check_component_renaming_conflict( + tmp_path, _make_context("sample") + ) + + assert len(findings) == 1 + finding = findings[0] + assert finding["engine"] == "python" + assert finding["engine_rule"] == "check-component-renaming-conflict" + assert finding["level"] == "error" + assert finding["path"] == "code/API_definitions/sample.yaml" + assert finding["line"] == 1 + assert finding["api_name"] == "sample" + assert "Foo" in finding["message"] + assert "Foo-2" in finding["message"] + assert "isn't yours to rename" in finding["message"] + + def test_multiple_conflicts_produce_multiple_findings( + self, tmp_path: Path, monkeypatch + ): + from validation.engines.python_checks import bundling_checks + + _write_spec(tmp_path, "sample", _WITH_EXTERNAL_REF) + monkeypatch.setattr( + bundling_checks.subprocess, + "run", + Mock( + return_value=subprocess.CompletedProcess( + [], 1, stdout="", stderr=_CONFLICT_STDERR_DOUBLE + ) + ), + ) + + findings = bundling_checks.check_component_renaming_conflict( + tmp_path, _make_context("sample") + ) + + assert len(findings) == 2 + messages = " ".join(f["message"] for f in findings) + assert "Foo" in messages + assert "Bar" in messages + + def test_unrelated_bundle_failure_returns_no_findings( + self, tmp_path: Path, monkeypatch + ): + from validation.engines.python_checks import bundling_checks + + _write_spec(tmp_path, "sample", _WITH_EXTERNAL_REF) + monkeypatch.setattr( + bundling_checks.subprocess, + "run", + Mock( + return_value=subprocess.CompletedProcess( + [], 1, stdout="", stderr=_UNRELATED_FAILURE_STDERR + ) + ), + ) + + findings = bundling_checks.check_component_renaming_conflict( + tmp_path, _make_context("sample") + ) + + assert findings == [] + + def test_subprocess_missing_returns_execution_error( + self, tmp_path: Path, monkeypatch + ): + from validation.engines.python_checks import bundling_checks + + _write_spec(tmp_path, "sample", _WITH_EXTERNAL_REF) + monkeypatch.setattr( + bundling_checks.subprocess, + "run", + Mock(side_effect=FileNotFoundError("redocly not found")), + ) + + findings = bundling_checks.check_component_renaming_conflict( + tmp_path, _make_context("sample") + ) + + assert len(findings) == 1 + assert findings[0]["engine_rule"] == "component-renaming-conflict-execution-error" + assert findings[0]["level"] == "error" + + def test_subprocess_timeout_returns_execution_error( + self, tmp_path: Path, monkeypatch + ): + from validation.engines.python_checks import bundling_checks + + _write_spec(tmp_path, "sample", _WITH_EXTERNAL_REF) + monkeypatch.setattr( + bundling_checks.subprocess, + "run", + Mock(side_effect=subprocess.TimeoutExpired(cmd="redocly", timeout=60)), + ) + + findings = bundling_checks.check_component_renaming_conflict( + tmp_path, _make_context("sample") + ) + + assert len(findings) == 1 + assert findings[0]["engine_rule"] == "component-renaming-conflict-execution-error" + assert findings[0]["level"] == "error" + + +class TestComponentRenamingConflictIntegration: + """Exercises the real ``redocly`` binary, not a mocked subprocess. + + Guards against Redocly changing its renaming-conflict message shape out + from under ``_CONFLICT_RE``. + """ + + def _run_with_real_redocly(self, tmp_path, monkeypatch, api_name): + from validation.engines.python_checks import bundling_checks + + monkeypatch.setenv("PATH", f"{_REDOCLY_BIN_DIR}{os.pathsep}{os.environ['PATH']}") + return bundling_checks.check_component_renaming_conflict( + tmp_path, _make_context(api_name) + ) + + def test_real_conflict_is_detected(self, tmp_path: Path, monkeypatch): + common_dir = tmp_path / "code" / "common" + common_dir.mkdir(parents=True) + (common_dir / "common.yaml").write_text( + "components:\n" + " schemas:\n" + " Foo:\n" + " type: object\n" + " properties:\n" + " bar:\n" + " type: string\n", + encoding="utf-8", + ) + _write_spec(tmp_path, "sample", _WITH_EXTERNAL_REF) + + findings = self._run_with_real_redocly(tmp_path, monkeypatch, "sample") + + assert len(findings) == 1 + assert findings[0]["engine_rule"] == "check-component-renaming-conflict" + assert "Foo" in findings[0]["message"] + + def test_real_proxy_with_matching_content_is_not_flagged( + self, tmp_path: Path, monkeypatch + ): + common_dir = tmp_path / "code" / "common" + common_dir.mkdir(parents=True) + (common_dir / "common.yaml").write_text( + "components:\n" " schemas:\n" " Foo:\n" " type: string\n", + encoding="utf-8", + ) + _write_spec(tmp_path, "sample", _WITH_EXTERNAL_REF) + + findings = self._run_with_real_redocly(tmp_path, monkeypatch, "sample") + + assert findings == [] diff --git a/validation/tests/test_rule_metadata_integrity.py b/validation/tests/test_rule_metadata_integrity.py index 060de333..23130092 100644 --- a/validation/tests/test_rule_metadata_integrity.py +++ b/validation/tests/test_rule_metadata_integrity.py @@ -88,7 +88,7 @@ def test_expected_rule_counts(self, all_rules): counts = {} for r in all_rules: counts[r.engine] = counts.get(r.engine, 0) + 1 - assert counts["python"] == 36 + assert counts["python"] == 37 assert counts["spectral"] == 88 assert counts["gherkin"] == 25 assert counts["yamllint"] == 13 @@ -353,8 +353,8 @@ def test_suggestions_are_exception_not_norm(self, all_rules): """ with_suggestions = [r.id for r in all_rules if r.suggestion is not None] with_overrides = [r.id for r in all_rules if r.message_override is not None] - assert len(with_suggestions) == 26, ( - f"Expected 26 explicit suggestions (update test if adding " + assert len(with_suggestions) == 27, ( + f"Expected 27 explicit suggestions (update test if adding " f"suggestions): {with_suggestions}" ) assert len(with_overrides) == 0, ( From e4c98bd1656f00439141bdda95b44ea268609a44 Mon Sep 17 00:00:00 2001 From: Herbert Damker <52109189+hdamker@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:09:44 +0200 Subject: [PATCH 2/3] fix(validation): strip ANSI codes before matching P-040 conflict regex GitHub Actions' runner made redocly force-color its bundle output, and a reset code landing between the location token and "at" broke _CONFLICT_RE's \s+, so the check silently found nothing there. No CLI flag exists to suppress this (checked --help and the CLI source); the underlying color library's own env-var detection proved inconsistent in local testing, so strip ANSI in our own code instead. Adds a regression test using the exact colored output captured from CI. --- .../engines/python_checks/bundling_checks.py | 3 +- .../tests/test_python_checks_bundling.py | 44 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/validation/engines/python_checks/bundling_checks.py b/validation/engines/python_checks/bundling_checks.py index 5848da09..1b160a8f 100644 --- a/validation/engines/python_checks/bundling_checks.py +++ b/validation/engines/python_checks/bundling_checks.py @@ -23,6 +23,7 @@ _ENGINE_RULE = "check-component-renaming-conflict" _EXECUTION_ERROR_RULE = "component-renaming-conflict-execution-error" _EXTERNAL_REF_RE = re.compile(r'\$ref\s*:\s*["\']?\.\.') +_ANSI_ESCAPE_RE = re.compile(r'\x1b\[[0-9;]*[A-Za-z]') _CONFLICT_RE = re.compile( r'\[\d+\]\s+\S+:\d+:\d+\s+at\s+\S+\s*\n+' r"Two schemas are referenced with the same name but different content\. " @@ -80,7 +81,7 @@ def check_component_renaming_conflict( if result.returncode == 0: return [] - output = result.stdout + result.stderr + output = _ANSI_ESCAPE_RE.sub("", result.stdout + result.stderr) matches = list(_CONFLICT_RE.finditer(output)) if not matches: return [] diff --git a/validation/tests/test_python_checks_bundling.py b/validation/tests/test_python_checks_bundling.py index 4f696b2b..a1c68b31 100644 --- a/validation/tests/test_python_checks_bundling.py +++ b/validation/tests/test_python_checks_bundling.py @@ -138,6 +138,27 @@ def _write_spec(tmp_path: Path, name: str, content: str) -> Path: ❌ Errors encountered while bundling code/API_definitions/sample.yaml: bundle not created (use --force to ignore errors). """ +# Captured verbatim from a GitHub Actions run: redocly forces ANSI color codes +# in its output there (piped or not), and a reset code lands between the +# location token and "at", which broke an earlier, color-naive _CONFLICT_RE. +_CONFLICT_STDERR_WITH_ANSI = ( + "\x1b[90mbundling code/API_definitions/sample.yaml...\n\x1b[39m[1] " + "\x1b[41mcode/API_definitions/sample.yaml:21:17\x1b[49m " + "\x1b[90mat #/paths/~1ping/get/responses/500/content/application~1json/schema\x1b[39m\n\n" + "Two schemas are referenced with the same name but different content. " + "Renamed Foo to Foo-2.\n\n" + "\x1b[90m19 |\x1b[39m \x1b[90mapplication/json:\x1b[39m\n" + "\x1b[90m20 |\x1b[39m \x1b[90mschema:\x1b[39m\n" + "\x1b[90m21 |\x1b[39m " + "\x1b[31m$ref: '../common/common.yaml#/components/schemas/Foo'\x1b[39m\n" + "\x1b[90m22 |\x1b[39m \x1b[90mcomponents:\x1b[39m\n" + "\x1b[90m23 |\x1b[39m \x1b[90mschemas:\x1b[39m\n\n" + "Error was generated by the \x1b[34mbundler\x1b[39m rule.\n\n\n" + "\x1b[31m❌ Errors encountered while bundling " + "\x1b[34mcode/API_definitions/sample.yaml\x1b[31m: bundle not created " + "(use --force to ignore errors).\n\x1b[39m" +) + class TestCheckComponentRenamingConflict: def test_no_api_context_returns_no_findings(self, tmp_path: Path, monkeypatch): @@ -226,6 +247,29 @@ def test_single_conflict_produces_one_finding(self, tmp_path: Path, monkeypatch) assert "Foo-2" in finding["message"] assert "isn't yours to rename" in finding["message"] + def test_conflict_with_ansi_color_codes_is_still_detected( + self, tmp_path: Path, monkeypatch + ): + from validation.engines.python_checks import bundling_checks + + _write_spec(tmp_path, "sample", _WITH_EXTERNAL_REF) + monkeypatch.setattr( + bundling_checks.subprocess, + "run", + Mock( + return_value=subprocess.CompletedProcess( + [], 1, stdout="", stderr=_CONFLICT_STDERR_WITH_ANSI + ) + ), + ) + + findings = bundling_checks.check_component_renaming_conflict( + tmp_path, _make_context("sample") + ) + + assert len(findings) == 1 + assert "Foo" in findings[0]["message"] + def test_multiple_conflicts_produce_multiple_findings( self, tmp_path: Path, monkeypatch ): From d3a6e127b0be4fceaf8e3c97593e35f192f865c3 Mon Sep 17 00:00:00 2001 From: Herbert Damker <52109189+hdamker@users.noreply.github.com> Date: Thu, 6 Aug 2026 18:14:42 +0200 Subject: [PATCH 3/3] fix(validation): use NO_COLOR instead of stripping ANSI for P-040 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A real-runner reconstruction showed NO_COLOR alone (no ANSI-stripping) satisfies the unmodified conflict regex against redocly's colored output — the same mechanism already verified for 047's illegible bundling-failure annotation. Simplifies the check to one suppression path instead of two. --- .../engines/python_checks/bundling_checks.py | 9 +++- .../tests/test_python_checks_bundling.py | 44 ++++--------------- 2 files changed, 15 insertions(+), 38 deletions(-) diff --git a/validation/engines/python_checks/bundling_checks.py b/validation/engines/python_checks/bundling_checks.py index 1b160a8f..935fa104 100644 --- a/validation/engines/python_checks/bundling_checks.py +++ b/validation/engines/python_checks/bundling_checks.py @@ -10,6 +10,7 @@ from __future__ import annotations +import os import re import subprocess import tempfile @@ -23,7 +24,6 @@ _ENGINE_RULE = "check-component-renaming-conflict" _EXECUTION_ERROR_RULE = "component-renaming-conflict-execution-error" _EXTERNAL_REF_RE = re.compile(r'\$ref\s*:\s*["\']?\.\.') -_ANSI_ESCAPE_RE = re.compile(r'\x1b\[[0-9;]*[A-Za-z]') _CONFLICT_RE = re.compile( r'\[\d+\]\s+\S+:\d+:\d+\s+at\s+\S+\s*\n+' r"Two schemas are referenced with the same name but different content\. " @@ -65,6 +65,11 @@ def check_component_renaming_conflict( text=True, timeout=_TIMEOUT_SECONDS, check=False, + # GitHub Actions sets GITHUB_ACTIONS unconditionally, which + # redocly's color library treats as reason enough to force + # ANSI color into captured (non-TTY) output; NO_COLOR + # suppresses it. Verified against a real Actions run. + env={**os.environ, "NO_COLOR": "1"}, ) except (FileNotFoundError, OSError, subprocess.TimeoutExpired) as exc: return [ @@ -81,7 +86,7 @@ def check_component_renaming_conflict( if result.returncode == 0: return [] - output = _ANSI_ESCAPE_RE.sub("", result.stdout + result.stderr) + output = result.stdout + result.stderr matches = list(_CONFLICT_RE.finditer(output)) if not matches: return [] diff --git a/validation/tests/test_python_checks_bundling.py b/validation/tests/test_python_checks_bundling.py index a1c68b31..ad852e89 100644 --- a/validation/tests/test_python_checks_bundling.py +++ b/validation/tests/test_python_checks_bundling.py @@ -138,28 +138,6 @@ def _write_spec(tmp_path: Path, name: str, content: str) -> Path: ❌ Errors encountered while bundling code/API_definitions/sample.yaml: bundle not created (use --force to ignore errors). """ -# Captured verbatim from a GitHub Actions run: redocly forces ANSI color codes -# in its output there (piped or not), and a reset code lands between the -# location token and "at", which broke an earlier, color-naive _CONFLICT_RE. -_CONFLICT_STDERR_WITH_ANSI = ( - "\x1b[90mbundling code/API_definitions/sample.yaml...\n\x1b[39m[1] " - "\x1b[41mcode/API_definitions/sample.yaml:21:17\x1b[49m " - "\x1b[90mat #/paths/~1ping/get/responses/500/content/application~1json/schema\x1b[39m\n\n" - "Two schemas are referenced with the same name but different content. " - "Renamed Foo to Foo-2.\n\n" - "\x1b[90m19 |\x1b[39m \x1b[90mapplication/json:\x1b[39m\n" - "\x1b[90m20 |\x1b[39m \x1b[90mschema:\x1b[39m\n" - "\x1b[90m21 |\x1b[39m " - "\x1b[31m$ref: '../common/common.yaml#/components/schemas/Foo'\x1b[39m\n" - "\x1b[90m22 |\x1b[39m \x1b[90mcomponents:\x1b[39m\n" - "\x1b[90m23 |\x1b[39m \x1b[90mschemas:\x1b[39m\n\n" - "Error was generated by the \x1b[34mbundler\x1b[39m rule.\n\n\n" - "\x1b[31m❌ Errors encountered while bundling " - "\x1b[34mcode/API_definitions/sample.yaml\x1b[31m: bundle not created " - "(use --force to ignore errors).\n\x1b[39m" -) - - class TestCheckComponentRenamingConflict: def test_no_api_context_returns_no_findings(self, tmp_path: Path, monkeypatch): from validation.engines.python_checks import bundling_checks @@ -247,28 +225,22 @@ def test_single_conflict_produces_one_finding(self, tmp_path: Path, monkeypatch) assert "Foo-2" in finding["message"] assert "isn't yours to rename" in finding["message"] - def test_conflict_with_ansi_color_codes_is_still_detected( - self, tmp_path: Path, monkeypatch - ): + def test_subprocess_env_disables_color(self, tmp_path: Path, monkeypatch): from validation.engines.python_checks import bundling_checks _write_spec(tmp_path, "sample", _WITH_EXTERNAL_REF) - monkeypatch.setattr( - bundling_checks.subprocess, - "run", - Mock( - return_value=subprocess.CompletedProcess( - [], 1, stdout="", stderr=_CONFLICT_STDERR_WITH_ANSI - ) - ), + run = Mock( + return_value=subprocess.CompletedProcess([], 0, stdout="", stderr="") ) + monkeypatch.setattr(bundling_checks.subprocess, "run", run) - findings = bundling_checks.check_component_renaming_conflict( + bundling_checks.check_component_renaming_conflict( tmp_path, _make_context("sample") ) - assert len(findings) == 1 - assert "Foo" in findings[0]["message"] + env = run.call_args.kwargs["env"] + assert env["NO_COLOR"] == "1" + assert env.get("PATH") == os.environ.get("PATH") def test_multiple_conflicts_produce_multiple_findings( self, tmp_path: Path, monkeypatch