(MOT-4299) feat(release): add auditable worker release pipelines - #719
Conversation
The harness_smoke setup output has had no consumer since 7f57418 gated candidates on the smoke only; candidate-ready hardcodes the harness gate as skipped.
A promotion always ships the candidate behind next, so Promote Worker now only requires the worker: the version is resolved from the Registry and the Release run is located from the resulting tag. Both inputs remain as overrides for the repair paths (retrying after next moved on, dispatched Release re-runs).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR adds catalog-driven contract-v2 release flow. It introduces shared version rules, worker catalog validation, immutable evidence, image aliasing, promotion and repair workflows, stack-version checks, terminal result artifacts, and updated release documentation. ChangesRelease contract and execution flows
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
skill-check — worker0 verified, 54 skipped (no docs/).
Four for four. Nicely done. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (9)
.github/scripts/release_candidate.py (1)
32-34: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRequire a valid
image_digestin the readiness gate for image deploys.
validate_evidencerejects image evidence whoseimage_digestis not asha256:digest (Line 124).build_evidencedoes not apply that rule. An image release withcontainer_alias_result == "success"and an empty--image-digesttherefore writescandidate_ready: true, passes theRequire all candidate gatesstep in.github/workflows/release.yml, and only fails later at promotion.Enforce the same rule at build time so the candidate fails in the Release run.
♻️ Proposed change to align build and validate gates
candidate_ready = results["publish"] == "success" and results["candidate_smoke"] == "success" if args.deploy == "image": - candidate_ready = candidate_ready and results["container_alias"] == "success" + candidate_ready = ( + candidate_ready + and results["container_alias"] == "success" + and bool(DIGEST_RE.fullmatch(args.image_digest or "")) + )🤖 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/scripts/release_candidate.py around lines 32 - 34, Update the image-deploy readiness gate in the release candidate logic so candidate_ready is true only when image_digest is a valid sha256: digest, matching the validation performed by validate_evidence. Preserve the existing publish, candidate_smoke, and container_alias success requirements, and apply the additional digest check only for image deployments..github/scripts/tests/test_release_result.py (1)
41-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for the
failedstatus.The three tests cover
succeededandpartial. No test drives the third branch, where nothing irreversible happened andstatusbecomesfailed. A dry-run case also covers therequiredlist withoutgithub_releaseandregistry_publish.💚 Proposed additional cases
def test_image_requires_alias_after_publish(): result = build_result( args( deploy="image", binary_build_result="skipped", container_build_result="success", container_alias_result="failure", ) ) assert result["status"] == "partial" assert "container_alias" in result["failed_requirements"] + + +def test_preflight_failure_is_failed(): + result = build_result( + args( + setup_result="failure", + github_release_result="skipped", + binary_build_result="skipped", + publish_result="skipped", + candidate_result="skipped", + ) + ) + assert result["status"] == "failed" + assert result["phase"] == "preflight" + + +def test_dry_run_skips_release_and_publish_requirements(): + result = build_result( + args( + dry_run="true", + staged="false", + github_release_result="skipped", + publish_result="skipped", + candidate_result="skipped", + ) + ) + assert result["status"] == "succeeded"🤖 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/scripts/tests/test_release_result.py around lines 41 - 63, Add a test alongside the existing status tests in test_release_result.py that exercises build_result with a dry-run configuration where no irreversible actions occur, then assert the result status is "failed" and validate the expected required entries exclude github_release and registry_publish..github/workflows/_container.yml (1)
61-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
tagsandimage_tagnow hold the same value.The alias list is gone, so the two outputs are identical. Keep one output and reference it at Line 90 to prevent later drift between the pushed tag and the exported tag.
♻️ Proposed change
{ echo "base=${BASE}" echo "image_tag=${BASE}:${VERSION}" - echo "tags=${BASE}:${VERSION}" } >> "$GITHUB_OUTPUT"Then at Line 90:
- tags: ${{ steps.refs.outputs.tags }} + tags: ${{ steps.refs.outputs.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 @.github/workflows/_container.yml around lines 61 - 66, Update the output block near the image publication setup to define only one canonical image-tag output, removing the duplicate tags/image_tag values. Update the later reference around the workflow’s line 90 to consume that single output so the pushed tag and exported tag cannot diverge..github/workflows/_container-alias.yml (1)
72-84: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a job timeout for the registry calls.
The step makes four
docker buildx imagetoolsnetwork calls. Withouttimeout-minutes, a hung registry call holds the job for the 360-minute default and blocks the downstreamcandidate-readyandrelease-resultjobs in.github/workflows/release.yml.♻️ Proposed change
alias: name: Move ${{ inputs.worker }}:${{ inputs.channel }} runs-on: ubuntu-latest + timeout-minutes: 15 outputs:🤖 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/_container-alias.yml around lines 72 - 84, Add a suitable timeout-minutes setting to the job or step containing “Move and verify alias” so hung docker buildx imagetools registry calls cannot consume the default 360-minute limit. Keep the existing registry commands and verification behavior unchanged..github/scripts/_lib.py (1)
85-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSeparate "no tags" from "tag lookup failed".
list_tagged_versionsreturns an empty list whengitfails or is absent.validate_release_historythen returns early, somanifest_version.py check-historypasses with no history check. A checkout without fetched tags produces the same silent pass. Make the failure explicit so the guard cannot disappear quietly.♻️ Proposed change
prefix = f"{worker}/v" try: output = subprocess.check_output( ["git", "tag", "--list", f"{prefix}*"], text=True, stderr=subprocess.DEVNULL, ) - except (subprocess.CalledProcessError, FileNotFoundError): - return [] + except (subprocess.CalledProcessError, FileNotFoundError) as error: + raise ValueError(f"cannot list tags for {worker}: {error}") from error return [line[len(prefix):] for line in output.splitlines() if line.startswith(prefix)]🤖 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/scripts/_lib.py around lines 85 - 96, Update list_tagged_versions to distinguish a successful lookup with no matching tags from a failed or unavailable git command, using an explicit failure signal rather than returning [] for both cases. Update validate_release_history to handle that failure as an error and prevent check-history from silently passing; preserve [] only for a successful lookup with no tags..github/scripts/parse_release_tag.py (1)
62-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCollapse the duplicated legacy dry-run branch.
The same condition
DRY_RUN_RE.search(version) and release_contract == "1"is evaluated at Line 62 and again at Line 75, andmaturityis assigned in two places. The behavior is correct, but one branch is easier to verify.♻️ Proposed change
- if DRY_RUN_RE.search(version) and release_contract == "1": - dry_run, is_pre = "true", "true" - else: + legacy_dry_run = bool(DRY_RUN_RE.search(version)) and release_contract == "1" + if legacy_dry_run: + dry_run, is_pre, maturity = "true", "true", "legacy-dry-run" + else: try: maturity = _lib.release_maturity(version) except ValueError as error: if release_contract == "2": print(f"::error::{error}", file=sys.stderr) return 1 maturity = "stable" if STABLE_VERSION_RE.fullmatch(version) else "legacy-prerelease" dry_run = "false" is_pre = "false" if maturity == "stable" else "true" - - if DRY_RUN_RE.search(version) and release_contract == "1": - maturity = "legacy-dry-run"🤖 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/scripts/parse_release_tag.py around lines 62 - 76, Collapse the duplicated DRY_RUN_RE/release_contract condition in the release maturity flow: assign the legacy dry-run maturity within the existing branch that sets dry_run and is_pre, and remove the later standalone condition while preserving all non-dry-run maturity and error-handling behavior..github/scripts/release_catalog.py (2)
17-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConvert malformed-catalog failures into the CLI error path.
maincatches onlyFileNotFoundErrorandValueError. Three realistic inputs escape it and print a traceback: invalid YAML raisesyaml.YAMLError, a top-level scalar or list makesraw.getraiseAttributeError, and a non-mappingdefaultsmakes{**defaults}raiseTypeError. Workflow steps then show a stack trace instead oferror: ....♻️ Proposed change
def load_catalog(path: Path = CATALOG_PATH) -> dict[str, dict[str, Any]]: import yaml - raw = yaml.safe_load(path.read_text()) or {} + try: + raw = yaml.safe_load(path.read_text()) or {} + except yaml.YAMLError as error: + raise ValueError(f"release catalog is not valid YAML: {error}") from error + if not isinstance(raw, dict): + raise ValueError("release catalog must be a mapping") if raw.get("schema_version") != 1: raise ValueError("release catalog schema_version must be 1") defaults = raw.get("defaults") or {} standard = raw.get("standard_workers") or [] special = raw.get("special_workers") or {} policies = raw.get("policies") or {} - if not isinstance(standard, list) or not isinstance(special, dict) or not isinstance(policies, dict): + if ( + not isinstance(defaults, dict) + or not isinstance(standard, list) + or not isinstance(special, dict) + or not isinstance(policies, dict) + ): raise ValueError("invalid release catalog shape")🤖 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/scripts/release_catalog.py around lines 17 - 28, Update load_catalog to normalize malformed catalog inputs into ValueError: catch yaml.YAMLError from safe_load, validate that the top-level result is a mapping before using raw.get, and validate defaults is a mapping before it is expanded. Preserve the existing main handling so these failures follow its CLI error path without tracebacks.
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCatalog paths resolve against the process CWD.
CATALOG_PATHis relative andvalidate_checkoutkeeps a separate default root, so both the CLI and its test depend on the caller's working directory.
.github/scripts/release_catalog.py#L14-L14: anchorCATALOG_PATHto the file location, and derive the checkout root passed tovalidate_checkoutandresolved_entriesfromargs.catalog..github/scripts/tests/test_release_catalog.py#L11-L13: after the default is anchored, keepload_catalog()with no argument; the test then passes from any working directory with no further change.🤖 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/scripts/release_catalog.py at line 14, Anchor CATALOG_PATH in release_catalog.py to the script/repository location instead of the process CWD, and derive the checkout root supplied to validate_checkout and resolved_entries from args.catalog. In .github/scripts/tests/test_release_catalog.py lines 11-13, make no direct change; retain load_catalog() without arguments so it uses the anchored default from any working directory..github/workflows/promote-worker.yml (1)
146-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrive the deployed-E2E requirement from the catalog, not the literal
harnessslug.Line 147 and Line 264 both hardcode the worker slug. If another worker later requires deployed E2E, Line 264 skips the validation step and the promotion proceeds without the gate. Read the requirement from the release catalog instead, next to the existing
release_catalog.py get "$WORKER" release_workflowcall.Also consider
-f per_page=100on the run query. The default page size is 30, so a green run for the requested version can fall outside the first page when E2E is dispatched often.#!/bin/bash # Description: Check whether the release catalog already expresses a deployed-E2E requirement. set -euo pipefail fd -t f 'release-workers.yaml' .github --exec cat -n ast-grep outline .github/scripts/release_catalog.py --items all rg -n -i 'e2e|deployed|promotion_gate|requires' .github/scripts/release_catalog.py rg -n -i "== 'harness'|== harness" .github/workflows🤖 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/promote-worker.yml around lines 146 - 158, Replace the literal harness-slug checks in the workflow’s promotion validation, including the logic around the existing release_catalog.py get "$WORKER" release_workflow call, with a catalog-driven deployed-E2E requirement so every configured worker receives the gate. Preserve the current run lookup and failure behavior, and add per_page=100 to the GitHub Actions run query so eligible successful runs beyond the default first page are considered.
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/scripts/release_candidate.py:
- Line 146: Update the maturity argument definition in the release-candidate
argument parser to accept the legacy-prerelease value alongside the existing
experimental, alpha, beta, and stable choices. Leave validate_evidence unchanged
so its existing stable-maturity promotion gate remains authoritative.
In @.github/scripts/tests/test_release_candidate.py:
- Around line 111-121: Extend test_image_candidate_requires_alias_and_digest
with a separate case that omits image_digest while keeping the image candidate
inputs otherwise valid, and assert evidence["candidate_ready"] is False. Retain
the existing alias-failure case so the test covers both required conditions.
In @.github/workflows/create-tag.yml:
- Around line 204-211: The existing-tag validation around annotation is
incompatible with the indented tag message and provides no field-level
diagnostics. Update the checks using the annotation variable in the create-tag
workflow to trim leading indentation before exact matching, and make each failed
field report which identity value mismatched before exiting; preserve the
existing ancestry validation and idempotent success path.
In @.github/workflows/promote-worker.yml:
- Around line 124-134: Update the auto-discovery query in the run_id lookup to
filter release.yml workflow runs by successful completion using the GitHub API
status parameter. Keep the existing tag-based head_branch filter and fallback
error handling unchanged, so only a completed green Release run is selected.
In @.github/workflows/repair-worker-release.yml:
- Around line 168-172: Update the digest-resolution step that writes the
image_digest GITHUB_OUTPUT to fail with a clear diagnostic when DEPLOY is image
and both artifact lookups leave digest empty. Add DEPLOY from
steps.meta.outputs.deploy to the step environment, preserve the fallback
lookups, and validate the resolved digest before setup continues.
In `@docs/sops/release.md`:
- Around line 56-63: Add a prerequisite to the release documentation near the
existing workflow and secret requirements directing users to migrate existing
Harness Release Control requests away from the direct latest-release path before
enabling these workflows on main.
- Around line 143-149: Update the “Candidate evidence” section to apply only to
workers gated by Registry/interface smoke, consistent with release.yml setting
staged and candidate-ready only when INTERFACE_SMOKE is true. Document the
separate completion path for workers with interface_smoke: false, without
requiring candidate evidence or Registry gates for them.
---
Nitpick comments:
In @.github/scripts/_lib.py:
- Around line 85-96: Update list_tagged_versions to distinguish a successful
lookup with no matching tags from a failed or unavailable git command, using an
explicit failure signal rather than returning [] for both cases. Update
validate_release_history to handle that failure as an error and prevent
check-history from silently passing; preserve [] only for a successful lookup
with no tags.
In @.github/scripts/parse_release_tag.py:
- Around line 62-76: Collapse the duplicated DRY_RUN_RE/release_contract
condition in the release maturity flow: assign the legacy dry-run maturity
within the existing branch that sets dry_run and is_pre, and remove the later
standalone condition while preserving all non-dry-run maturity and
error-handling behavior.
In @.github/scripts/release_candidate.py:
- Around line 32-34: Update the image-deploy readiness gate in the release
candidate logic so candidate_ready is true only when image_digest is a valid
sha256: digest, matching the validation performed by validate_evidence. Preserve
the existing publish, candidate_smoke, and container_alias success requirements,
and apply the additional digest check only for image deployments.
In @.github/scripts/release_catalog.py:
- Around line 17-28: Update load_catalog to normalize malformed catalog inputs
into ValueError: catch yaml.YAMLError from safe_load, validate that the
top-level result is a mapping before using raw.get, and validate defaults is a
mapping before it is expanded. Preserve the existing main handling so these
failures follow its CLI error path without tracebacks.
- Line 14: Anchor CATALOG_PATH in release_catalog.py to the script/repository
location instead of the process CWD, and derive the checkout root supplied to
validate_checkout and resolved_entries from args.catalog. In
.github/scripts/tests/test_release_catalog.py lines 11-13, make no direct
change; retain load_catalog() without arguments so it uses the anchored default
from any working directory.
In @.github/scripts/tests/test_release_result.py:
- Around line 41-63: Add a test alongside the existing status tests in
test_release_result.py that exercises build_result with a dry-run configuration
where no irreversible actions occur, then assert the result status is "failed"
and validate the expected required entries exclude github_release and
registry_publish.
In @.github/workflows/_container-alias.yml:
- Around line 72-84: Add a suitable timeout-minutes setting to the job or step
containing “Move and verify alias” so hung docker buildx imagetools registry
calls cannot consume the default 360-minute limit. Keep the existing registry
commands and verification behavior unchanged.
In @.github/workflows/_container.yml:
- Around line 61-66: Update the output block near the image publication setup to
define only one canonical image-tag output, removing the duplicate
tags/image_tag values. Update the later reference around the workflow’s line 90
to consume that single output so the pushed tag and exported tag cannot diverge.
In @.github/workflows/promote-worker.yml:
- Around line 146-158: Replace the literal harness-slug checks in the workflow’s
promotion validation, including the logic around the existing release_catalog.py
get "$WORKER" release_workflow call, with a catalog-driven deployed-E2E
requirement so every configured worker receives the gate. Preserve the current
run lookup and failure behavior, and add per_page=100 to the GitHub Actions run
query so eligible successful runs beyond the default first page are considered.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b407ee03-db25-47b3-b3c4-e7f615a8c16d
📒 Files selected for processing (26)
.github/release-workers.yaml.github/scripts/_lib.py.github/scripts/harness_e2e_evidence.py.github/scripts/manifest_version.py.github/scripts/parse_release_tag.py.github/scripts/release_candidate.py.github/scripts/release_catalog.py.github/scripts/release_result.py.github/scripts/tests/test_harness_e2e_evidence.py.github/scripts/tests/test_manifest_version.py.github/scripts/tests/test_parse_release_tag.py.github/scripts/tests/test_release_candidate.py.github/scripts/tests/test_release_catalog.py.github/scripts/tests/test_release_result.py.github/workflows/_container-alias.yml.github/workflows/_container.yml.github/workflows/alpha-release.yml.github/workflows/ci.yml.github/workflows/create-tag.yml.github/workflows/harness-e2e-deployed.yml.github/workflows/promote-worker.yml.github/workflows/release-lsp-vscode.yml.github/workflows/release.yml.github/workflows/repair-worker-release.ymldocs/sops/new-worker.mddocs/sops/release.md
| build.add_argument("--release-tag", required=True) | ||
| build.add_argument("--worker", required=True) | ||
| build.add_argument("--version", required=True) | ||
| build.add_argument("--maturity", choices=("experimental", "alpha", "beta", "stable"), required=True) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
--maturity rejects the legacy maturity values that contract-v1 tags produce.
parse_release_tag.py sets maturity = "legacy-prerelease" when a contract-1 tag carries a version that _lib.release_maturity cannot classify. .github/workflows/release.yml forwards that value as --maturity "$MATURITY" (Line 326). A contract-1 prerelease tag reaches this path, because staged only requires registry_tag == next, interface_smoke == true, and dry_run != true. Argparse then exits with code 2 and the candidate-ready job fails.
Accept the legacy maturity values and keep the stable-maturity requirement in validate_evidence, which already gates promotion (Lines 103-104).
🐛 Proposed fix to accept legacy maturity values
- build.add_argument("--maturity", choices=("experimental", "alpha", "beta", "stable"), required=True)
+ build.add_argument(
+ "--maturity",
+ choices=(
+ "experimental",
+ "alpha",
+ "beta",
+ "stable",
+ "legacy-prerelease",
+ "legacy-dry-run",
+ ),
+ required=True,
+ )📝 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.
| build.add_argument("--maturity", choices=("experimental", "alpha", "beta", "stable"), required=True) | |
| build.add_argument( | |
| "--maturity", | |
| choices=( | |
| "experimental", | |
| "alpha", | |
| "beta", | |
| "stable", | |
| "legacy-prerelease", | |
| "legacy-dry-run", | |
| ), | |
| required=True, | |
| ) |
🤖 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/scripts/release_candidate.py at line 146, Update the maturity
argument definition in the release-candidate argument parser to accept the
legacy-prerelease value alongside the existing experimental, alpha, beta, and
stable choices. Leave validate_evidence unchanged so its existing
stable-maturity promotion gate remains authoritative.
| def test_image_candidate_requires_alias_and_digest(): | ||
| evidence = build_evidence( | ||
| build_args( | ||
| worker="image-resize", | ||
| release_tag="image-resize/v1.2.3", | ||
| deploy="image", | ||
| image_digest="sha256:" + "c" * 64, | ||
| container_alias_result="failure", | ||
| ) | ||
| ) | ||
| assert evidence["candidate_ready"] is False |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The test name promises digest coverage that the assertion does not check.
test_image_candidate_requires_alias_and_digest only varies container_alias_result. The digest is valid in this case, so nothing here exercises the digest rule. Add a case that omits the digest.
If you accept the readiness-gate change proposed in .github/scripts/release_candidate.py, this case also proves the new build-time rule.
💚 Proposed additional case
def test_image_candidate_requires_alias_and_digest():
evidence = build_evidence(
build_args(
worker="image-resize",
release_tag="image-resize/v1.2.3",
deploy="image",
image_digest="sha256:" + "c" * 64,
container_alias_result="failure",
)
)
assert evidence["candidate_ready"] is False
+
+
+def test_image_candidate_without_digest_is_not_ready():
+ evidence = build_evidence(
+ build_args(
+ worker="image-resize",
+ release_tag="image-resize/v1.2.3",
+ deploy="image",
+ image_digest="",
+ container_alias_result="success",
+ )
+ )
+ assert evidence["candidate_ready"] is False📝 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.
| def test_image_candidate_requires_alias_and_digest(): | |
| evidence = build_evidence( | |
| build_args( | |
| worker="image-resize", | |
| release_tag="image-resize/v1.2.3", | |
| deploy="image", | |
| image_digest="sha256:" + "c" * 64, | |
| container_alias_result="failure", | |
| ) | |
| ) | |
| assert evidence["candidate_ready"] is False | |
| def test_image_candidate_requires_alias_and_digest(): | |
| evidence = build_evidence( | |
| build_args( | |
| worker="image-resize", | |
| release_tag="image-resize/v1.2.3", | |
| deploy="image", | |
| image_digest="sha256:" + "c" * 64, | |
| container_alias_result="failure", | |
| ) | |
| ) | |
| assert evidence["candidate_ready"] is False | |
| def test_image_candidate_without_digest_is_not_ready(): | |
| evidence = build_evidence( | |
| build_args( | |
| worker="image-resize", | |
| release_tag="image-resize/v1.2.3", | |
| deploy="image", | |
| image_digest="", | |
| container_alias_result="success", | |
| ) | |
| ) | |
| assert evidence["candidate_ready"] is False |
🤖 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/scripts/tests/test_release_candidate.py around lines 111 - 121,
Extend test_image_candidate_requires_alias_and_digest with a separate case that
omits image_digest while keeping the image candidate inputs otherwise valid, and
assert evidence["candidate_ready"] is False. Retain the existing alias-failure
case so the test covers both required conditions.
| annotation=$(git tag -l --format='%(contents)' "$TAG") | ||
| grep -Fx "worker: $WORKER" <<<"$annotation" >/dev/null | ||
| grep -Fx "version: $VERSION" <<<"$annotation" >/dev/null | ||
| grep -Fx "registry-tag: $REGISTRY_TAG" <<<"$annotation" >/dev/null | ||
| grep -Fx "experimental: $EXPERIMENTAL" <<<"$annotation" >/dev/null | ||
| git merge-base --is-ancestor "$TAG^{}" main | ||
| echo "exists=true" >>"$GITHUB_OUTPUT" | ||
| echo "::notice::$TAG already exists with matching identity; treating create-tag as idempotent" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
grep -Fx cannot match the annotation this workflow writes.
Lines 294-305 write the annotation lines with 10 spaces of leading indentation, because they sit inside an indented git tag -a -m string. Git's default tag-message cleanup removes trailing whitespace and edge blank lines; it does not remove leading indentation. The stored line is therefore worker: $WORKER.
grep -Fx "worker: $WORKER" requires an exact full-line match, so it fails. With set -euo pipefail, the step exits non-zero and the idempotent rerun path fails for every existing tag, which is the case this step exists to handle. _lib.read_tag_annotation tolerates the indentation, since parse_release_tag.py already reads these tags, so the writer, the parser, and this check disagree.
A mismatch also exits with no diagnostic message. Trim the indentation and report which field failed.
🐛 Proposed fix
- annotation=$(git tag -l --format='%(contents)' "$TAG")
- grep -Fx "worker: $WORKER" <<<"$annotation" >/dev/null
- grep -Fx "version: $VERSION" <<<"$annotation" >/dev/null
- grep -Fx "registry-tag: $REGISTRY_TAG" <<<"$annotation" >/dev/null
- grep -Fx "experimental: $EXPERIMENTAL" <<<"$annotation" >/dev/null
+ annotation=$(git tag -l --format='%(contents)' "$TAG" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')
+ for expected in "worker: $WORKER" "version: $VERSION" \
+ "registry-tag: $REGISTRY_TAG" "experimental: $EXPERIMENTAL"; do
+ grep -qFx "$expected" <<<"$annotation" || {
+ echo "::error::$TAG exists but its annotation does not contain '$expected'"
+ exit 2
+ }
+ done
git merge-base --is-ancestor "$TAG^{}" mainRun the following script to confirm the parser tolerates indentation while this check does not:
#!/bin/bash
# Inspect the annotation parser and a real annotated tag body.
set -uo pipefail
ast-grep run --pattern 'def read_tag_annotation($$$):
$$$' --lang python .github/scripts/_lib.py
# Show the stored annotation of an existing worker tag, with visible leading whitespace.
tag=$(git tag --list '*/v*' | head -n 1)
echo "tag: $tag"
git tag -l --format='%(contents)' "$tag" | cat -A | head -n 15Based on learnings, I did not raise SHA-pinning suggestions for workflow references in .github/workflows/.
🤖 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/create-tag.yml around lines 204 - 211, The existing-tag
validation around annotation is incompatible with the indented tag message and
provides no field-level diagnostics. Update the checks using the annotation
variable in the create-tag workflow to trim leading indentation before exact
matching, and make each failed field report which identity value mismatched
before exiting; preserve the existing ancestry validation and idempotent success
path.
| run_id="$RUN_ID_INPUT" | ||
| if [[ -z "$run_id" ]]; then | ||
| tag="${WORKER}/v${version}" | ||
| run_id=$(gh api -X GET "repos/${GITHUB_REPOSITORY}/actions/workflows/release.yml/runs" \ | ||
| -f head_branch="$tag" --jq '.workflow_runs[0].id // empty') | ||
| [[ -n "$run_id" ]] || { | ||
| echo "::error::No Release run found for tag ${tag}; pass release_run_id explicitly" | ||
| exit 2 | ||
| } | ||
| echo "::notice::candidate evidence expected in Release run ${run_id}" | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Filter the auto-located Release run by conclusion.
The query at Lines 127-128 selects the newest release.yml run for the tag with no status filter. If the newest run for that tag was cancelled, failed, or is still in progress, promotion picks it and then fails later at the evidence download with an artifact-not-found error. Add status=success so the lookup selects a completed, green run.
♻️ Proposed fix
run_id=$(gh api -X GET "repos/${GITHUB_REPOSITORY}/actions/workflows/release.yml/runs" \
- -f head_branch="$tag" --jq '.workflow_runs[0].id // empty')
+ -f head_branch="$tag" -f status=success \
+ --jq '.workflow_runs[0].id // empty')📝 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.
| run_id="$RUN_ID_INPUT" | |
| if [[ -z "$run_id" ]]; then | |
| tag="${WORKER}/v${version}" | |
| run_id=$(gh api -X GET "repos/${GITHUB_REPOSITORY}/actions/workflows/release.yml/runs" \ | |
| -f head_branch="$tag" --jq '.workflow_runs[0].id // empty') | |
| [[ -n "$run_id" ]] || { | |
| echo "::error::No Release run found for tag ${tag}; pass release_run_id explicitly" | |
| exit 2 | |
| } | |
| echo "::notice::candidate evidence expected in Release run ${run_id}" | |
| fi | |
| run_id="$RUN_ID_INPUT" | |
| if [[ -z "$run_id" ]]; then | |
| tag="${WORKER}/v${version}" | |
| run_id=$(gh api -X GET "repos/${GITHUB_REPOSITORY}/actions/workflows/release.yml/runs" \ | |
| -f head_branch="$tag" -f status=success \ | |
| --jq '.workflow_runs[0].id // empty') | |
| [[ -n "$run_id" ]] || { | |
| echo "::error::No Release run found for tag ${tag}; pass release_run_id explicitly" | |
| exit 2 | |
| } | |
| echo "::notice::candidate evidence expected in Release run ${run_id}" | |
| fi |
🤖 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/promote-worker.yml around lines 124 - 134, Update the
auto-discovery query in the run_id lookup to filter release.yml workflow runs by
successful completion using the GitHub API status parameter. Keep the existing
tag-based head_branch filter and fallback error handling unchanged, so only a
completed green Release run is selected.
| registry=false | ||
| github_release=false | ||
| image_alias=false | ||
| [[ -s registry-promotion.json ]] && registry=true | ||
| [[ -s github-release-after.json ]] && github_release=true | ||
| if [[ "$DEPLOY" != image || -s latest-manifest.json ]]; then image_alias=true; fi | ||
| status=failed | ||
| if [[ "$registry" == true ]]; then status=partial; fi | ||
| if [[ "$registry" == true && "$github_release" == true && "$image_alias" == true ]]; then | ||
| status=succeeded | ||
| fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Derive component results from step outcomes, not from file presence.
github-release-after.json is written at Line 363, and the assertion that validates it runs at Lines 364-366. latest-manifest.json is written at Line 351, and the alias cmp verification runs at Line 352. When either verification fails, the file still exists and is non-empty, so this step records github_release=true or image_alias=true and can report status=succeeded for a promotion whose alias does not match the candidate digest. The step runs with if: always(), so this is the failure path the artifact exists to describe.
Add ids to the three promotion steps and read steps.<id>.outcome.
🐛 Proposed fix
- name: Promote Registry release tag
+ id: registry
env:
@@
- name: Promote image latest alias
+ id: image_alias
if: steps.candidate.outputs.deploy == 'image'
@@
- name: Finalize GitHub Release
+ id: github_release
env: env:
OPERATION_ID: ${{ inputs.operation_id || format('github:{0}', github.run_id) }}
STEP_ID: ${{ inputs.step_id || 'promote' }}
NOTIFICATION_RESULT: ${{ steps.slack.outcome }}
DEPLOY: ${{ steps.candidate.outputs.deploy }}
+ REGISTRY_OUTCOME: ${{ steps.registry.outcome }}
+ GITHUB_RELEASE_OUTCOME: ${{ steps.github_release.outcome }}
+ IMAGE_ALIAS_OUTCOME: ${{ steps.image_alias.outcome }}
run: |
registry=false
github_release=false
image_alias=false
- [[ -s registry-promotion.json ]] && registry=true
- [[ -s github-release-after.json ]] && github_release=true
- if [[ "$DEPLOY" != image || -s latest-manifest.json ]]; then image_alias=true; fi
+ [[ "$REGISTRY_OUTCOME" == success ]] && registry=true
+ [[ "$GITHUB_RELEASE_OUTCOME" == success ]] && github_release=true
+ if [[ "$DEPLOY" != image || "$IMAGE_ALIAS_OUTCOME" == success ]]; then image_alias=true; fi| digest=$(jq -r '.image_digest // empty' source/release-candidate.json 2>/dev/null || true) | ||
| if [[ -z "$digest" ]]; then | ||
| digest=$(jq -r '.image_digest // empty' source/release-result.json 2>/dev/null || true) | ||
| fi | ||
| echo "image_digest=$digest" >>"$GITHUB_OUTPUT" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fail in setup when an image worker has no recoverable digest.
Lines 155-158 tolerate missing artifacts, so image_digest can be empty while setup still succeeds. The empty value then flows to Line 233 as expected_digest for _container-alias.yml, and to Line 361, where the bare regex test fails with no diagnostic. Reject the empty digest here for deploy == image so the cause is visible at the source.
🛡️ Proposed fix
digest=$(jq -r '.image_digest // empty' source/release-candidate.json 2>/dev/null || true)
if [[ -z "$digest" ]]; then
digest=$(jq -r '.image_digest // empty' source/release-result.json 2>/dev/null || true)
fi
+ if [[ "$DEPLOY" == image && -z "$digest" ]]; then
+ echo "::error::run $RELEASE_RUN_ID has no candidate or result evidence with an image digest"
+ exit 1
+ fi
echo "image_digest=$digest" >>"$GITHUB_OUTPUT"Add DEPLOY: ${{ steps.meta.outputs.deploy }} to this step's env block.
🤖 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/repair-worker-release.yml around lines 168 - 172, Update
the digest-resolution step that writes the image_digest GITHUB_OUTPUT to fail
with a clear diagnostic when DEPLOY is image and both artifact lookups leave
digest empty. Add DEPLOY from steps.meta.outputs.deploy to the step environment,
preserve the fallback lookups, and validate the resolved digest before setup
continues.
| ## Prerequisites | ||
|
|
||
| Tick **Experimental** on Create Tag to mark the worker unstable in the | ||
| registry. It is a badge and nothing else — the version publishes to the | ||
| selected channel, installs normally, and resolves normally. Promotion does not | ||
| clear the badge. | ||
| - Dispatch **Create Tag** from `main`. | ||
| - Configure `III_CI_APP_ID`, `III_CI_APP_PRIVATE_KEY`, and | ||
| `WORKERS_REGISTRY_API_KEY` as repository or organization secrets. | ||
| - Add the worker to `.github/release-workers.yaml`; see | ||
| [`new-worker.md`](new-worker.md#6-release-wiring-one-time-per-worker). | ||
| - Run the worker's lint and test suite before creating a release. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Document the required Release Control migration.
The catalog rejects direct latest releases for Harness. Existing Release Control requests that still use the direct-latest path will fail after these workflows are enabled on main. Add this migration as a prerequisite.
Proposed documentation change
- Dispatch **Create Tag** from `main`.
+- Migrate Release Control's Harness path to `next -> deployed E2E -> promote`.
+- Enable these workflows on `main` only after that migration.📝 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.
| ## Prerequisites | |
| Tick **Experimental** on Create Tag to mark the worker unstable in the | |
| registry. It is a badge and nothing else — the version publishes to the | |
| selected channel, installs normally, and resolves normally. Promotion does not | |
| clear the badge. | |
| - Dispatch **Create Tag** from `main`. | |
| - Configure `III_CI_APP_ID`, `III_CI_APP_PRIVATE_KEY`, and | |
| `WORKERS_REGISTRY_API_KEY` as repository or organization secrets. | |
| - Add the worker to `.github/release-workers.yaml`; see | |
| [`new-worker.md`](new-worker.md#6-release-wiring-one-time-per-worker). | |
| - Run the worker's lint and test suite before creating a release. | |
| ## Prerequisites | |
| - Dispatch **Create Tag** from `main`. | |
| - Migrate Release Control's Harness path to `next -> deployed E2E -> promote`. | |
| - Enable these workflows on `main` only after that migration. | |
| - Configure `III_CI_APP_ID`, `III_CI_APP_PRIVATE_KEY`, and | |
| `WORKERS_REGISTRY_API_KEY` as repository or organization secrets. | |
| - Add the worker to `.github/release-workers.yaml`; see | |
| [`new-worker.md`](new-worker.md#6-release-wiring-one-time-per-worker). | |
| - Run the worker's lint and test suite before creating a release. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~61-~61: The official name of this software platform is spelled with a capital “H”.
Context: ...ganization secrets. - Add the worker to .github/release-workers.yaml; see [`new-work...
(GITHUB)
🤖 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/sops/release.md` around lines 56 - 63, Add a prerequisite to the release
documentation near the existing workflow and secret requirements directing users
to migrate existing Harness Release Control requests away from the direct
latest-release path before enabling these workflows on main.
| ### Candidate evidence | ||
|
|
||
| A `next` release must resolve and install the exact candidate, verify the lock | ||
| and registered interface, and write | ||
| `release-candidate-<worker>-<version>/release-candidate.json`. Schema v2 binds | ||
| the original Release run, evidence-producing run, run attempt, tag SHA, source | ||
| SHA, operation identity, maturity, gate results, and image digest. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Scope candidate evidence requirements to Registry-gated workers.
The SOP says every next release must produce candidate evidence. However, release.yml sets staged and candidate-ready only when INTERFACE_SMOKE == true. Workers with interface_smoke: false intentionally skip these Registry gates. Qualify this section and document their separate completion path.
Proposed documentation change
-A `next` release must resolve and install the exact candidate, verify the lock
+ A Registry-gated `next` release must resolve and install the exact candidate,
+ verify the lock
and registered interface, and write
`release-candidate-<worker>-<version>/release-candidate.json`.
+ Workers with `interface_smoke: false` skip these candidate gates; document
+ their separate release-completion path.🤖 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/sops/release.md` around lines 143 - 149, Update the “Candidate evidence”
section to apply only to workers gated by Registry/interface smoke, consistent
with release.yml setting staged and candidate-ready only when INTERFACE_SMOKE is
true. Document the separate completion path for workers with interface_smoke:
false, without requiring candidate evidence or Registry gates for them.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/workflows/_harness-e2e.yml:
- Around line 179-186: Update the stack-version validation around STACK_VERSIONS
to normalize an empty map to an object containing RELEASE_WORKER mapped to
RELEASE_VERSION before running the jq schema and worker-version checks. Preserve
the existing validation for non-empty stack maps so dispatches with explicit
stack_versions continue to be validated unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4cee0eb3-d9c0-4910-b2f4-83f5202e205f
📒 Files selected for processing (11)
.github/release-workers.yaml.github/scripts/harness_e2e_evidence.py.github/scripts/release_catalog.py.github/scripts/tests/test_harness_e2e_evidence.py.github/scripts/tests/test_parse_release_tag.py.github/scripts/tests/test_release_catalog.py.github/scripts/tests/test_verify_registry_lock.py.github/scripts/verify_registry_lock.py.github/workflows/_harness-e2e.yml.github/workflows/harness-e2e-deployed.ymlharness/tests/e2e/run-deployed-ci.sh
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/scripts/tests/test_parse_release_tag.py
- .github/scripts/tests/test_harness_e2e_evidence.py
- .github/scripts/tests/test_release_catalog.py
- .github/release-workers.yaml
- .github/scripts/harness_e2e_evidence.py
- .github/scripts/release_catalog.py
| jq -e --arg worker "$RELEASE_WORKER" --arg version "$RELEASE_VERSION" ' | ||
| type == "object" and | ||
| all(to_entries[]; | ||
| (.key | test("^[a-z0-9][a-z0-9_-]*$")) and | ||
| (.value | type == "string" and test("^[0-9]+\\.[0-9]+\\.[0-9]+(-(experimental|alpha|beta))?$")) | ||
| ) and | ||
| .[$worker] == $version | ||
| ' <<<"$STACK_VERSIONS" >/dev/null |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Accept the documented empty stack map.
The optional input defaults to {}. This check rejects {} because it has no release-worker entry. The job fails before run-deployed-ci.sh can apply its empty-map fallback.
Normalize {} to {release_worker: release_version} before this validation. This preserves existing dispatches that do not provide stack_versions.
Proposed fix
jq -e --arg worker "$RELEASE_WORKER" --arg version "$RELEASE_VERSION" '
- type == "object" and
+ if . == {} then {($worker): $version} else . end |
+ type == "object" and
all(to_entries[];
(.key | test("^[a-z0-9][a-z0-9_-]*$")) and
(.value | type == "string" and test("^[0-9]+\\.[0-9]+\\.[0-9]+(-(experimental|alpha|beta))?$"))
) and
.[$worker] == $version
' <<<"$STACK_VERSIONS" >/dev/null📝 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.
| jq -e --arg worker "$RELEASE_WORKER" --arg version "$RELEASE_VERSION" ' | |
| type == "object" and | |
| all(to_entries[]; | |
| (.key | test("^[a-z0-9][a-z0-9_-]*$")) and | |
| (.value | type == "string" and test("^[0-9]+\\.[0-9]+\\.[0-9]+(-(experimental|alpha|beta))?$")) | |
| ) and | |
| .[$worker] == $version | |
| ' <<<"$STACK_VERSIONS" >/dev/null | |
| jq -e --arg worker "$RELEASE_WORKER" --arg version "$RELEASE_VERSION" ' | |
| if . == {} then {($worker): $version} else . end | | |
| type == "object" and | |
| all(to_entries[]; | |
| (.key | test("^[a-z0-9][a-z0-9_-]*$")) and | |
| (.value | type == "string" and test("^[0-9]+\\.[0-9]+\\.[0-9]+(-(experimental|alpha|beta))?$")) | |
| ) and | |
| .[$worker] == $version | |
| ' <<<"$STACK_VERSIONS" >/dev/null |
🤖 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/_harness-e2e.yml around lines 179 - 186, Update the
stack-version validation around STACK_VERSIONS to normalize an empty map to an
object containing RELEASE_WORKER mapped to RELEASE_VERSION before running the jq
schema and worker-version checks. Preserve the existing validation for non-empty
stack maps so dispatches with explicit stack_versions continue to be validated
unchanged.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/scripts/harness_e2e_evidence.py:
- Around line 49-52: Update the snapshot-loading logic in the evidence helper
around the path validation and subjects extraction to catch read, JSON-decoding,
and object-shape failures. Treat unreadable, malformed, or non-object snapshots
as incomplete coverage by returning the existing empty evidence values with a
false completion status, allowing the caller to write and upload the terminal
failure record.
In @.github/scripts/harness_e2e_profiles.py:
- Around line 55-94: Update load_profile_catalog to validate release_control,
harness_e2e, profiles, release, and full as mappings before calling .get(),
raising the existing documented ValueError for malformed catalog objects. Also
require each scenario group to be a string before checking membership in the
allowed group set, and add tests covering scalar/list mappings and non-string
group values.
In @.github/workflows/_harness-e2e.yml:
- Around line 336-341: Update the profile resolver invocation assigning resolved
in the workflow to resolve the checked-out repository HEAD after checkout and
pass that revision as catalog_sha instead of the event-based github.sha value,
ensuring evidence identifies the catalog actually used.
In `@harness/tests/e2e/README.md`:
- Line 223: Update the paragraph following the “Harness E2E deployed” table
entry to state that deployed E2E can be triggered by either a successful
post-release smoke or a Release Control dispatch, while preserving the
requirement that promotion must pass the release gate.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6cd9865f-e488-4091-a5c2-d96e666100b3
📒 Files selected for processing (12)
.github/release-workers.yaml.github/scripts/harness_e2e_evidence.py.github/scripts/harness_e2e_profiles.py.github/scripts/release_catalog.py.github/scripts/tests/test_harness_e2e_evidence.py.github/scripts/tests/test_harness_e2e_profiles.py.github/scripts/tests/test_parse_release_tag.py.github/scripts/tests/test_release_catalog.py.github/workflows/_harness-e2e.yml.github/workflows/harness-e2e-deployed.yml.github/workflows/promote-worker.ymlharness/tests/e2e/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/scripts/tests/test_release_catalog.py
- .github/scripts/tests/test_parse_release_tag.py
- .github/scripts/release_catalog.py
- .github/workflows/promote-worker.yml
| if path is None or not path.is_file(): | ||
| return [], [], False | ||
| snapshot = json.loads(path.read_text()) | ||
| subjects = snapshot.get("subjects") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Preserve terminal evidence when the snapshot is malformed.
A malformed downloaded snapshot raises from json.loads() or .get(). The evidence job then exits before it writes and uploads the terminal failure record. Treat unreadable, invalid, and non-object snapshots as incomplete coverage.
Proposed fix
- snapshot = json.loads(path.read_text())
+ try:
+ snapshot = json.loads(path.read_text())
+ except (OSError, json.JSONDecodeError):
+ return [], [], False
+ if not isinstance(snapshot, dict):
+ return [], [], False
subjects = snapshot.get("subjects")📝 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.
| if path is None or not path.is_file(): | |
| return [], [], False | |
| snapshot = json.loads(path.read_text()) | |
| subjects = snapshot.get("subjects") | |
| if path is None or not path.is_file(): | |
| return [], [], False | |
| try: | |
| snapshot = json.loads(path.read_text()) | |
| except (OSError, json.JSONDecodeError): | |
| return [], [], False | |
| if not isinstance(snapshot, dict): | |
| return [], [], False | |
| subjects = snapshot.get("subjects") |
🤖 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/scripts/harness_e2e_evidence.py around lines 49 - 52, Update the
snapshot-loading logic in the evidence helper around the path validation and
subjects extraction to catch read, JSON-decoding, and object-shape failures.
Treat unreadable, malformed, or non-object snapshots as incomplete coverage by
returning the existing empty evidence values with a false completion status,
allowing the caller to write and upload the terminal failure record.
| def load_profile_catalog(path: Path = CATALOG_PATH) -> HarnessE2eCatalog: | ||
| raw = yaml.safe_load(path.read_text()) or {} | ||
| release_control = raw.get("release_control") or {} | ||
| if release_control.get("harness_e2e_profiles") != 1: | ||
| raise ValueError("release catalog must expose harness_e2e_profiles: 1") | ||
| config = raw.get("harness_e2e") or {} | ||
| required_profile = config.get("required_profile") | ||
| if required_profile != "release": | ||
| raise ValueError("harness_e2e.required_profile must be release") | ||
|
|
||
| raw_scenarios = config.get("scenarios") | ||
| if not isinstance(raw_scenarios, list) or not raw_scenarios: | ||
| raise ValueError("harness_e2e.scenarios must be a non-empty array") | ||
| scenarios: list[dict[str, str]] = [] | ||
| seen: set[str] = set() | ||
| for entry in raw_scenarios: | ||
| if not isinstance(entry, dict): | ||
| raise ValueError("harness_e2e.scenarios entries must be objects") | ||
| scenario_id = entry.get("id") | ||
| group = entry.get("group") | ||
| if not isinstance(scenario_id, str) or not scenario_id: | ||
| raise ValueError("Harness E2E scenario ids must be non-empty strings") | ||
| if scenario_id in seen: | ||
| raise ValueError(f"Harness E2E scenario id repeats {scenario_id}") | ||
| if group not in {"Quality", "Operations", "Validation"}: | ||
| raise ValueError(f"{scenario_id}: unsupported Harness E2E group") | ||
| seen.add(scenario_id) | ||
| scenarios.append({"id": scenario_id, "group": group}) | ||
|
|
||
| profiles = config.get("profiles") or {} | ||
| release = profiles.get("release") or {} | ||
| release_scenarios = _string_list( | ||
| release.get("scenarios"), "harness_e2e.profiles.release.scenarios" | ||
| ) | ||
| unknown = [scenario for scenario in release_scenarios if scenario not in seen] | ||
| if unknown: | ||
| raise ValueError(f"release profile references unknown scenarios: {', '.join(unknown)}") | ||
| full = profiles.get("full") or {} | ||
| if full.get("scenarios") != "all": | ||
| raise ValueError("harness_e2e.profiles.full.scenarios must be all") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Validate YAML mapping types before accessing them.
A scalar or list in release_control, harness_e2e, profiles, release, or full causes AttributeError on .get(). A list or mapping in group causes TypeError during set membership. These exceptions bypass the ValueError handler in main() and emit a traceback instead of the documented invalid-catalog error.
Add mapping validation for each object field. Validate that group is a string before checking its allowed values. Add malformed-mapping tests.
Proposed fix
+def _mapping(value: Any, field: str) -> dict[str, Any]:
+ if value is None:
+ return {}
+ if not isinstance(value, dict):
+ raise ValueError(f"{field} must be an object")
+ return value
+
def load_profile_catalog(path: Path = CATALOG_PATH) -> HarnessE2eCatalog:
- raw = yaml.safe_load(path.read_text()) or {}
- release_control = raw.get("release_control") or {}
+ raw = _mapping(yaml.safe_load(path.read_text()), "release catalog")
+ release_control = _mapping(raw.get("release_control"), "release_control")
if release_control.get("harness_e2e_profiles") != 1:
raise ValueError("release catalog must expose harness_e2e_profiles: 1")
- config = raw.get("harness_e2e") or {}
+ config = _mapping(raw.get("harness_e2e"), "harness_e2e")
@@
- if group not in {"Quality", "Operations", "Validation"}:
+ if not isinstance(group, str) or group not in {"Quality", "Operations", "Validation"}:
raise ValueError(f"{scenario_id}: unsupported Harness E2E group")
@@
- profiles = config.get("profiles") or {}
- release = profiles.get("release") or {}
+ profiles = _mapping(config.get("profiles"), "harness_e2e.profiles")
+ release = _mapping(profiles.get("release"), "harness_e2e.profiles.release")
@@
- full = profiles.get("full") or {}
+ full = _mapping(profiles.get("full"), "harness_e2e.profiles.full")📝 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.
| def load_profile_catalog(path: Path = CATALOG_PATH) -> HarnessE2eCatalog: | |
| raw = yaml.safe_load(path.read_text()) or {} | |
| release_control = raw.get("release_control") or {} | |
| if release_control.get("harness_e2e_profiles") != 1: | |
| raise ValueError("release catalog must expose harness_e2e_profiles: 1") | |
| config = raw.get("harness_e2e") or {} | |
| required_profile = config.get("required_profile") | |
| if required_profile != "release": | |
| raise ValueError("harness_e2e.required_profile must be release") | |
| raw_scenarios = config.get("scenarios") | |
| if not isinstance(raw_scenarios, list) or not raw_scenarios: | |
| raise ValueError("harness_e2e.scenarios must be a non-empty array") | |
| scenarios: list[dict[str, str]] = [] | |
| seen: set[str] = set() | |
| for entry in raw_scenarios: | |
| if not isinstance(entry, dict): | |
| raise ValueError("harness_e2e.scenarios entries must be objects") | |
| scenario_id = entry.get("id") | |
| group = entry.get("group") | |
| if not isinstance(scenario_id, str) or not scenario_id: | |
| raise ValueError("Harness E2E scenario ids must be non-empty strings") | |
| if scenario_id in seen: | |
| raise ValueError(f"Harness E2E scenario id repeats {scenario_id}") | |
| if group not in {"Quality", "Operations", "Validation"}: | |
| raise ValueError(f"{scenario_id}: unsupported Harness E2E group") | |
| seen.add(scenario_id) | |
| scenarios.append({"id": scenario_id, "group": group}) | |
| profiles = config.get("profiles") or {} | |
| release = profiles.get("release") or {} | |
| release_scenarios = _string_list( | |
| release.get("scenarios"), "harness_e2e.profiles.release.scenarios" | |
| ) | |
| unknown = [scenario for scenario in release_scenarios if scenario not in seen] | |
| if unknown: | |
| raise ValueError(f"release profile references unknown scenarios: {', '.join(unknown)}") | |
| full = profiles.get("full") or {} | |
| if full.get("scenarios") != "all": | |
| raise ValueError("harness_e2e.profiles.full.scenarios must be all") | |
| def _mapping(value: Any, field: str) -> dict[str, Any]: | |
| if value is None: | |
| return {} | |
| if not isinstance(value, dict): | |
| raise ValueError(f"{field} must be an object") | |
| return value | |
| def load_profile_catalog(path: Path = CATALOG_PATH) -> HarnessE2eCatalog: | |
| raw = _mapping(yaml.safe_load(path.read_text()), "release catalog") | |
| release_control = _mapping(raw.get("release_control"), "release_control") | |
| if release_control.get("harness_e2e_profiles") != 1: | |
| raise ValueError("release catalog must expose harness_e2e_profiles: 1") | |
| config = _mapping(raw.get("harness_e2e"), "harness_e2e") | |
| required_profile = config.get("required_profile") | |
| if required_profile != "release": | |
| raise ValueError("harness_e2e.required_profile must be release") | |
| raw_scenarios = config.get("scenarios") | |
| if not isinstance(raw_scenarios, list) or not raw_scenarios: | |
| raise ValueError("harness_e2e.scenarios must be a non-empty array") | |
| scenarios: list[dict[str, str]] = [] | |
| seen: set[str] = set() | |
| for entry in raw_scenarios: | |
| if not isinstance(entry, dict): | |
| raise ValueError("harness_e2e.scenarios entries must be objects") | |
| scenario_id = entry.get("id") | |
| group = entry.get("group") | |
| if not isinstance(scenario_id, str) or not scenario_id: | |
| raise ValueError("Harness E2E scenario ids must be non-empty strings") | |
| if scenario_id in seen: | |
| raise ValueError(f"Harness E2E scenario id repeats {scenario_id}") | |
| if not isinstance(group, str) or group not in {"Quality", "Operations", "Validation"}: | |
| raise ValueError(f"{scenario_id}: unsupported Harness E2E group") | |
| seen.add(scenario_id) | |
| scenarios.append({"id": scenario_id, "group": group}) | |
| profiles = _mapping(config.get("profiles"), "harness_e2e.profiles") | |
| release = _mapping(profiles.get("release"), "harness_e2e.profiles.release") | |
| release_scenarios = _string_list( | |
| release.get("scenarios"), "harness_e2e.profiles.release.scenarios" | |
| ) | |
| unknown = [scenario for scenario in release_scenarios if scenario not in seen] | |
| if unknown: | |
| raise ValueError(f"release profile references unknown scenarios: {', '.join(unknown)}") | |
| full = _mapping(profiles.get("full"), "harness_e2e.profiles.full") | |
| if full.get("scenarios") != "all": | |
| raise ValueError("harness_e2e.profiles.full.scenarios must be all") |
🤖 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/scripts/harness_e2e_profiles.py around lines 55 - 94, Update
load_profile_catalog to validate release_control, harness_e2e, profiles,
release, and full as mappings before calling .get(), raising the existing
documented ValueError for malformed catalog objects. Also require each scenario
group to be a string before checking membership in the allowed group set, and
add tests covering scalar/list mappings and non-string group values.
| resolved=$(python3 .github/scripts/harness_e2e_profiles.py \ | ||
| --available-json "$available" \ | ||
| --profile "$VALIDATION_PROFILE" \ | ||
| --scenarios-json "$REQUESTED_SCENARIOS" \ | ||
| --catalog-sha "${{ github.sha }}" \ | ||
| --expected-catalog-sha "$EXPECTED_CATALOG_SHA") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Record the checked-out catalog revision.
When inputs.source_ref selects a commit other than the workflow event SHA, this step reads that commit’s catalog but emits ${{ github.sha }} as catalog_sha. The evidence then claims a catalog revision that the suite did not use. Resolve HEAD after checkout and pass that value to the profile resolver.
Proposed fix
set -euo pipefail
available=$(harness/target/release/harness-e2e list)
+ catalog_sha=$(git rev-parse HEAD)
resolved=$(python3 .github/scripts/harness_e2e_profiles.py \
--available-json "$available" \
--profile "$VALIDATION_PROFILE" \
--scenarios-json "$REQUESTED_SCENARIOS" \
- --catalog-sha "${{ github.sha }}" \
+ --catalog-sha "$catalog_sha" \
--expected-catalog-sha "$EXPECTED_CATALOG_SHA")📝 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.
| resolved=$(python3 .github/scripts/harness_e2e_profiles.py \ | |
| --available-json "$available" \ | |
| --profile "$VALIDATION_PROFILE" \ | |
| --scenarios-json "$REQUESTED_SCENARIOS" \ | |
| --catalog-sha "${{ github.sha }}" \ | |
| --expected-catalog-sha "$EXPECTED_CATALOG_SHA") | |
| set -euo pipefail | |
| available=$(harness/target/release/harness-e2e list) | |
| catalog_sha=$(git rev-parse HEAD) | |
| resolved=$(python3 .github/scripts/harness_e2e_profiles.py \ | |
| --available-json "$available" \ | |
| --profile "$VALIDATION_PROFILE" \ | |
| --scenarios-json "$REQUESTED_SCENARIOS" \ | |
| --catalog-sha "$catalog_sha" \ | |
| --expected-catalog-sha "$EXPECTED_CATALOG_SHA") |
🤖 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/_harness-e2e.yml around lines 336 - 341, Update the
profile resolver invocation assigning resolved in the workflow to resolve the
checked-out repository HEAD after checkout and pass that revision as catalog_sha
instead of the event-based github.sha value, ensuring evidence identifies the
catalog actually used.
| | Harness E2E Main | Relevant push to `main` | 1 per subject/scenario | Score advisory; empty reports, hard-gate and technical failures blocking | | ||
| | Harness E2E Daily | Daily at 06:00 UTC, or manual dispatch on `main` | 3 per subject/scenario | Score is advisory; empty reports, hard-gate and technical failures are blocking; history is always published | | ||
| | Harness E2E deployed | Successful post-release smoke for Harness or a mandatory dependency | 1 per subject/scenario | Score advisory; empty reports, hard-gate and technical failures blocking | | ||
| | Harness E2E deployed | Successful post-release smoke or Release Control dispatch | 1 per selected subject/scenario | Score advisory; empty reports, hard-gate and technical failures blocking; promotion requires the release gate | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the deployed-E2E trigger documentation.
The table permits a Release Control dispatch. The following paragraph states that only release smoke dispatches deployed E2E. Update that paragraph to document both trigger paths and retain the promotion-gate requirement.
🤖 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 `@harness/tests/e2e/README.md` at line 223, Update the paragraph following the
“Harness E2E deployed” table entry to state that deployed E2E can be triggered
by either a successful post-release smoke or a Release Control dispatch, while
preserving the requirement that promotion must pass the release gate.
1 participant
Summary
Why
Release orchestration currently depends on duplicated worker lists and inferred run identity. Mutable Registry and GHCR surfaces can also diverge when a later publication step fails, while retries do not clearly distinguish safe reconciliation from creating a new version.
This change gives Release Control stable inputs and outputs, makes partial releases observable and repairable, and keeps the higher Harness validation policy scoped to Harness itself.
Compatibility and rollout
Existing dispatch inputs remain accepted and contract-v1 tags and candidate evidence remain readable. Stable workers may still publish directly to
latest, but Harness now requiresnext -> deployed E2E -> promote.Release Control should migrate its Harness direct-latest path before these workflows are enabled on
main.This PR supersedes the release changes in #714.
Verification
git diff --checkRefs MOT-4299
Summary by CodeRabbit
New Features
Bug Fixes
Documentation