ci(api-review-gate): publish the gate as a commit status - #3440

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/core/talos/Makefile (1)

17-17: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider removing a stale archive before the copy.

skopeo copy to an existing oci-archive: destination can fail or append a second manifest, which then breaks the single-image assumption in e2e-fork.yaml. A repeated make image-talos in the same workspace hits this. Add an rm -f before the copy.

♻️ Proposed change

-	$(if $(strip $(OCI_EXPORT_DIR)),mkdir -p "$(OCI_EXPORT_DIR)"; skopeo copy "$$SRC" "oci-archive:$(OCI_EXPORT_DIR)/talos.oci.tar:$(IMAGE_TAG)",skopeo copy "$$SRC" docker://$(REGISTRY)/talos:$(IMAGE_TAG)); \
+	$(if $(strip $(OCI_EXPORT_DIR)),mkdir -p "$(OCI_EXPORT_DIR)"; rm -f "$(OCI_EXPORT_DIR)/talos.oci.tar"; skopeo copy "$$SRC" "oci-archive:$(OCI_EXPORT_DIR)/talos.oci.tar:$(IMAGE_TAG)",skopeo copy "$$SRC" docker://$(REGISTRY)/talos:$(IMAGE_TAG)); \

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/talos/Makefile` at line 17, Update the OCI export branch in the
Makefile command to remove the existing talos.oci.tar archive with rm -f
immediately before invoking skopeo copy, while leaving the registry-copy branch
unchanged.

hack/common-envs_test.bats (1)

24-24: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Pass COZYSTACK_VERSION=0 in these three tests to keep them hermetic.

hack/common-envs.mk runs a version probe at parse time. When COZYSTACK_VERSION is empty, it executes git remote add upstream … and git fetch upstream --tags (lines 128-132). Tests 4 and 5 already set COZYSTACK_VERSION=0, and e2e-fork.yaml sets it for the same reason. Tests 1-3 omit it, so a shallow or tagless checkout makes them mutate git remotes and reach the network.

♻️ Proposed change

-  out=$(make -n -C packages/system/cozystack-controller image OCI_EXPORT_DIR=/tmp/ocitest IMAGE_TAG=pr-1-abc BUILDER=b)
+  out=$(make -n -C packages/system/cozystack-controller image OCI_EXPORT_DIR=/tmp/ocitest IMAGE_TAG=pr-1-abc COZYSTACK_VERSION=0 BUILDER=b)

Apply the same addition to the capi-providers-cpprovider and talos invocations.

Also applies to: 37-37, 51-51

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/common-envs_test.bats` at line 24, Set COZYSTACK_VERSION=0 on the three
hermetic make invocations in hack/common-envs_test.bats, including the shown
system/cozystack-controller command and the capi-providers-cpprovider and talos
invocations. Preserve the existing arguments and command structure while
preventing parse-time version probing.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/core/talos/Makefile (1)

17-17: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider removing a stale archive before the copy.

skopeo copy to an existing oci-archive: destination can fail or append a second manifest, which then breaks the single-image assumption in e2e-fork.yaml. A repeated make image-talos in the same workspace hits this. Add an rm -f before the copy.

♻️ Proposed change

-	$(if $(strip $(OCI_EXPORT_DIR)),mkdir -p "$(OCI_EXPORT_DIR)"; skopeo copy "$$SRC" "oci-archive:$(OCI_EXPORT_DIR)/talos.oci.tar:$(IMAGE_TAG)",skopeo copy "$$SRC" docker://$(REGISTRY)/talos:$(IMAGE_TAG)); \
+	$(if $(strip $(OCI_EXPORT_DIR)),mkdir -p "$(OCI_EXPORT_DIR)"; rm -f "$(OCI_EXPORT_DIR)/talos.oci.tar"; skopeo copy "$$SRC" "oci-archive:$(OCI_EXPORT_DIR)/talos.oci.tar:$(IMAGE_TAG)",skopeo copy "$$SRC" docker://$(REGISTRY)/talos:$(IMAGE_TAG)); \

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/talos/Makefile` at line 17, Update the OCI export branch in the
Makefile command to remove the existing talos.oci.tar archive with rm -f
immediately before invoking skopeo copy, while leaving the registry-copy branch
unchanged.

hack/common-envs_test.bats (1)

24-24: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Pass COZYSTACK_VERSION=0 in these three tests to keep them hermetic.

hack/common-envs.mk runs a version probe at parse time. When COZYSTACK_VERSION is empty, it executes git remote add upstream … and git fetch upstream --tags (lines 128-132). Tests 4 and 5 already set COZYSTACK_VERSION=0, and e2e-fork.yaml sets it for the same reason. Tests 1-3 omit it, so a shallow or tagless checkout makes them mutate git remotes and reach the network.

♻️ Proposed change

-  out=$(make -n -C packages/system/cozystack-controller image OCI_EXPORT_DIR=/tmp/ocitest IMAGE_TAG=pr-1-abc BUILDER=b)
+  out=$(make -n -C packages/system/cozystack-controller image OCI_EXPORT_DIR=/tmp/ocitest IMAGE_TAG=pr-1-abc COZYSTACK_VERSION=0 BUILDER=b)

Apply the same addition to the capi-providers-cpprovider and talos invocations.

Also applies to: 37-37, 51-51

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/common-envs_test.bats` at line 24, Set COZYSTACK_VERSION=0 on the three
hermetic make invocations in hack/common-envs_test.bats, including the shown
system/cozystack-controller command and the capi-providers-cpprovider and talos
invocations. Preserve the existing arguments and command structure while
preventing parse-time version probing.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

.github/workflows/api-review-gate-fork.yaml (1)

44-67: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Privileged publishers do not bind the status SHA to an open pull request from the triggering fork. Both workflows publish a required merge-gate status using only workflow_run.head_sha. Neither proves that the SHA is the head of an open pull request whose head repository equals github.event.workflow_run.head_repository.full_name. A fork can point a branch at a commit that another open pull request also uses as its head, so the verdict computed for the fork branch lands on the shared SHA. Commit statuses are deduplicated per context and newest wins, so the fork verdict replaces the verdict a real run reached.

• .github/workflows/api-review-gate-fork.yaml#L44-L67: add pull-requests: read, look up the pull requests associated with HEAD_SHA, and post nothing unless an open pull request exists with head.sha === HEAD_SHA and head.repo.full_name === github.event.workflow_run.head_repository.full_name.
• .github/workflows/e2e-fork.yaml#L143-L152: extend the existing assoc.data.find(...) predicate with p.head.sha === HEAD_SHA and p.head.repo?.full_name === github.event.workflow_run.head_repository.full_name, so the docs-only decision, the TIA inputs, and the status target all come from the triggering fork's own pull request.

📍 Affects 2 files

• .github/workflows/api-review-gate-fork.yaml#L44-L67 (this comment)
• .github/workflows/e2e-fork.yaml#L143-L152

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/api-review-gate-fork.yaml around lines 44 - 67, Bind fork
status publication to the triggering pull request: in
.github/workflows/api-review-gate-fork.yaml lines 44-67, grant pull-requests:
read, query pull requests associated with HEAD_SHA, and publish nothing unless
an open request has head.sha === HEAD_SHA and head.repo.full_name matching
github.event.workflow_run.head_repository.full_name; in
.github/workflows/e2e-fork.yaml lines 143-152, extend the existing
assoc.data.find predicate with those same SHA and repository checks so all
decisions and status targets use the triggering fork’s pull request.

.github/workflows/e2e-fork.yaml (1)

304-322: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the installer image targets and what the derivation would emit.
fd -t f 'Makefile' packages/core/installer --exec cat -n {}
rg -n 'image-operator|^image:|image-packages' packages/core/installer

Repository: cozystack/cozystack

Length of output: 2743

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect the workflow derivation and push sections.
fd -t f 'e2e-fork.yaml' .github/workflows --exec sh -c '
  echo "--- $1 ---"
  sed -n "300,370p" "$1" | cat -n
' sh {}

# Inspect helper functions used in the installer Makefile image target.
fd -t f 'common-envs.mk|pre-checks.sh|Dockerfile' --exec sh -c '
  echo "--- $1 ---"
  wc -l "$1"
  sed -n "1,220p" "$1" | cat -n
' sh {}

# Simulate the make -n derivation behavior as shell (read-only from sources).
python3 - <<'PY'
from pathlib import Path
text = Path("packages/core/installer/Makefile").read_text()
print("Makefile contains image operator line:", "IMAGE=\"$(REGISTRY)/cozystack-operator" in text)
print("make derivation command present:", "make -n" in Path(".github/workflows/e2e-fork.yaml").read_text())
print("operator image name literal:", "cozystack-operator" in text)
PY

Repository: cozystack/cozystack

Length of output: 50375

---

🌐 Web query:

GNU make -n rule execution order prerequisities target recipe error handling

💡 Result:

In GNU make, the execution flow and error handling follow specific mechanisms: Rule Execution and Prerequisites GNU make processes targets by first identifying the dependency graph [1]. When you request a target, make evaluates its prerequisites to determine if the target is out of date (i.e., if the target does not exist or is older than any of its prerequisites) [1]. It recursively processes these prerequisites, ensuring they are built before the recipe for the main target is executed [2][1]. The order in which rules appear in the makefile does not determine execution order, except that the first non-pattern target in the first makefile becomes the default goal [1]. Error Handling in Recipes When a recipe line (a shell command) returns a non-zero exit status, make treats it as an error [2][3]. By default, make stops executing the current rule and immediately terminates, returning a non-zero status [2][4]. You can modify this behavior using several methods: - Ignore Errors for a Specific Line: Prefix a recipe line with a hyphen - (e.g., -rm file.o). This causes make to ignore any non-zero exit status for that specific command [2][3]. - Global Ignore: Run make with the -i or --ignore-errors flag to ignore all errors in all recipes [2][4]. Alternatively, you can define the special target.IGNORE in the makefile [2][3]. - Continue Processing: Run make with the -k or --keep-going flag. This instructs make to continue building other independent prerequisites even after a failure occurs, rather than exiting immediately [2][4]. - Cleanup on Failure: To prevent corrupted files from remaining with an updated timestamp after a failed recipe, use the special target.DELETE_ON_ERROR [2][3]. When enabled, make will delete a target if its recipe fails after it has begun modifying the file [2][3]. Expansion Timing Note that make differentiates between variable expansion and prerequisite evaluation [5]. Prerequisites are typically expanded during the first phase (parsing) of the makefile, whereas variables used within recipes are expanded during the second phase (when the recipe is executed) [5].

Citations:

• 1: https://web.mit.edu/gnu/doc/html/make_4.html
• 2: https://software.cfht.hawaii.edu/info/make/errors
• 3: https://tack.ch/gnu/make-3.82/make_41.html
• 4: https://www.cs.dartmouth.edu/~cs23/make-documentation/Errors.html
• 5: https://blog.summercat.com/on-gnu-make-prerequisites-expansion.html

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "--- installer Makefile image target ---"
sed -n '1,35p' packages/core/installer/Makefile | cat -n

echo "--- prereq files/scripts that image: depends on ---"
git ls-files packages/core/installer/pre-checks packages/*/hack/pre-checks.sh hack/pre-checks.sh | xargs -r ls -la

echo "--- make dry-run if make is available ---"
if command -v make >/dev/null 2>&1; then
  make -n -C packages/core/installer image OCI_EXPORT_DIR=/oci REGISTRY=r COZYSTACK_VERSION=0 | sed -n '1,80p'
fi

Repository: cozystack/cozystack

Length of output: 1625

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "--- make dry-run output ---"
make -n -C packages/core/installer image OCI_EXPORT_DIR=/oci REGISTRY=r COZYSTACK_VERSION=0 | sed -n '1,100p'

echo "--- pipeline output for current dry-run string ---"
make -n -C packages/core/installer image OCI_EXPORT_DIR=/oci REGISTRY=r COZYSTACK_VERSION=0 | \
  grep -oE '(dest=/oci/|oci-archive:/oci/)[a-z0-9._-]+\.oci\.tar' || true

Repository: cozystack/cozystack

Length of output: 227

---

Add cozystack-operator to the image allowlist.

The derivation only scans make -n image output for dest=.../.oci.tar, but the installer image target builds the operator first with image-tags(cozystack-operator, ...) and writes that image name to pr-oci-installer. A failed or skipped pre-checks prereq stops the recipe for image-operator, so the operator archive can be excluded while the push still creates r/cozystack-operator@.... Derive from the same targets the fork build invokes, or include cozystack-operator reliably.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/e2e-fork.yaml around lines 304 - 322, Update the “Derive
trusted image allowlist (base tree)” step so cozystack-operator is always
included, matching the installer targets invoked by the fork build even when
image-operator’s prereq output is skipped. Prefer deriving the name from the
same installer/image targets; otherwise explicitly add cozystack-operator to
_allow.txt while preserving the existing archive-derived entries and validation.

docs/agents/e2e-testing.md (1)

103-103: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the section heading: the gate is a commit status.

The heading says check-run, but the section states the opposite and explains why a check run is wrong. The heading is the first thing a reader sees, so it inverts the invariant.

📝 Proposed change

-### 10. The E2E workflow split: same-repo vs fork, and the "E2E Tests" check-run
+### 10. The E2E workflow split: same-repo vs fork, and the "E2E Tests" commit status

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

### 10. The E2E workflow split: same-repo vs fork, and the "E2E Tests" commit status

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/agents/e2e-testing.md` at line 103, Update the section 10 heading to
refer to the “E2E Tests” commit status instead of a check-run, keeping the
existing same-repo versus fork workflow wording unchanged.