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
17 changes: 17 additions & 0 deletions .github/workflows/test-actions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,22 @@ jobs:
- uses: ./actions/identity-check
with:
working-directory: tests/fixtures/identity/${{ matrix.fixture }}
# self-contained fixtures: the goreleaser config is co-located with the
# manifest, so repo-root == working-directory (a flat-repo shape). The
# monorepo root-vs-tool asymmetry is covered by identity-unit.
repo-root: tests/fixtures/identity/${{ matrix.fixture }}

# Monorepo shape: tool-local identity/packaging under tools/<tool>, but the
# goreleaser config lives at the repo root and resolves via repo-root (≠
# working-directory). Proves the action's repo-root plumbing end-to-end.
identity-monorepo:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: ./actions/identity-check
with:
working-directory: tests/fixtures/identity/monorepo/tools/cfl
repo-root: tests/fixtures/identity/monorepo

# Exercise the action's export-json output and the require-manifest=false skip.
identity-interface:
Expand All @@ -195,6 +211,7 @@ jobs:
uses: ./actions/identity-check
with:
working-directory: tests/fixtures/identity/slck
repo-root: tests/fixtures/identity/slck
mode: validate-and-export
- name: assert validate-and-export emitted json
shell: bash
Expand Down
9 changes: 7 additions & 2 deletions actions/identity-check/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ inputs:
required: false
default: packaging/identity.yml
working-directory:
description: "module / tool root"
description: "module / tool root — manifest, packaging/, and version_file resolve here"
required: false
default: "."
repo-root:
description: "checkout root — goreleaser_config resolves here (distribution.md §8.3). Default '.' suits flat repos and monorepos whose goreleaser configs live at the repo root."
required: false
default: "."
require-manifest:
Expand Down Expand Up @@ -34,6 +38,7 @@ runs:
env:
MANIFEST: ${{ inputs.manifest-path }}
WD: ${{ inputs.working-directory }}
REPO_ROOT: ${{ inputs.repo-root }}
REQUIRE: ${{ inputs.require-manifest }}
MODE: ${{ inputs.mode }}
ACTION_PATH: ${{ github.action_path }}
Expand All @@ -45,7 +50,7 @@ runs:
*) echo "::error::unknown mode '$MODE' (expected validate|export-json|validate-and-export)"; exit 1 ;;
esac
if [ "$MODE" = "validate" ] || [ "$MODE" = "validate-and-export" ]; then
python "$script" validate --working-dir "$WD" --manifest "$MANIFEST" --require-manifest "$REQUIRE"
python "$script" validate --working-dir "$WD" --repo-root "$REPO_ROOT" --manifest "$MANIFEST" --require-manifest "$REQUIRE"
fi
if [ "$MODE" = "export-json" ] || [ "$MODE" = "validate-and-export" ]; then
# capture first (incl. stderr) so a failure surfaces in the log and
Expand Down
30 changes: 24 additions & 6 deletions actions/identity-check/identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
export-json print the normalized manifest as JSON (consumed by the
auto-release / release workflows so they never re-parse YAML)

Paths in the manifest (goreleaser_config) and the packaging/ dirs resolve
relative to --working-dir.
Path resolution is asymmetric (distribution.md §8.3): the tool-local identity —
the manifest, its packaging/ dirs, and version_file — resolves relative to
--working-dir, while goreleaser_config resolves relative to --repo-root (the
checkout root), because goreleaser is a repo-root release operation even in a
monorepo. For a flat repo both default to "." so behavior is unchanged.
"""
from __future__ import annotations

Expand Down Expand Up @@ -94,15 +97,25 @@ def _nuspec_id(path: str) -> str | None:
return None


def validate(manifest_path: str, working_dir: str) -> list[str]:
"""Return a list of drift errors (empty == clean)."""
def validate(manifest_path: str, working_dir: str, repo_root: str = ".") -> list[str]:
"""Return a list of drift errors (empty == clean).

Path resolution is intentionally ASYMMETRIC (distribution.md §8.3):
`goreleaser_config` resolves relative to `repo_root` (goreleaser is the
release-orchestration layer and, in a monorepo, runs from the repo root with
root context — go.work, shared modules, root tags), while the tool-local
identity (`packaging/*`, `version_file`, and the manifest itself) resolves
relative to `working_dir`. For a flat repo the two are the same dir, so
behavior is unchanged; a monorepo passes `working_dir=tools/<tool>` and leaves
`repo_root` at the checkout root.
"""
m = load_manifest(manifest_path)
errors: list[str] = []

# --- .goreleaser (binary + archive templates). If it's missing, record the
# error but still run the packaging/ checks below — they don't need it, so a
# mis-named goreleaser file shouldn't hide winget/choco drift. ---
gor_path = os.path.join(working_dir, m["goreleaser_config"])
gor_path = os.path.join(repo_root, m["goreleaser_config"])
gor: dict | None = None
if not os.path.isfile(gor_path):
errors.append(f"goreleaser_config not found: {gor_path}")
Expand Down Expand Up @@ -206,7 +219,7 @@ def cmd_validate(args) -> int:
print(f"no identity manifest at {manifest_path}; require-manifest is false — skipping")
return 0
try:
errors = validate(manifest_path, args.working_dir)
errors = validate(manifest_path, args.working_dir, args.repo_root)
except ManifestError as exc:
print(f"::error::{exc}")
return 1
Expand Down Expand Up @@ -244,6 +257,11 @@ def main(argv: list[str] | None = None) -> int:
sp.add_argument("--manifest", default="packaging/identity.yml")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Low (harness-engineering:harness-architecture-reviewer): --repo-root is registered only under the validate subparser. If dispatch logic ever changes, cmd_validate will raise AttributeError on args.repo_root; and callers who try to pass --repo-root to export-json will receive an unrecognized-argument error rather than a clear diagnostic. A brief comment at the subparser registration explaining why export-json intentionally omits --repo-root (goreleaser is a validate-only concern) would prevent both surprises without changing the interface.

Reply to this thread when addressed.

if name == "validate":
sp.add_argument("--require-manifest", type=_bool, default=True)
# goreleaser_config resolves relative to --repo-root (the checkout
# root), NOT --working-dir — see validate(). Defaults to "." so flat
# repos (working-dir ".") are unchanged; a monorepo leaves it at the
# checkout root while pointing --working-dir at tools/<tool>.
sp.add_argument("--repo-root", default=".")
args = p.parse_args(argv)
return cmd_validate(args) if args.cmd == "validate" else cmd_export_json(args)

Expand Down
88 changes: 73 additions & 15 deletions actions/identity-check/test_identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,35 +70,35 @@ def manifest_path(wd):

def test_pass(tmp_path):
wd = build(tmp_path)
assert identity.validate(manifest_path(wd), wd) == []
assert identity.validate(manifest_path(wd), wd, wd) == []


def test_drift_binary(tmp_path):
g = copy.deepcopy(BASE_GORELEASER)
g["builds"][0]["binary"] = "wrong"
wd = build(tmp_path, goreleaser=g)
assert any("binary" in e for e in identity.validate(manifest_path(wd), wd))
assert any("binary" in e for e in identity.validate(manifest_path(wd), wd, wd))


def test_drift_archive_template(tmp_path):
g = copy.deepcopy(BASE_GORELEASER)
g["archives"][0]["name_template"] = "slck_{{ .Version }}"
wd = build(tmp_path, goreleaser=g)
assert any("name_template" in e for e in identity.validate(manifest_path(wd), wd))
assert any("name_template" in e for e in identity.validate(manifest_path(wd), wd, wd))


def test_drift_nfpm_package_name(tmp_path):
g = copy.deepcopy(BASE_GORELEASER)
g["nfpms"][0]["package_name"] = "wrong"
wd = build(tmp_path, goreleaser=g)
assert any("nfpm" in e for e in identity.validate(manifest_path(wd), wd))
assert any("nfpm" in e for e in identity.validate(manifest_path(wd), wd, wd))


def test_declared_homebrew_without_block_fails(tmp_path):
g = copy.deepcopy(BASE_GORELEASER)
del g["homebrew_casks"]
wd = build(tmp_path, goreleaser=g)
assert any("homebrew" in e for e in identity.validate(manifest_path(wd), wd))
assert any("homebrew" in e for e in identity.validate(manifest_path(wd), wd, wd))


def test_drift_winget_id(tmp_path):
Expand All @@ -107,13 +107,13 @@ def test_drift_winget_id(tmp_path):
bad = os.path.join(wd, "packaging", "winget", "OpenCLICollective.slack-chat-cli.installer.yaml")
with open(bad, "w") as fh:
yaml.safe_dump({"PackageIdentifier": "OpenCLICollective.wrong"}, fh)
assert any("PackageIdentifier" in e for e in identity.validate(manifest_path(wd), wd))
assert any("PackageIdentifier" in e for e in identity.validate(manifest_path(wd), wd, wd))


def test_missing_winget_manifest(tmp_path):
wd = build(tmp_path)
os.remove(os.path.join(wd, "packaging", "winget", "OpenCLICollective.slack-chat-cli.installer.yaml"))
assert any("installer manifest missing" in e for e in identity.validate(manifest_path(wd), wd))
assert any("installer manifest missing" in e for e in identity.validate(manifest_path(wd), wd, wd))


def test_drift_choco_id(tmp_path):
Expand All @@ -123,7 +123,7 @@ def test_drift_choco_id(tmp_path):
wd = build(tmp_path, manifest=m)
nuspec = os.path.join(wd, "packaging", "chocolatey", "expected-id.nuspec")
open(nuspec, "w").write(NUSPEC.format(id="actually-different"))
assert any("chocolatey.id" in e for e in identity.validate(manifest_path(wd), wd))
assert any("chocolatey.id" in e for e in identity.validate(manifest_path(wd), wd, wd))


def test_namespaced_nuspec_id_extracted(tmp_path):
Expand All @@ -138,7 +138,7 @@ def test_namespaced_nuspec_id_extracted(tmp_path):
g = {"builds": [{"binary": "nrq"}], "archives": [{"name_template": "nrq_v{{ .Version }}_{{ .Os }}_{{ .Arch }}"}],
"nfpms": [{"package_name": "nrq"}], "homebrew_casks": [{"name": "nrq"}]}
wd = build(tmp_path, manifest=m, goreleaser=g)
assert identity.validate(manifest_path(wd), wd) == []
assert identity.validate(manifest_path(wd), wd, wd) == []


def test_missing_manifest_required_fails(tmp_path):
Expand All @@ -155,7 +155,7 @@ def test_binary_omitted_fails(tmp_path):
g = copy.deepcopy(BASE_GORELEASER)
del g["builds"][0]["binary"] # goreleaser would infer — unverifiable
wd = build(tmp_path, goreleaser=g)
assert any("binary:' explicitly" in e for e in identity.validate(manifest_path(wd), wd))
assert any("binary:' explicitly" in e for e in identity.validate(manifest_path(wd), wd, wd))


def test_goreleaser_missing_still_reports_winget_drift(tmp_path):
Expand All @@ -165,7 +165,7 @@ def test_goreleaser_missing_still_reports_winget_drift(tmp_path):
bad = os.path.join(wd, "packaging", "winget", "OpenCLICollective.slack-chat-cli.yaml")
with open(bad, "w") as fh:
yaml.safe_dump({"PackageIdentifier": "OpenCLICollective.wrong"}, fh)
errs = identity.validate(manifest_path(wd), wd)
errs = identity.validate(manifest_path(wd), wd, wd)
assert any("goreleaser_config not found" in e for e in errs)
assert any("PackageIdentifier" in e for e in errs) # not hidden by the goreleaser miss

Expand All @@ -175,29 +175,29 @@ def test_malformed_nuspec_clean_error(tmp_path):
nuspec = os.path.join(wd, "packaging", "chocolatey", "slack-chat-cli.nuspec")
open(nuspec, "w").write("<package><metadata><id>oops") # truncated XML
with pytest.raises(identity.ManifestError):
identity.validate(manifest_path(wd), wd)
identity.validate(manifest_path(wd), wd, wd)


def test_no_builds_fails(tmp_path):
g = copy.deepcopy(BASE_GORELEASER)
g["builds"] = []
wd = build(tmp_path, goreleaser=g)
assert any("no builds" in e for e in identity.validate(manifest_path(wd), wd))
assert any("no builds" in e for e in identity.validate(manifest_path(wd), wd, wd))


def test_no_archives_when_template_declared_fails(tmp_path):
g = copy.deepcopy(BASE_GORELEASER)
g["archives"] = []
wd = build(tmp_path, goreleaser=g)
assert any("no archives" in e for e in identity.validate(manifest_path(wd), wd))
assert any("no archives" in e for e in identity.validate(manifest_path(wd), wd, wd))


def test_stale_extra_nuspec_fails(tmp_path):
wd = build(tmp_path)
# a second .nuspec with a different <id> must fail, not be ignored
extra = os.path.join(wd, "packaging", "chocolatey", "stale.nuspec")
open(extra, "w").write(NUSPEC.format(id="stale-id"))
assert any("stale-id" in e for e in identity.validate(manifest_path(wd), wd))
assert any("stale-id" in e for e in identity.validate(manifest_path(wd), wd, wd))


def test_export_json_missing_manifest_errors(tmp_path):
Expand All @@ -213,3 +213,61 @@ def test_export_json_shape(tmp_path):
assert norm["packages"]["homebrew"]["alias_casks"] == ["slack-chat-cli"]
assert norm["packages"]["linux"]["package_name"] == "slck"
assert norm["version_file"] == "version.txt"


# --- monorepo: tool-local identity + packaging under tools/<tool>, but the
# goreleaser config lives at the repo root and resolves via --repo-root, not
# --working-dir (distribution.md §8.3). Models atlassian-cli's cfl tool. ---

def build_monorepo(tmp_path, tool="cfl", goreleaser_config=".goreleaser-cfl.yml"):
"""cfl-shaped fixture: root-level goreleaser config, tool-local packaging.
Returns (repo_root, working_dir)."""
m = {
"schema": "open-cli-identity/v1",
"repo": "atlassian-cli",
"binary": tool,
"version_file": "version.txt",
"goreleaser_config": goreleaser_config,
"tag": {"prefix": f"{tool}-v", "version_scheme": "major_minor_run_patch"},
"archives": {"name_template": "{{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }}"},
"packages": {
"homebrew": {"canonical_cask": tool},
"winget": {"id": f"OpenCLICollective.{tool}"},
"chocolatey": {"id": "confluence-cli"},
"linux": {"package_name": tool},
},
}
g = {
"builds": [{"binary": tool, "dir": f"tools/{tool}"}],
"archives": [{"name_template": "{{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }}"}],
"nfpms": [{"package_name": tool}],
"homebrew_casks": [{"name": tool}],
}
# goreleaser config at the REPO ROOT
(tmp_path / goreleaser_config).write_text(yaml.safe_dump(g))
# tool-local identity + packaging under tools/<tool>
wd = tmp_path / "tools" / tool
(wd / "packaging" / "winget").mkdir(parents=True, exist_ok=True)
(wd / "packaging" / "chocolatey").mkdir(parents=True, exist_ok=True)
(wd / "packaging" / "identity.yml").write_text(yaml.safe_dump(m))
wid = m["packages"]["winget"]["id"]
for suffix in (".yaml", ".installer.yaml", ".locale.en-US.yaml"):
(wd / "packaging" / "winget" / f"{wid}{suffix}").write_text(yaml.safe_dump({"PackageIdentifier": wid}))
cid = m["packages"]["chocolatey"]["id"]
(wd / "packaging" / "chocolatey" / f"{cid}.nuspec").write_text(NUSPEC.format(id=cid))
return str(tmp_path), str(wd)


def test_monorepo_root_relative_goreleaser_passes(tmp_path):
repo_root, wd = build_monorepo(tmp_path)
# working_dir=tools/cfl finds tool-local packaging; repo_root finds the
# root goreleaser config. Asymmetric resolution → clean.
assert identity.validate(os.path.join(wd, "packaging", "identity.yml"), wd, repo_root) == []


def test_monorepo_goreleaser_not_found_under_working_dir(tmp_path):
# Regression guard: the root goreleaser config must NOT be resolved relative
# to working_dir — if repo_root is (wrongly) the tool dir, it isn't found.
repo_root, wd = build_monorepo(tmp_path)
errs = identity.validate(os.path.join(wd, "packaging", "identity.yml"), wd, wd)
assert any("goreleaser_config not found" in e for e in errs)
9 changes: 9 additions & 0 deletions tests/fixtures/identity/monorepo/.goreleaser-cfl.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
builds:
- binary: cfl
dir: tools/cfl
archives:
- name_template: "{{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }}"
nfpms:
- package_name: cfl
homebrew_casks:
- name: cfl
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2015/06/nuspec.xsd">
<metadata><id>confluence-cli</id><version>0.0.0</version></metadata>
</package>
15 changes: 15 additions & 0 deletions tests/fixtures/identity/monorepo/tools/cfl/packaging/identity.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
schema: open-cli-identity/v1
repo: atlassian-cli
binary: cfl
goreleaser_config: .goreleaser-cfl.yml
version_file: version.txt
tag:
prefix: cfl-v
version_scheme: major_minor_run_patch
archives:
name_template: "{{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }}"
packages:
homebrew: { canonical_cask: cfl }
winget: { id: OpenCLICollective.cfl }
chocolatey: { id: confluence-cli }
linux: { package_name: cfl }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
PackageIdentifier: OpenCLICollective.cfl

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Low (harness-engineering:harness-enforcement-reviewer): Winget installer manifest is a minimal stub (PackageIdentifier only). This is sufficient for identity-check's current validation scope, but if identity-check ever validates additional required winget fields (PackageVersion, Installers, ManifestType, ManifestVersion), the fixture will generate false negatives. Worth noting for future fixture maintainers.

Reply to this thread when addressed.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
PackageIdentifier: OpenCLICollective.cfl
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
PackageIdentifier: OpenCLICollective.cfl
Loading