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
47 changes: 46 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,52 @@ jobs:
- name: Run HACS validation
if: steps.brands_probe.outputs.available == 'true'
id: hacs
continue-on-error: true
uses: hacs/action@main
with:
category: integration
ignore: topics issues

# Transient HACS regression guard (hacs/integration#5252): the action
# intermittently fetches the raw hacs.json / manifest.json as None from
# the GitHub contents API and reports "invalid 'hacs.json' file" and
# "integration_manifest ... Got None" on byte-identical, valid files.
# If the first run fails, re-run with exactly those two transient-prone
# validators isolated. If everything else passes, the failure was the
# known transient signature and is tolerated (job stays green with a
# warning). The isolation is NOT permanent: it only runs on failure, so a
# healthy HACS still exercises all checks. The real validity of both files
# is still enforced independently at the ci-success level: manifest.json by
# the hassfest job (this job needs: hassfest) and hacs.json by the test job
# (tests/test_hacs_validation.py). Any other failing check keeps this red.
- name: Re-run HACS validation (isolate transient checks)
if: steps.brands_probe.outputs.available == 'true' && steps.hacs.outcome == 'failure'
id: hacs_surgical
continue-on-error: true
uses: hacs/action@main
with:
category: integration
ignore: topics issues hacsjson integration_manifest

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use a documented HACS check name

When the upstream #5252 failure occurs, the first run reports both hacsjson and integration_manifest, but I checked the HACS Action docs for with.ignore: they document a space-separated list of ignorable checks and list archived, brands, description, hacsjson, images, information, issues, and topics, not integration_manifest. Because this retry relies on ignoring integration_manifest, the retry will still run that failing validator in the transient scenario and RETRY remains failure, so the classifier exits 1 instead of making CI resilient.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the careful read. I verified this against the HACS validation source (not the docs page), and integration_manifest is in fact a valid, effective ignore token, so the surgical retry does isolate it.

Chain of evidence (hacs/integration main, which is exactly what hacs/action@main runs via docker://ghcr.io/hacs/action:main):

  • The ignore filter keys off the check slug, and the slug is the check module basename:
    custom_components/hacs/validate/base.py:
    @property
    def slug(self) -> str:
        return self.__class__.__module__.rsplit(".", maxsplit=1)[-1]
  • The integration-manifest check lives in custom_components/hacs/validate/integration_manifest.py, so its slug is exactly integration_manifest.
  • The manager applies the ignore list uniformly over every validator by slug:
    custom_components/hacs/validate/manager.py:
    and validator.slug not in os.getenv("INPUT_IGNORE", "").split(" ")
  • GitHub Actions maps with.ignore to INPUT_IGNORE, so ignore: topics issues hacsjson integration_manifest yields INPUT_IGNORE="topics issues hacsjson integration_manifest", whose split contains integration_manifest, and the not in check excludes that validator from execution.

The list under https://hacs.xyz/docs/publish/action/ is an incomplete enumeration (it also omits license, which is likewise a real check module validate/license.py). The authoritative set of ignorable slugs is the set of module basenames under validate/, and integration_manifest is one of them. The #5252 error label "integration_manifest ... Got None" is that very slug.

So in the transient scenario the retry (ignore: ... hacsjson integration_manifest) skips both transient-prone validators; if only those two failed, hacs_surgical passes and the classifier reports transient (green with a warning). A failure on any other check keeps the retry red and the classifier exits 1. No change needed here.


- name: Classify HACS result
if: steps.brands_probe.outputs.available == 'true'
id: hacs_verdict
env:
FIRST: ${{ steps.hacs.outcome }}
RETRY: ${{ steps.hacs_surgical.outcome }}
run: |
set -u
if [ "$FIRST" = "success" ]; then
echo "status=passed" >> "$GITHUB_OUTPUT"
elif [ "$RETRY" = "success" ]; then
echo "::warning ::HACS validation failed only on the transient-prone checks (hacsjson, integration_manifest) and passed once they were isolated; treating as the known transient upstream regression (hacs/integration#5252). File validity is still enforced by the hassfest and test jobs."
echo "status=transient" >> "$GITHUB_OUTPUT"
Comment on lines +171 to +173

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require the exact HACS transient signature

When the first HACS run fails because of a real hacsjson or integration_manifest problem that hassfest/local tests do not mirror (for example a HACS-only manifest or hacs.json requirement), the fallback reruns with both validators ignored and this branch marks the job successful solely because the remaining checks pass. That turns genuine HACS metadata regressions into green CI, so the classifier should inspect the original failure signature/logs or otherwise reproduce all ignored validators locally before setting status=transient.

Useful? React with 👍 / 👎.

else
echo "::error ::HACS validation failed on a check beyond the known transient signature; treating as a real defect."
echo "status=failed" >> "$GITHUB_OUTPUT"
exit 1
fi

- name: Write HACS summary
if: always()
run: |
Expand All @@ -144,10 +185,14 @@ jobs:
echo ""
if [ "${{ steps.brands_probe.outputs.available }}" != "true" ]; then
echo "⚠️ **Skipped** - brands.home-assistant.io unavailable"
elif [ "${{ steps.hacs.outcome }}" = "success" ]; then
elif [ "${{ steps.hacs_verdict.outputs.status }}" = "passed" ]; then
echo "✅ **HACS validation passed**"
echo ""
echo "The integration meets HACS requirements."
elif [ "${{ steps.hacs_verdict.outputs.status }}" = "transient" ]; then
echo "⚠️ **HACS validation passed after isolating transient checks** - tolerated upstream regression (hacs/integration#5252)."
echo ""
echo "Only hacsjson/integration_manifest failed; file validity is enforced by the hassfest and test jobs."
else
echo "❌ **HACS validation failed**"
echo ""
Expand Down
24 changes: 17 additions & 7 deletions .github/workflows/release-stamp.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,25 +66,35 @@ jobs:
python-version: "3.13"

- name: Validate published tag
# Early, human-readable abort. The canonical regex (SSOT) lives in
# script/stamp_version.py (VERSION_RE); this guard mirrors it byte-for-byte
# (locked by tests/test_stamp_version.py). The shape is deliberately PEP
# Early, human-readable abort. The published git tag may carry a leading
# `v` (BSkando tags `vX.Y.Z`) or not (jleinenbach tags `X.Y.Z[.N]`) -- the
# shared release tooling must honour both fork conventions. The code
# literals, however, are ALWAYS PEP 440 versions with no `v`, so we strip
# an optional single leading `v` here and validate/stamp that VERSION.
# The canonical regex (SSOT) lives in script/stamp_version.py (VERSION_RE);
# this guard mirrors it byte-for-byte (locked by tests/test_stamp_version.py)
# and is applied to the stripped VERSION. The shape is deliberately PEP
# 440-broad (bN/aN betas, four-segment .N) so hand-picked maintenance tags
# stamp cleanly. semantic-release cannot *propose* for those lines (see
# release.yml Workflow A, which opens a hand-tag draft there instead); this
# step stamps the exact published tag regardless. Keep both regexes in sync.
run: |
set -euo pipefail
if ! printf '%s' "$TAG_NAME" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?([ab][0-9]+)?$'; then
echo "::error::tag_name '$TAG_NAME' violates tag_format='{version}'; expected X.Y.Z[.N][bN], no 'v' prefix"
# Strip at most one leading `v` (POSIX ${VAR#v}); a no-op for v-less
# tags. VERSION is the v-less value written into the code literals; the
# full TAG_NAME (with any `v`) still addresses the git tag / release asset.
VERSION="${TAG_NAME#v}"
if ! printf '%s' "$VERSION" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?([ab][0-9]+)?$'; then
echo "::error::tag_name '$TAG_NAME' (version '$VERSION') violates tag_format; expected X.Y.Z[.N][bN] with an optional leading 'v'"
exit 1
fi
echo "validated tag=$TAG_NAME"
echo "validated tag=$TAG_NAME version=$VERSION"
echo "VERSION=$VERSION" >> "$GITHUB_ENV"

- name: Stamp version into the three literals
run: |
set -euo pipefail
python script/stamp_version.py --version "$TAG_NAME"
python script/stamp_version.py --version "$VERSION"

- name: Commit the stamp
run: |
Expand Down
15 changes: 10 additions & 5 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,18 @@ jobs:
# (safe). Real PSR errors still surface loudly in the dry-run preview
# step above (`--noop version`), which is the diagnostic path.
REAL_TIP="$(git describe --tags --abbrev=0 HEAD 2>/dev/null || true)"
# Strip an optional single leading `v` (BSkando tags `vX.Y.Z`,
# jleinenbach `X.Y.Z[.N]`) so a genuine `v`-prefixed line tip is
# recognised as a version below rather than blanked as foreign.
REAL_TIP="${REAL_TIP#v}"
# Gate REAL_TIP against the canonical PEP 440 version shape (SSOT:
# script/stamp_version.py VERSION_RE, byte-mirrored in release-stamp.yml).
# A foreign, non-version tag (e.g. `set-release-tag`, `nightly`, `v2`)
# is not a valid line tip: it must NOT poison the PSR_LAST != REAL_TIP
# comparison below and spuriously route to the hand-tag draft. PEP 440
# beta/four-segment tags DO match VERSION_RE, so they are kept and still
# trigger MODE=hand on a real stale-base mismatch -- the intended guard.
# A foreign, non-version tag (e.g. `set-release-tag`, `nightly`, `v2`
# -> `2` after the v-strip) is not a valid line tip: it must NOT poison
# the PSR_LAST != REAL_TIP comparison below and spuriously route to the
# hand-tag draft. PEP 440 beta/four-segment tags DO match VERSION_RE, so
# they are kept and still trigger MODE=hand on a real stale-base
# mismatch -- the intended guard.
if [ -n "$REAL_TIP" ] && ! printf '%s' "$REAL_TIP" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?([ab][0-9]+)?$'; then
echo "::notice::ignoring non-PEP-440 line tip '${REAL_TIP}' for PSR routing (not a version tag)."
REAL_TIP=""
Expand Down
2 changes: 1 addition & 1 deletion custom_components/googlefindmy/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
# NOTE: no ": str" annotation on purpose -- semantic-release's version_variables
# regex only matches `NAME = "x"`, not `NAME: str = "x"`. Re-adding the annotation
# would silently skip this file on the automated version bump.
INTEGRATION_VERSION = "1.7.14.0"
INTEGRATION_VERSION = "1.7.14.1"

# --------------------------------------------------------------------------------------
# Shared textual constants
Expand Down
2 changes: 1 addition & 1 deletion custom_components/googlefindmy/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@
"selenium>=4.25.0",
"undetected_chromedriver>=3.5.5"
],
"version": "1.7.14.0"
"version": "1.7.14.1"
}
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ name = "googlefindmy-ha"
# script/stamp_version.py, so on those lines the hand-tag/stamp path is primary,
# not a fallback. Canonical anchor:
# custom_components/googlefindmy/AGENTS.md ("Version bump touches three files").
version = "1.7.14.0"
version = "1.7.14.1"
description = "Home Assistant integration for Google's Find My Device network."
readme = "README.md"
license = "MIT"
Expand Down
24 changes: 15 additions & 9 deletions script/stamp_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@
python script/stamp_version.py --version 1.7.14
python script/stamp_version.py --version 1.7.14b1 --repo-root /path/to/repo

The tag is validated against the ``tag_format = "{version}"`` convention
(no ``v`` prefix): ``X.Y.Z`` with an optional ``.N`` fourth segment followed by
an optional ``bN``/``aN`` prerelease (covers 1.7.14, 1.7.14b1, 1.7.13.1,
1.7.13.1b1). The segment order is PEP 440: the numeric release tuple precedes
the prerelease, so ``1.7.14.1b1`` is valid but ``1.7.14b1.1`` is not.
The ``--version`` argument is a *version* and never carries a ``v`` prefix:
``X.Y.Z`` with an optional ``.N`` fourth segment followed by an optional
``bN``/``aN`` prerelease (covers 1.7.14, 1.7.14b1, 1.7.13.1, 1.7.13.1b1). The
segment order is PEP 440: the numeric release tuple precedes the prerelease, so
``1.7.14.1b1`` is valid but ``1.7.14b1.1`` is not. The *published git tag* may
carry a leading ``v`` (BSkando tags ``vX.Y.Z``, jleinenbach ``X.Y.Z[.N]``);
release-stamp.yml strips that optional ``v`` before calling this script, so both
fork tag conventions produce the same v-less version literals.
"""

from __future__ import annotations
Expand All @@ -37,10 +40,13 @@
import re
from pathlib import Path

# Canonical tag/version shape (SSOT). tag_format = "{version}" means the git tag
# IS the version, so a leading `v` is rejected. Mirrors the real tag set on 1.7,
# which is PEP 440 (`bN`/`aN` betas, four-segment `.N`) -- deliberately broader
# than SemVer so hand-picked maintenance tags stamp cleanly. Consequence:
# Canonical VERSION shape (SSOT). This matches a *version* (what goes into the
# code literals), which never carries a `v`. A `v`-prefixed git tag (BSkando
# `vX.Y.Z`) is normalised by release-stamp.yml -- it strips the optional leading
# `v` before validating/stamping -- so this regex stays strictly v-less. Mirrors
# the real tag set on 1.7, which is PEP 440 (`bN`/`aN` betas, four-segment `.N`)
# -- deliberately broader than SemVer so hand-picked maintenance tags stamp
# cleanly. Consequence:
# semantic-release (SemVer-only) cannot compute a trustworthy *proposal* for such
# lines, so release.yml (Workflow A) opens an empty hand-tag draft there instead
# of proposing a stale number; the stamp path here still honours the exact chosen
Expand Down
143 changes: 142 additions & 1 deletion tests/test_hacs_validation.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# tests/test_hacs_validation.py
# tests/test_hacs_validation.py
"""Validate HACS metadata alignment and guard against unsupported characters."""

from __future__ import annotations
Expand All @@ -9,6 +8,7 @@
from pathlib import Path

import pytest
import yaml

from custom_components.googlefindmy.const import INTEGRATION_VERSION

Expand Down Expand Up @@ -52,6 +52,90 @@ def test_hacs_metadata_matches_manifest(
assert "homeassistant" not in manifest


def test_hacs_metadata_field_types(hacs_metadata: dict[str, object]) -> None:
"""hacs.json fields must carry the types HACS's ``hacsjson`` validator requires.

The ci.yml transient guard (see test_ci_tolerates_transient_hacs_regression)
may classify a HACS run ``transient`` when only ``hacsjson`` +
``integration_manifest`` fail, on the premise that a *genuine* hacs.json schema
error is caught locally instead. That premise only holds if this job rejects the
same class HACS would: HACS documents ``content_in_root``/``zip_release`` and the
other switches as booleans, so a non-boolean value (e.g. the string ``"true"``)
fails HACS ``hacsjson`` but would otherwise slip through the fallback green.
``test_hacs_metadata_matches_manifest`` only checks the allowed *keys*, not their
types; validate the types here so the fallback tolerance stays safe.
"""

# ``isinstance(5, bool)`` is False and ``isinstance("true", bool)`` is False, so
# this rejects exactly the non-boolean values HACS's hacsjson validator rejects.
for field in (
"content_in_root",
"zip_release",
"render_readme",
"hide_default_branch",
):
if field in hacs_metadata:
assert isinstance(hacs_metadata[field], bool), (
f"hacs.json '{field}' must be a boolean; HACS rejects other types "
f"(got {type(hacs_metadata[field]).__name__})"
)
for field in ("name", "filename", "homeassistant"):
if field in hacs_metadata:
assert isinstance(hacs_metadata[field], str), (
f"hacs.json '{field}' must be a string "
f"(got {type(hacs_metadata[field]).__name__})"
)


def test_hacs_metadata_requires_name(hacs_metadata: dict[str, object]) -> None:
"""hacs.json must declare a non-empty ``name`` (HACS's ``hacsjson`` validator).

The ci.yml transient guard (see test_ci_tolerates_transient_hacs_regression)
tolerates a HACS run when only ``hacsjson`` + ``integration_manifest`` fail, on the
premise that a *genuine* metadata regression is caught locally instead.
``test_hacs_metadata_field_types`` covers a wrong field *type*; this covers a
*missing* required key -- which HACS also rejects, but the fallback would otherwise
wave through green. Source: hacs.xyz/docs/publish/integration.
"""

name = hacs_metadata.get("name")
assert isinstance(name, str) and name.strip(), (
"hacs.json must declare a non-empty 'name' string; HACS's hacsjson validator "
"requires it"
)


def test_manifest_satisfies_hacs_integration_contract(
manifest: dict[str, object],
) -> None:
"""manifest.json must carry the keys HACS's ``integration_manifest`` validator needs.

HACS requires an integration manifest to declare ``domain``, ``documentation``,
``issue_tracker``, ``codeowners``, ``name`` and ``version``
(hacs.xyz/docs/publish/integration). The ci.yml transient guard ignores
``integration_manifest`` in its fallback and marks the job ``transient`` when the
remaining checks pass; that is only safe if a *genuine* manifest regression is
caught locally. hassfest overlaps but does not mirror HACS's exact set, so
reproduce the HACS contract here: a missing or wrongly-typed required key fails
this job deterministically, independent of the HACS tolerance.
"""

for field in ("domain", "documentation", "issue_tracker", "name", "version"):
value = manifest.get(field)
assert isinstance(value, str) and value.strip(), (
f"manifest.json must declare a non-empty '{field}' string "
"(HACS integration_manifest requirement)"
)
codeowners = manifest.get("codeowners")
assert isinstance(codeowners, list) and codeowners, (
"manifest.json 'codeowners' must be a non-empty list "
"(HACS integration_manifest requirement)"
)
assert all(isinstance(owner, str) and owner.strip() for owner in codeowners), (
"manifest.json 'codeowners' entries must all be non-empty strings"
)


def test_no_micro_sign_in_integration_files(
integration_python_files: list[Path], integration_root: Path
) -> None:
Expand Down Expand Up @@ -112,3 +196,60 @@ def test_release_zip_places_manifest_at_root() -> None:
assert f'gh release upload "$TAG_NAME" {filename} --clobber' in workflow, (
f"workflow must upload the declared hacs.json asset name {filename!r}"
)


def test_ci_tolerates_transient_hacs_regression() -> None:
"""ci.yml must guard the HACS job against the transient hacs/integration#5252
regression without permanently disabling the two affected validators.

The upstream action intermittently fetches the raw hacs.json / manifest.json
as None and reports them invalid on byte-identical, valid files. The guard
runs HACS first (continue-on-error), and only on failure re-runs it with
exactly ``hacsjson`` and ``integration_manifest`` isolated. If everything
else passes the failure was the known transient signature (tolerated); a
failure of any other check still exits non-zero (real defect). The isolation
must NOT leak into the primary run, otherwise those two checks would be off
permanently. This guard locks the shape so the resilience cannot silently
regress into either a blanket-ignore or a hard block.
"""

ci = yaml.safe_load(Path(".github/workflows/ci.yml").read_text(encoding="utf-8"))
steps = ci["jobs"]["hacs"]["steps"]
by_id = {step["id"]: step for step in steps if "id" in step}

primary = by_id["hacs"]
surgical = by_id["hacs_surgical"]
verdict = by_id["hacs_verdict"]

# Primary run tolerates failure (so the classifier can decide) and keeps the
# original, narrower ignore list: the isolation must not be permanent.
assert primary.get("continue-on-error") is True, (
"primary HACS run must set continue-on-error so the guard can classify"
)
assert primary["with"]["ignore"].split() == ["topics", "issues"], (
"primary HACS run must keep the narrow ignore (isolation not permanent)"
)

# Fallback isolates exactly the two transient-prone validators and only runs
# when the primary run failed (never when HACS is healthy).
assert surgical.get("continue-on-error") is True, (
"fallback HACS run must set continue-on-error"
)
assert set(surgical["with"]["ignore"].split()) == {
"topics",
"issues",
"hacsjson",
"integration_manifest",
}, "fallback HACS run must isolate exactly hacsjson + integration_manifest"
assert "steps.hacs.outcome == 'failure'" in surgical["if"], (
"fallback HACS run must be gated on the primary run failing"
)

# The classifier must be able to fail the job on a real defect: it must NOT
# be continue-on-error, and must exit non-zero outside the tolerated paths.
assert not verdict.get("continue-on-error", False), (
"classifier must not be continue-on-error, or a real defect passes silently"
)
assert "exit 1" in verdict["run"], (
"classifier must exit non-zero when the failure is not the transient one"
)
Loading
Loading