Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -823,9 +823,12 @@ repos:
pass_filenames: true
require_serial: true
files: ^.*\.py$
exclude: ^(scripts/validation/validate-contracts\.py|scripts/validation/validate-string-versions\.py|scripts/validation/check_stub_implementations\.py|scripts/validation/validate-no-manual-yaml\.py|scripts/validation/validate_pr_deploy_required\.py|scripts/compute_contract_fingerprint\.py|scripts/lint_contract\.py|scripts/ci/canonical_handler_shape\.py|scripts/ci/skip_count_ratchet\.py|src/omnibase_core/utils/safe_yaml_loader\.py|src/omnibase_core/utils/util_safe_yaml_loader\.py|src/omnibase_core/validation/contracts\.py|src/omnibase_core/validation/contract_validator\.py|src/omnibase_core/validation/validator_base\.py|src/omnibase_core/validation/validator_any_type\.py|src/omnibase_core/validation/validator_contract_linter\.py|src/omnibase_core/validation/validator_requirements_consumer\.py|src/omnibase_core/validation/validator_rollup_coverage\.py|src/omnibase_core/validation/validator_standalone_workflow_registry\.py|src/omnibase_core/validation/validator_pull_request_workflow_ratchet\.py|src/omnibase_core/validation/validator_runtime_profiles\.py|src/omnibase_core/discovery/mixin_discovery\.py|src/omnibase_core/cli/cli_demo\.py|tests/fixtures/validation/|src/omnibase_core/validation/scripts/validate_string_versions\.py|src/omnibase_core/validators/contract_config_compliance\.py|src/omnibase_core/validators/gitignore_baseline\.py|src/omnibase_core/validators/skill_dispatch_receipt_mode\.py|src/omnibase_core/validators/backend_secret_discipline\.py|src/omnibase_core/validators/operation_match_requires_operation\.py|src/omnibase_core/validators/dispatched_contract_operation\.py|src/omnibase_core/validators/command_category_requires_event_model\.py|src/omnibase_core/validators/generated_node_model_class_exists\.py|src/omnibase_core/validators/projection_exposure_drift\.py|src/omnibase_core/validators/no_noncanonical_lifecycle_classes\.py|\.github/actions/required-check-skip-guard/validate_no_required_check_skip_vectors\.py|\.github/actions/required-check-skip-guard/_workflow_model\.py)
exclude: ^(scripts/validation/validate-contracts\.py|scripts/validation/validate-string-versions\.py|scripts/validation/check_stub_implementations\.py|scripts/validation/validate-no-manual-yaml\.py|scripts/validation/validate_pr_deploy_required\.py|scripts/compute_contract_fingerprint\.py|scripts/lint_contract\.py|scripts/ci/canonical_handler_shape\.py|scripts/ci/skip_count_ratchet\.py|src/omnibase_core/utils/safe_yaml_loader\.py|src/omnibase_core/utils/util_safe_yaml_loader\.py|src/omnibase_core/validation/contracts\.py|src/omnibase_core/validation/contract_validator\.py|src/omnibase_core/validation/validator_base\.py|src/omnibase_core/validation/validator_any_type\.py|src/omnibase_core/validation/validator_contract_linter\.py|src/omnibase_core/validation/validator_requirements_consumer\.py|src/omnibase_core/validation/validator_rollup_coverage\.py|src/omnibase_core/validation/validator_standalone_workflow_registry\.py|src/omnibase_core/validation/validator_pull_request_workflow_ratchet\.py|src/omnibase_core/validation/validator_runtime_profiles\.py|src/omnibase_core/discovery/mixin_discovery\.py|src/omnibase_core/cli/cli_demo\.py|tests/fixtures/validation/|src/omnibase_core/validation/scripts/validate_string_versions\.py|src/omnibase_core/validators/contract_config_compliance\.py|src/omnibase_core/validators/gitignore_baseline\.py|src/omnibase_core/validators/canonical_file_shape\.py|src/omnibase_core/validators/skill_dispatch_receipt_mode\.py|src/omnibase_core/validators/backend_secret_discipline\.py|src/omnibase_core/validators/operation_match_requires_operation\.py|src/omnibase_core/validators/dispatched_contract_operation\.py|src/omnibase_core/validators/command_category_requires_event_model\.py|src/omnibase_core/validators/generated_node_model_class_exists\.py|src/omnibase_core/validators/projection_exposure_drift\.py|src/omnibase_core/validators/no_noncanonical_lifecycle_classes\.py|\.github/actions/required-check-skip-guard/validate_no_required_check_skip_vectors\.py|\.github/actions/required-check-skip-guard/_workflow_model\.py)
stages: [pre-commit]
# validate_string_versions.py: validation script that must load YAML to validate it (OMN-4402)
# canonical_file_shape.py (OMN-17427): reads the target repo's .pre-commit-config.yaml to find the
# --baseline path its check-direct-model-call hook declares; that is a third-party-shaped config,
# not an ONEX contract, so a Pydantic contract model does not apply.
# scripts/ci/skip_count_ratchet.py (OMN-18790): vendored from omnibase_infra
# (canonical source, OMN-18776) unmodified — it parses config/skip_count_baseline.yaml,
# a CI baseline file, not an ONEX contract, so Pydantic contract-model
Expand Down
51 changes: 45 additions & 6 deletions src/omnibase_core/validators/canonical_file_shape.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
``baseline-stale``
a baseline entry whose file was removed, moved or made canonical. The
entry must be deleted in the same change, so the baseline only shrinks.
The old path of a detected rename (below) is not stale.
``baseline-growth``
a baseline entry that the base revision's baseline did not have.
a baseline entry that the base revision's baseline did not have, except
the new path of a detected rename (below).
``skill-imperative``
a ``SKILL.md`` with more imperative-pattern lines than at the base
revision. The patterns are the omniclaude thin-shim gate's
Expand All @@ -45,6 +47,23 @@
enforces that its list only shrinks. A baseline-named file nothing declares is
still refused.

Renames. A baselined file may be renamed (``git diff -M`` from base to head,
default similarity, so a rename plus an edit counts). The renamed path inherits
its old path's entry from the BASE baseline. Two forms are accepted:

* swap (preferred): the same change removes the old entry and adds the new
path, so the baseline count does not grow and the baseline stays true;
* untouched: the change leaves the baseline alone. The old entry reads as
satisfied by the rename and the new path is covered. The old entry is then
stale in the next change, which must delete it, so the swap is the cleaner
form.

Nothing is inherited when no rename is detected (a delete plus an add), when
the old path was not in the base baseline, when the new path is not itself a
non-canonical code file, or when the swap keeps the old entry and adds the new
one (growth). Another new script in the same change is still refused. Only
renames count, never copies.

There is no allowlist, no suppression comment and no option that widens the
canonical locations. The only state is the shrink-only baseline file, default
``.onex_ratchets/canonical_file_shape_baseline.txt``.
Expand Down Expand Up @@ -389,8 +408,27 @@ def check(
noncanonical = noncanonical_code_files(repo, head)
noncanonical_set = set(noncanonical)

base_baseline_text = (
None
if base is None
else _decode(repo.read_blobs(base, [baseline_path])[baseline_path])
)
base_entries = set(parse_baseline(base_baseline_text))
all_changes = [] if base is None else repo.changed_paths(base, head)
# new path -> old path, for a detected rename of a baselined file to a path
# that is itself a non-canonical code file.
renames = {
path: old
for status, path, old in all_changes
if status == "R"
and old is not None
and old in base_entries
and path in noncanonical_set
}
renamed_from = set(renames.values())

for path in noncanonical:
if path not in baselined:
if path not in baselined and path not in renames:
findings.append(
ModelCanonicalFileShapeFinding(
path=path,
Expand All @@ -402,7 +440,7 @@ def check(
),
)
)
for path in sorted(baselined - noncanonical_set):
for path in sorted(baselined - noncanonical_set - renamed_from):
findings.append(
ModelCanonicalFileShapeFinding(
path=baseline_path,
Expand All @@ -414,9 +452,10 @@ def check(
if base is None:
return findings

base_baseline_text = _decode(repo.read_blobs(base, [baseline_path])[baseline_path])
if base_baseline_text is not None:
for path in sorted(baselined - set(parse_baseline(base_baseline_text))):
for path in sorted(baselined - base_entries):
if path in renames and renames[path] not in baselined:
continue
findings.append(
ModelCanonicalFileShapeFinding(
path=baseline_path,
Expand All @@ -425,7 +464,7 @@ def check(
)
)

changes = [c for c in repo.changed_paths(base, head) if c[0] != "D"]
changes = [c for c in all_changes if c[0] != "D"]
head_text = repo.read_blobs(head, [path for _, path, _ in changes])
base_text = repo.read_blobs(base, [old or path for _, path, old in changes])

Expand Down
89 changes: 89 additions & 0 deletions tests/unit/validators/test_canonical_file_shape.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,95 @@ def test_baseline_shrink_accepted(repo: Path) -> None:
assert _rules(repo) == []


BODY = "".join(f"line_{i} = {i}\n" for i in range(20))


def _baselined_tool(repo: Path) -> None:
"""Commit a larger baselined file so a rename survives an edit."""
_write(repo, "plugins/p/_bin/tool.sh", "#!/bin/sh\n" + BODY)
_write(
repo,
DEFAULT_BASELINE,
render_baseline(["scripts/old_tool.py", "plugins/p/_bin/tool.sh"]),
)
_git(repo, "add", "-A")
_git(repo, "commit", "-q", "-m", "tool")


def _rename(repo: Path, new: str, extra: str = "") -> None:
(repo / new).parent.mkdir(parents=True, exist_ok=True)
_git(repo, "mv", "plugins/p/_bin/tool.sh", new)
if extra:
with (repo / new).open("a", encoding="utf-8") as fh:
fh.write(extra)


NEW_TOOL = "plugins/p/_bin/renamed.sh"
SWAPPED = ["scripts/old_tool.py", NEW_TOOL]


def test_rename_swap_accepted(repo: Path) -> None:
_baselined_tool(repo)
_rename(repo, NEW_TOOL)
_write(repo, DEFAULT_BASELINE, render_baseline(SWAPPED))
_stage(repo)
assert _rules(repo) == []


def test_rename_baseline_untouched_accepted(repo: Path) -> None:
_baselined_tool(repo)
_rename(repo, NEW_TOOL)
_stage(repo)
assert _rules(repo) == []


def test_rename_with_edit_accepted(repo: Path) -> None:
_baselined_tool(repo)
_rename(repo, NEW_TOOL, "added = 1\n")
_write(repo, DEFAULT_BASELINE, render_baseline(SWAPPED))
_stage(repo)
assert _rules(repo) == []


def test_rename_of_unbaselined_file_refused(repo: Path) -> None:
_write(repo, "plugins/p/_bin/free.sh", "#!/bin/sh\n" + BODY)
_stage(repo)
_git(repo, "commit", "-q", "-m", "unbaselined")
_git(repo, "mv", "plugins/p/_bin/free.sh", NEW_TOOL)
_stage(repo)
assert "noncanonical-file" in _rules(repo)


def test_rename_plus_extra_script_refused(repo: Path) -> None:
_baselined_tool(repo)
_rename(repo, NEW_TOOL)
_write(repo, "scripts/extra.py", "x = 1\n")
_write(repo, DEFAULT_BASELINE, render_baseline(SWAPPED))
_stage(repo)
assert _rules(repo) == ["noncanonical-file"]


def test_swap_without_detected_rename_refused(repo: Path) -> None:
_baselined_tool(repo)
(repo / "plugins/p/_bin/tool.sh").unlink()
_write(repo, NEW_TOOL, "#!/bin/sh\necho unrelated\n")
_write(repo, DEFAULT_BASELINE, render_baseline(SWAPPED))
_stage(repo)
assert _rules(repo) == ["baseline-growth"]


def test_rename_keeping_old_entry_and_adding_new_refused(repo: Path) -> None:
_baselined_tool(repo)
_rename(repo, NEW_TOOL)
_write(
repo,
DEFAULT_BASELINE,
render_baseline([*SWAPPED, "plugins/p/_bin/tool.sh"]),
)
_stage(repo)
assert "baseline-growth" in _rules(repo)


def test_skill_imperative_growth_refused(repo: Path) -> None:
_write(
repo,
Expand Down
Loading