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
16 changes: 16 additions & 0 deletions .github/workflows/validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ jobs:
config
linting/config
validation
release_automation
shared-actions
tooling_lib
path: .tooling
Expand Down Expand Up @@ -262,6 +263,18 @@ jobs:
--base-ref "${{ github.base_ref }}" \
--workspace "${GITHUB_WORKSPACE}"

# ── Step 6c: Resolve active release state for P-033 ───────────
#
# Reuse the release automation state resolver and pass its compact
# outputs into the offline Python orchestrator. P-033 blocks
# release-plan.yaml edits while a release snapshot is active.
- name: Resolve active release state
id: derive-release-state
if: >-
github.event_name == 'pull_request'
&& steps.detect-changes.outputs.release_plan_changed == 'true'
uses: ./.tooling/shared-actions/derive-release-state

# ── Step 7: Run validation (shared action) ─────────────────────
#
# The run-validation action installs dependencies, runs the Python
Expand All @@ -281,6 +294,9 @@ jobs:
commonalities_tag_exists: ${{ steps.detect-deps.outputs.commonalities_tag_exists || '' }}
icm_tag_exists: ${{ steps.detect-deps.outputs.icm_tag_exists || '' }}
non_release_plan_files_changed: ${{ steps.detect-changes.outputs.non_release_plan_files_changed || '[]' }}
active_release_state: ${{ steps.derive-release-state.outputs.state || '' }}
active_release_snapshot_branch: ${{ steps.derive-release-state.outputs.snapshot_branch || '' }}
active_release_issue_number: ${{ steps.derive-release-state.outputs.release_issue_number || '' }}
tooling_ref: ${{ steps.resolve-ref.outputs.tooling_checkout_ref }}
config_path: ${{ steps.resolve-config.outputs.config_path }}

Expand Down
21 changes: 21 additions & 0 deletions shared-actions/run-validation/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,24 @@ inputs:
the only changed file. Consumed by P-022 (exclusivity check).
required: false
default: '[]'
active_release_state:
description: >
Optional workflow-resolved release state from derive-release-state.
Consumed by P-033.
required: false
default: ''
active_release_snapshot_branch:
description: >
Optional active release-snapshot branch from derive-release-state.
Consumed by P-033.
required: false
default: ''
active_release_issue_number:
description: >
Optional workflow-owned Release Issue number from derive-release-state.
Consumed by P-033.
required: false
default: ''
tooling_ref:
description: 'Tooling ref used for this run (for diagnostics)'
required: false
Expand Down Expand Up @@ -217,6 +235,9 @@ runs:
VALIDATION_COMMONALITIES_TAG_EXISTS: ${{ inputs.commonalities_tag_exists }}
VALIDATION_ICM_TAG_EXISTS: ${{ inputs.icm_tag_exists }}
VALIDATION_NON_RELEASE_PLAN_FILES_CHANGED: ${{ inputs.non_release_plan_files_changed }}
VALIDATION_ACTIVE_RELEASE_STATE: ${{ inputs.active_release_state }}
VALIDATION_ACTIVE_RELEASE_SNAPSHOT_BRANCH: ${{ inputs.active_release_snapshot_branch }}
VALIDATION_ACTIVE_RELEASE_ISSUE_NUMBER: ${{ inputs.active_release_issue_number }}
VALIDATION_WORKFLOW_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
VALIDATION_TOOLING_REF: ${{ inputs.tooling_ref }}
VALIDATION_COMMIT_SHA: ${{ github.event.pull_request.head.sha || github.sha }}
Expand Down
1 change: 1 addition & 0 deletions validation/context/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
PROFILE_ADVISORY,
PROFILE_STANDARD,
PROFILE_STRICT,
ActiveReleaseState,
ApiContext,
ValidationContext,
build_validation_context,
Expand Down
23 changes: 22 additions & 1 deletion validation/context/context_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import dataclasses
import logging
import re
from dataclasses import dataclass
from dataclasses import dataclass, field
from pathlib import Path
from typing import Optional, Tuple

Expand Down Expand Up @@ -89,6 +89,15 @@ class ApiContext:
spec_file: str


@dataclass(frozen=True)
class ActiveReleaseState:
"""Workflow-resolved active release state consumed by P-033."""

state: str = ""
snapshot_branch: str = ""
release_issue_number: Optional[int] = None


@dataclass(frozen=True)
class ValidationContext:
"""Unified validation context. All fields always present.
Expand Down Expand Up @@ -150,6 +159,10 @@ class ValidationContext:
# warnings on the Release Review PR instead of a P-031 false positive.
fallback_canonical_path: Optional[str] = None

# active_release_state: compact workflow-resolved state for P-033.
# Populated from shared-actions/derive-release-state outputs.
active_release_state: ActiveReleaseState = field(default_factory=ActiveReleaseState)

def to_dict(self) -> dict:
"""Serialize to dict with all keys present.

Expand Down Expand Up @@ -298,6 +311,9 @@ def build_validation_context(
icm_tag_exists: Optional[bool] = None,
non_release_plan_files_changed: Tuple[str, ...] = (),
fallback_canonical_path: Optional[str] = None,
active_release_state: str = "",
active_release_snapshot_branch: str = "",
active_release_issue_number: Optional[int] = None,
) -> ValidationContext:
"""Assemble the unified validation context.

Expand Down Expand Up @@ -389,4 +405,9 @@ def build_validation_context(
icm_tag_exists=icm_tag_exists,
non_release_plan_files_changed=non_release_plan_files_changed,
fallback_canonical_path=fallback_canonical_path,
active_release_state=ActiveReleaseState(
state=active_release_state,
snapshot_branch=active_release_snapshot_branch,
release_issue_number=active_release_issue_number,
),
)
4 changes: 4 additions & 0 deletions validation/engines/python_checks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
from .release_plan_checks import (
check_declared_dependency_tags_exist,
check_orphan_api_definitions,
check_release_plan_active_release_state,
check_release_plan_api_names_unique,
check_release_plan_exclusivity,
check_release_plan_semantics,
)
Expand Down Expand Up @@ -67,12 +69,14 @@
# --- 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),
CheckDescriptor("check-release-plan-api-names-unique", CheckScope.REPO, check_release_plan_api_names_unique),
CheckDescriptor("check-readme-placeholder-removal", CheckScope.REPO, check_readme_placeholder_removal),
CheckDescriptor("check-api-readiness-checklist-removal", CheckScope.REPO, check_api_readiness_checklist_removal),
CheckDescriptor("check-release-review-file-restriction", CheckScope.REPO, check_release_review_file_restriction),
CheckDescriptor("check-orphan-api-definitions", CheckScope.REPO, check_orphan_api_definitions),
CheckDescriptor("check-common-cache-sync", CheckScope.REPO, check_common_cache_sync),
CheckDescriptor("check-release-plan-exclusivity", CheckScope.REPO, check_release_plan_exclusivity),
CheckDescriptor("check-release-plan-active-release-state", CheckScope.REPO, check_release_plan_active_release_state),
CheckDescriptor("check-declared-dependency-tags-exist", CheckScope.REPO, check_declared_dependency_tags_exist),
]

Expand Down
105 changes: 105 additions & 0 deletions validation/engines/python_checks/release_plan_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@

from __future__ import annotations

from collections import Counter
from pathlib import Path
from typing import List, Optional

from release_automation.scripts import config
from validation.context import ValidationContext
from validation.context.release_plan_parser import is_valid_release_tag

Expand All @@ -23,6 +25,13 @@

_RELEASE_PLAN_PATH = "release-plan.yaml"

_ACTIVE_RELEASE_STATES = frozenset(
{
config.STATE_SNAPSHOT_ACTIVE,
config.STATE_DRAFT_READY,
}
)


# ---------------------------------------------------------------------------
# Semantic check functions (ported from validate-release-plan.py)
Expand Down Expand Up @@ -277,6 +286,58 @@ def check_release_plan_semantics(
return findings


# ---------------------------------------------------------------------------
# P-034: check-release-plan-api-names-unique
# ---------------------------------------------------------------------------


def check_release_plan_api_names_unique(
repo_path: Path, context: ValidationContext
) -> List[dict]:
"""Detect duplicate ``apis[].api_name`` entries in release-plan.yaml."""
plan_path = repo_path / _RELEASE_PLAN_PATH
release_plan = load_yaml_safe(plan_path)
if release_plan is None:
return []

apis = release_plan.get("apis", [])
if not isinstance(apis, list):
return []

names = [
api.get("api_name")
for api in apis
if isinstance(api, dict) and isinstance(api.get("api_name"), str)
]
duplicates = sorted(
(name, count) for name, count in Counter(names).items() if count > 1
)

findings: List[dict] = []
for api_name, count in duplicates:
message = (
f"release-plan.yaml contains {count} entries for api_name "
f"'{api_name}'. Keep exactly one entry for each api_name."
)
if context.release_plan_changed is False:
message = (
"Pre-existing release-plan condition: "
f"{message} Submit the fix in a dedicated release-plan PR."
)
findings.append(
make_finding(
engine_rule="check-release-plan-api-names-unique",
level="error",
message=message,
path=_RELEASE_PLAN_PATH,
line=1,
api_name=api_name,
)
)

return findings


# ---------------------------------------------------------------------------
# P-019 (NEW-003): Orphan API definitions
# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -379,6 +440,50 @@ def check_release_plan_exclusivity(
]


# ---------------------------------------------------------------------------
# P-033: check-release-plan-active-release-state
# ---------------------------------------------------------------------------


def check_release_plan_active_release_state(
repo_path: Path, context: ValidationContext
) -> List[dict]:
"""Block release-plan.yaml edits while a release snapshot is active."""
if context.trigger_type != "pr" or context.release_plan_changed is not True:
return []

active_release = context.active_release_state
active_state = active_release.state
snapshot_branch = active_release.snapshot_branch
has_active_state = active_state in _ACTIVE_RELEASE_STATES

if not snapshot_branch and not has_active_state:
return []

details: list[str] = []
if snapshot_branch:
details.append(f"active snapshot branch: {snapshot_branch}")
if has_active_state:
state_detail = f"release state is '{active_state}'"
if active_release.release_issue_number is not None:
state_detail += f" (Release Issue #{active_release.release_issue_number})"
details.append(state_detail)

return [
make_finding(
engine_rule="check-release-plan-active-release-state",
level="error",
message=(
"release-plan.yaml cannot be changed while an active release "
f"exists ({'; '.join(details)}). Finish, discard, or publish "
"the active snapshot before changing release-plan.yaml."
),
path=_RELEASE_PLAN_PATH,
line=1,
)
]


# ---------------------------------------------------------------------------
# P-023: check-declared-dependency-tags-exist
# ---------------------------------------------------------------------------
Expand Down
11 changes: 11 additions & 0 deletions validation/orchestrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ class OrchestratorArgs:
# empty everywhere else, keeping local/main/working-branch runs offline.
fallback_canonical_path: str

# Optional workflow-resolved active release state for P-033.
active_release_state: str
active_release_snapshot_branch: str
active_release_issue_number: Optional[int]

repo_name: str # e.g. "camaraproject/QualityOnDemand"
repo_owner: str # e.g. "camaraproject"
event_name: str # e.g. "pull_request", "workflow_dispatch"
Expand Down Expand Up @@ -169,6 +174,9 @@ def parse_args() -> OrchestratorArgs:
output_dir=Path(_env("OUTPUT_DIR", "validation-output")),
config_path=_env("CONFIG_PATH"),
fallback_canonical_path=_env("FALLBACK_CANONICAL_PATH"),
active_release_state=_env("ACTIVE_RELEASE_STATE"),
active_release_snapshot_branch=_env("ACTIVE_RELEASE_SNAPSHOT_BRANCH"),
active_release_issue_number=_env_optional_int("ACTIVE_RELEASE_ISSUE_NUMBER"),
repo_name=_env("REPO_NAME"),
repo_owner=_env("REPO_OWNER"),
event_name=_env("EVENT_NAME"),
Expand Down Expand Up @@ -539,6 +547,9 @@ def main() -> int:
icm_tag_exists=args.icm_tag_exists,
non_release_plan_files_changed=args.non_release_plan_files_changed,
fallback_canonical_path=args.fallback_canonical_path or None,
active_release_state=args.active_release_state,
active_release_snapshot_branch=args.active_release_snapshot_branch,
active_release_issue_number=args.active_release_issue_number,
)
logger.info(
"Context: branch=%s trigger=%s profile=%s release_review=%s apis=%d",
Expand Down
10 changes: 5 additions & 5 deletions validation/postfilter/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,13 +273,13 @@ def run_post_filter(
# and on-disk content are stale relative to the declared tag.
# Running version-context-dependent rules against that stale
# content produces misleading findings (DEC-029 exclusivity
# principle). Only rules that explicitly gate on
# release_plan_changed: true survive — those are release-plan
# validation rules (P-009, P-022, P-023) which check the
# release-plan.yaml content itself, not the consumption side.
# principle). Only rules explicitly marked safe survive; the
# flag is separate from release_plan_changed applicability so
# always-valid release-plan consistency checks can still run on
# unrelated PRs.
if (
context.release_plan_check_only
and rule.applicability.get("release_plan_changed") is not True
and not rule.release_plan_check_only_safe
):
continue

Expand Down
9 changes: 9 additions & 0 deletions validation/postfilter/metadata_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ class RuleMetadata:
Rendered as a details link in the workflow summary and carried
into diagnostics; deliberately omitted from annotations.
``None`` means no documentation link (selective by design).
release_plan_check_only_safe: Whether this rule is safe to keep when
``release_plan_check_only`` skips context-dependent engines after
a Commonalities dependency declaration advanced in release-plan.yaml.
applicability: Condition dict — omitted fields are unconstrained.
conditional_level: Severity specification, or ``None`` to preserve
engine-reported severity (identity mapping).
Expand All @@ -102,6 +105,7 @@ class RuleMetadata:
short_title: Optional[str] = None
suppress_schema_paths: Tuple[str, ...] = ()
documentation_url: Optional[str] = None
release_plan_check_only_safe: bool = False


# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -165,6 +169,10 @@ def parse_rule_metadata(raw: dict) -> RuleMetadata:
entry for entry in raw_suppress if isinstance(entry, str) and entry
)

raw_check_only_safe = raw.get("release_plan_check_only_safe", False)
if not isinstance(raw_check_only_safe, bool):
raise ValueError("release_plan_check_only_safe must be a boolean")

return RuleMetadata(
id=raw["id"],
name=raw.get("name", raw["engine_rule"]),
Expand All @@ -177,6 +185,7 @@ def parse_rule_metadata(raw: dict) -> RuleMetadata:
short_title=raw.get("short_title"),
suppress_schema_paths=suppress_schema_paths,
documentation_url=raw.get("documentation_url"),
release_plan_check_only_safe=raw_check_only_safe,
)


Expand Down
Loading