feat(release): release several binaries from one manifest by rianjs · Pull Request #41 · open-cli-collective/.github

rianjs · 2026-09-03T17:08:35Z

Closes #40

First consumer: open-cli-collective/google-cli, which ships gro and grw from one module, one goreleaser config, and one bare-v tag stream.

packaging/identity.yml declares exactly one of binary: (unchanged) or binaries: (list of {name, archives, packages, keychain_probe}); repo, goreleaser_config, version_file, tag stay top-level.
identity-check normalizes both shapes to a binaries list (the only consumers of the JSON are the workflows in this repo, updated here) and validates archives/nfpms/casks per owning binary through their ids/builds filters. Multi-binary Chocolatey packages live under packaging/chocolatey/<id>/; the flat single-binary layout is unchanged.
release.yml turns homebrew (max-parallel 1, one tap writer), chocolatey, winget, and linux-packages into matrix jobs over the binaries declaring each channel, resolves each binary's Windows archive names from dist/artifacts.json, and runs darwin-gate and code-signing per binary (identifier derived from the binary name).
New fixture tests/fixtures/identity/google-cli; existing fixtures unchanged and still validate.

Verified locally: pytest (identity-check + chocolatey-push), darwin-gate and codesign gate shell suites, all six fixtures through identity.py validate, actionlint. Real signing and channel publishing remain runner-level coverage.

packaging/identity.yml may declare 'binaries:' (a list of {name, archives, packages, keychain_probe}) instead of a single 'binary:'. identity-check normalizes both shapes to a 'binaries' list and validates goreleaser archives, nfpms, and casks per owning binary via their ids/builds filters. release.yml fans each publish channel out as a matrix over the binaries that declare it, resolves per-binary Windows archive names from artifacts.json, and runs the darwin gate and code-signing per binary. Multi-binary Chocolatey packages live under packaging/chocolatey/<id>/.

monit-reviewer · 2026-09-03T17:12:26Z

  support code.
+
+## Release identity manifests
+


🔵 Low (documentation:docs-reviewer): README states the manifest schema is open-cli-identity/v1, an identifier not mentioned in the PR description and not verifiable against packaging/identity.yml or identity-check's validation code from the diff alone. Confirm this schema string matches what identity-check actually expects before merging.

Reply to this thread when addressed.

Confirmed: SCHEMA in identity.py is the string 'open-cli-identity/v1' and load_manifest rejects anything else.

monit-reviewer · 2026-09-03T17:12:27Z

+nfpm, and Homebrew cask must use `ids` or `builds` to select builds belonging
+to one binary. Multi-binary Chocolatey packages live under
+`packaging/chocolatey/<id>/`; the single-binary flat layout is unchanged.
+


🔵 Low (documentation:docs-reviewer): New text documents calling auto-release.yml/release.yml with manifest-path and working-directory inputs, but these workflow_call inputs aren't described in the PR summary. Verify these exact input names exist on both reusable workflows so the documented invocation actually works.

Reply to this thread when addressed.

Confirmed: both reusable workflows declare workflow_call inputs manifest-path and working-directory (auto-release.yml lines 9-12, release.yml lines 15-18).

monit-reviewer · 2026-09-03T17:12:28Z

+        shell: bash
+        env:
+          CHOCOLATEY: ${{ steps.meta.outputs.chocolatey-base-matrix }}
+          WINGET: ${{ steps.meta.outputs.winget-base-matrix }}


🔵 Low (harness-engineering:harness-enforcement-reviewer): The archive-ownership predicate (owns($name): matches .extra.ID or membership in .extra.Binaries) is duplicated verbatim as inline jq in both release.yml's enrich() step and actions/darwin-gate/darwin-gate.sh's check_artifacts(). Since this rule determines artifact-to-binary ownership across the whole multi-binary release path, the two independently-maintained copies risk silent drift if goreleaser's extra.ID/extra.Binaries semantics change. Consider extracting a single shared jq filter used by both call sites.

Reply to this thread when addressed.

Leaving as is. The two call sites run in different contexts (a workflow step vs. a standalone script unit-tested on Linux) and a shared jq file would need its own plumbing into the step; the predicate is three lines and mirrors goreleaser's documented extra.ID/extra.Binaries fields.

monit-reviewer · 2026-09-03T17:12:29Z

+        binaries = [
+            _normalize_binary(
+                binary,
+                f"packaging/chocolatey/{(binary.get('packages', {}).get('chocolatey', {}) or {}).get('id')}"


🔵 Low (harness-engineering:harness-self-documenting-code-reviewer): normalize() computes the chocolatey directory with a ternary that repeats the same binary.get('packages', {}).get('chocolatey', {}) or {}).get('id') lookup twice inline inside a list comprehension. Extract into a named helper or local variable (the pattern is already handled this way inside _normalize_binary) to remove the duplication.

Reply to this thread when addressed.

Extracted _chocolatey_id() and used it in load_manifest and normalize.

monit-reviewer · 2026-09-03T17:12:29Z

+                    errors.append(f"duplicate goreleaser build id '{build.get('id')}'")
+                elif build.get("binary"):
+                    build_to_binary[build.get("id", f"__single_{index}")] = build["binary"]
+


🔵 Low (harness-engineering:harness-self-documenting-code-reviewer): The refactor dropped the comment explaining why every goreleaser build must set binary: explicitly (GoReleaser infers binary from the module when omitted, which can't be verified, so the drift guard would silently pass on an inferred name that may differ). Worth restoring a short version of this rationale near the new multi-binary validation code.

Reply to this thread when addressed.

Restored the rationale comment above the explicit-binary check.

monit-reviewer · 2026-09-03T17:12:30Z

+                elif owner in owned:
+                    owned[owner][kind].append(entry)
+
+    for name, binary in by_name.items():


🔵 Low (harness-engineering:harness-self-documenting-code-reviewer): The new duplicate-build-id detection uses build.get("id", f"__single_{index}") as a synthetic per-index key so builds without an explicit id never collide, while builds sharing an explicit id are correctly flagged. This is a non-obvious invariant with no explanation; a short comment or a named local (e.g. dedupe_key) would make the intent legible.

Reply to this thread when addressed.

Named the key dedupe_key with a comment explaining the synthetic per-index fallback.

monit-reviewer · 2026-09-03T17:12:31Z

+        if winget_id:
+            wdir = os.path.join(working_dir, "packaging", "winget")
+            expected = {
+                "version": os.path.join(wdir, f"{winget_id}.yaml"),


🔵 Low (harness-engineering:harness-self-documenting-code-reviewer): The old flat validation carried a comment explaining that alias_casks are intentionally not checked here (manifest-only, generated by a downstream alias step, per distribution.md §8.2). That rationale was dropped in the per-binary rewrite of the homebrew cask check; consider re-adding a condensed version near the new owned[name]["homebrew_casks"] block.

Reply to this thread when addressed.

Restored the alias_casks note above the cask check.

monit-reviewer · 2026-09-03T17:12:32Z

+                if entry.get("package_name") != linux_pkg:
+                    errors.append(f"{name}: goreleaser nfpm package_name '{entry.get('package_name')}' != manifest '{linux_pkg}'")
+
+        cask = (pkgs.get("homebrew", {}) or {}).get("canonical_cask")


🔵 Low (security:security-code-auditor): The chocolatey directory for a binary is built with os.path.join(working_dir, 'packaging', 'chocolatey', choco_id), and chocolatey_push.py's new --package-directory flag is used directly as Path(working_dir) / package_dir for the choco pack/push cwd. Neither choco_id nor binary['name'] is restricted to safe path-segment characters, so a value containing '../' could resolve outside packaging/chocolatey/. This PR is what turns a previously-hardcoded path into one derived from manifest data threaded through to the packing/pushing step; consider validating binary/chocolatey ids against a safe identifier pattern (e.g. ^[A-Za-z0-9._-]+$) as defense-in-depth.

Reply to this thread when addressed.

Added SAFE_ID_RE (^[A-Za-z0-9][A-Za-z0-9._-]*$) enforced in load_manifest for binary names and chocolatey ids, with two traversal tests.

monit-reviewer

Automated PR Review

Reviewed commit: 17a9f74

Approved with 10 non-blocking suggestions below. Address at your discretion.

Summary

Reviewer	Findings
documentation:docs-reviewer	2
harness-engineering:harness-enforcement-reviewer	2
harness-engineering:harness-self-documenting-code-reviewer	5
security:security-code-auditor	1

documentation:docs-reviewer (2 findings)

💡 Suggestion - README.md:54

README states the manifest schema is open-cli-identity/v1, an identifier not mentioned in the PR description and not verifiable against packaging/identity.yml or identity-check's validation code from the diff alone. Confirm this schema string matches what identity-check actually expects before merging.

💡 Suggestion - README.md:82

New text documents calling auto-release.yml/release.yml with manifest-path and working-directory inputs, but these workflow_call inputs aren't described in the PR summary. Verify these exact input names exist on both reusable workflows so the documented invocation actually works.

harness-engineering:harness-enforcement-reviewer (2 findings)

💡 Suggestion - actions/chocolatey-push/__pycache__/chocolatey_push.cpython-314.pyc:None

Compiled Python bytecode artifacts were committed to the repo (this file and actions/chocolatey-push/pycache/test_chocolatey_push.cpython-314-pytest-8.4.2.pyc). Add a .gitignore entry for pycache/ and remove these two files.

💡 Suggestion - .github/workflows/release.yml:191

The archive-ownership predicate (owns($name): matches .extra.ID or membership in .extra.Binaries) is duplicated verbatim as inline jq in both release.yml's enrich() step and actions/darwin-gate/darwin-gate.sh's check_artifacts(). Since this rule determines artifact-to-binary ownership across the whole multi-binary release path, the two independently-maintained copies risk silent drift if goreleaser's extra.ID/extra.Binaries semantics change. Consider extracting a single shared jq filter used by both call sites.

harness-engineering:harness-self-documenting-code-reviewer (5 findings)

💡 Suggestion - actions/identity-check/identity.py:110

normalize() computes the chocolatey directory with a ternary that repeats the same binary.get('packages', {}).get('chocolatey', {}) or {}).get('id') lookup twice inline inside a list comprehension. Extract into a named helper or local variable (the pattern is already handled this way inside _normalize_binary) to remove the duplication.

💡 Suggestion - actions/identity-check/identity.py:273

The refactor dropped the comment explaining why every goreleaser build must set binary: explicitly (GoReleaser infers binary from the module when omitted, which can't be verified, so the drift guard would silently pass on an inferred name that may differ). Worth restoring a short version of this rationale near the new multi-binary validation code.

💡 Suggestion - actions/identity-check/identity.py:282

The new duplicate-build-id detection uses build.get("id", f"__single_{index}") as a synthetic per-index key so builds without an explicit id never collide, while builds sharing an explicit id are correctly flagged. This is a non-obvious invariant with no explanation; a short comment or a named local (e.g. dedupe_key) would make the intent legible.

💡 Suggestion - actions/identity-check/identity.py:321

The old flat validation carried a comment explaining that alias_casks are intentionally not checked here (manifest-only, generated by a downstream alias step, per distribution.md §8.2). That rationale was dropped in the per-binary rewrite of the homebrew cask check; consider re-adding a condensed version near the new owned[name]["homebrew_casks"] block.

💡 Suggestion - actions/identity-check/identity.py:353

The prior chocolatey validation had a comment explaining why every matching .nuspec in a directory is checked rather than just one (a stale extra one with a different id could still publish under the wrong name). That rationale was lost when the loop was rewritten to iterate nuspecs per binary directory; worth restoring since it explains a non-obvious defensive check.

security:security-code-auditor (1 findings)

💡 Suggestion - actions/identity-check/identity.py:308

The chocolatey directory for a binary is built with os.path.join(working_dir, 'packaging', 'chocolatey', choco_id), and chocolatey_push.py's new --package-directory flag is used directly as Path(working_dir) / package_dir for the choco pack/push cwd. Neither choco_id nor binary['name'] is restricted to safe path-segment characters, so a value containing '../' could resolve outside packaging/chocolatey/. This PR is what turns a previously-hardcoded path into one derived from manifest data threaded through to the packing/pushing step; consider validating binary/chocolatey ids against a safe identifier pattern (e.g. ^[A-Za-z0-9._-]+$) as defense-in-depth.

Completed in 3m 43s | $3.82 | sonnet | daemon 0.2.142 | Glorfindel

Field	Value
Model	sonnet
Reviewers	hybrid-synthesis, database:database-reviewer, documentation:docs-reviewer, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer, harness-engineering:harness-self-documenting-code-reviewer, security:security-code-auditor
Engine	claude · sonnet
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	3m 43s wall · 3m 40s compute (Reviewers: 3m 05s · Synthesis: 35s)
Cost	$3.82 (estimated)
Tokens	737.1k in / 40.5k out
Turns	16

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost
hybrid-synthesis	sonnet	64.3k	3.5k	26.4k	37.9k (1h)	$0.29
database:database-reviewer	sonnet	69.7k	292	26.4k	43.3k (1h)	$0.27
documentation:docs-reviewer	sonnet	56.0k	3.8k	26.4k	29.5k (1h)	$0.24
harness-engineering:harness-architecture-reviewer	sonnet	123.5k	549	26.4k	97.1k (1h)	$0.60
harness-engineering:harness-enforcement-reviewer	sonnet	128.8k	5.3k	26.4k	102.4k (1h)	$0.70
harness-engineering:harness-knowledge-reviewer	sonnet	124.9k	1.4k	26.4k	98.4k (1h)	$0.62
harness-engineering:harness-self-documenting-code-reviewer	sonnet	91.4k	22.8k	26.4k	65.0k (1h)	$0.74
security:security-code-auditor	sonnet	78.6k	3.0k	26.4k	52.2k (1h)	$0.37

_{Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.}

Note: Posted 8 inline comments. 1 finding remained in the summary because GitHub could not attach them to the current diff.

…identifiers Both become path segments (packaging/chocolatey/<id>, the release title, the signing identifier), so a manifest value containing a separator or traversal now fails load_manifest. Also restores the rationale comments dropped in the per-binary rewrite and names the build dedupe key.

Adds §8.4 for repos that ship several binaries from one module, goreleaser config and tag stream via `binaries:` in `packaging/identity.yml` — the shape open-cli-collective/.github#41 introduced and google-cli is the first consumer of. Contrasts it with the §8.3 monorepo shape.

monit-reviewer reviewed Sep 3, 2026

View reviewed changes

monit-reviewer approved these changes Sep 3, 2026

View reviewed changes

rianjs merged commit e8daf35 into main Sep 3, 2026
21 checks passed

rianjs deleted the feat/40-multi-binary-release branch September 3, 2026 17:17

rianjs mentioned this pull request Sep 3, 2026

docs(distribution): describe multi-binary identity manifests open-cli-collective/cli-common#74

Merged

Conversation

rianjs commented Sep 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

monit-reviewer left a comment

Choose a reason for hiding this comment

Automated PR Review

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants