Add exact-artifact stable promotion - #69
Conversation
Promote verified candidate bytes without rebuilding, gate publication behind explicit owner approval, and replace source-coded fallback digests with validated stable manifests. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Joseph Yaksich <gitcommit90@users.noreply.github.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change adds a retained-candidate promotion pipeline, guarded Stable publication workflow, promotion and manifest validation libraries, immutable release controls, and fail-closed website handling for validated Stable metadata. ChangesStable release promotion
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant CandidateWorkflow
participant PromoteStableWorkflow
participant GitHub
participant Website
Operator->>CandidateWorkflow: build and test
CandidateWorkflow->>CandidateWorkflow: retain rehearsal and candidate evidence
Operator->>PromoteStableWorkflow: dispatch candidate identifiers and confirmation
PromoteStableWorkflow->>GitHub: verify workflow, artifact, tag, release, and attestation
PromoteStableWorkflow->>GitHub: publish verified tag and Release
Website->>GitHub: download and validate Stable manifest
Website->>Website: retain last validated manifest when GitHub unavailable
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Avoid dynamic regular expressions for owner-supplied version input and cover metacharacter handling. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Joseph Yaksich <gitcommit90@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
scripts/promotion-status.mjs (1)
36-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRead
promotion.jsondirectly instead of spawning Node.Line 37 starts a child Node process to run
readFileSyncon a local file.readFileSyncis available in this module already. The subprocess adds startup cost, and it hides read errors behind the samecatchthat also swallows the git failures.♻️ Proposed refactor
- const promotion = JSON.parse(capture(process.execPath, ["-e", `process.stdout.write(require('fs').readFileSync(${JSON.stringify(resolve(bundleDir, "promotion.json"))},'utf8'))`])); + const promotion = JSON.parse(readFileSync(resolve(bundleDir, "promotion.json"), "utf8"));Add
import { readFileSync } from "node:fs";with the other imports.🤖 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 `@scripts/promotion-status.mjs` around lines 36 - 41, Update the promotion status logic around the promotion JSON parsing to import and use readFileSync directly for promotion.json, removing the spawned Node process while preserving the existing JSON parsing and catch behavior.scripts/github-promotion-gates.mjs (1)
32-41: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
fileURLToPathfor the main-module check.
new URL(\file://${process.argv[1]}`)does not round-trip every path.import.meta.urlpercent-encodes characters such as#and?, so a checkout path that contains them produces a differenthref`. The comparison then fails and the CLI exits silently with status 0. For a gate script, a silent no-op is worse than an error. Compare the resolved paths instead.♻️ Proposed refactor
-if (process.argv[1] && new URL(`file://${process.argv[1]}`).href === import.meta.url) { +import { fileURLToPath } from "node:url"; + +if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {Add
import { resolve } from "node:path";with the other imports.🤖 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 `@scripts/github-promotion-gates.mjs` around lines 32 - 41, Update the main-module check in the CLI entrypoint to compare filesystem paths using fileURLToPath(import.meta.url) and resolve(process.argv[1]), importing fileURLToPath from node:url and resolve from node:path alongside the existing imports. Preserve the current command dispatch and error handling while ensuring paths containing characters such as # or ? still execute the gate.
🤖 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 `@docs/release-lifecycle.md`:
- Around line 142-149: Update Step 4 in the release lifecycle instructions to
remove the manual tag creation and push step. Instruct operators to verify the
intended commit only, leaving tag creation to the protected “Promote exact
candidate to Stable” workflow.
In `@scripts/candidate-promotion-skeleton.mjs`:
- Around line 7-13: Validate the raw environment values before calling resolve,
so missing required paths cannot become the current working directory. In
scripts/candidate-promotion-skeleton.mjs lines 7-13, check
HELM_CANDIDATE_DOWNLOAD, HELM_REHEARSAL_EVIDENCE, and HELM_PROMOTION_OUTPUT
before resolving them; apply the same change to HELM_PROMOTION_BUNDLE,
HELM_PROMOTION_RUN_JSON, and HELM_PROMOTION_ARTIFACT_JSON in
scripts/prepare-promotion-bundle.mjs lines 18-22, preserving the existing
raw-value pattern for HELM_PROMOTION_CI_JSON.
In `@scripts/promotion-lib.mjs`:
- Around line 200-207: Replace the dynamic RegExp construction in the changelog
validation within the promotion flow with a line-wise literal comparison against
the expected version heading, so arbitrary expected.version values cannot throw.
Preserve the existing blocker message and require an exact matching heading line
with the expected date format.
In `@scripts/promotion-status.mjs`:
- Around line 47-50: Reorder the execution in the promotion-status flow so the
report is written to stdout before calling writeVerifiedPromotion when
--write-verified is provided. Preserve the existing resolve(output) argument and
exit-code handling, ensuring blocked reports display their blocker list before
writeVerifiedPromotion can throw.
In `@scripts/publish-promotion.mjs`:
- Around line 76-82: Update the release asset validation loop around
expectedAssets to handle nullable asset.digest values by hashing each asset from
its download URL when the digest is absent. Validate the computed hash against
expected.sha256, and emit distinct errors for malformed digests versus content
mismatches before invoking the release edit command.
In `@scripts/stable-manifest-lib.mjs`:
- Line 47: Normalize options.promotedAt through the same promotion-time
formatting path used by the fallback before validating it in the ISO_TIME check.
Update the logic around the promoted_at assignment and validation so
HELM_PROMOTION_TIME values are normalized consistently, while preserving
rejection of genuinely invalid promotion times.
In `@test/release-governance.mjs`:
- Line 19: Update the assertion in the release governance test to check
checklist, .github/workflows/promote-stable.yml, and
scripts/publish-promotion.mjs separately for --notes-file. Remove the
concatenated input so each individual file must contain the flag.
---
Nitpick comments:
In `@scripts/github-promotion-gates.mjs`:
- Around line 32-41: Update the main-module check in the CLI entrypoint to
compare filesystem paths using fileURLToPath(import.meta.url) and
resolve(process.argv[1]), importing fileURLToPath from node:url and resolve from
node:path alongside the existing imports. Preserve the current command dispatch
and error handling while ensuring paths containing characters such as # or ?
still execute the gate.
In `@scripts/promotion-status.mjs`:
- Around line 36-41: Update the promotion status logic around the promotion JSON
parsing to import and use readFileSync directly for promotion.json, removing the
spawned Node process while preserving the existing JSON parsing and catch
behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f82eab4-dade-4865-a03a-7083fb19ef8f
📒 Files selected for processing (23)
.github/workflows/candidate.yml.github/workflows/promote-stable.ymlCHANGELOG.mddocs/GOVERNANCE.mddocs/release-checklist.mddocs/release-lifecycle.mdpackage.jsonscripts/candidate-promotion-skeleton.mjsscripts/github-promotion-gates.mjsscripts/prepare-promotion-bundle.mjsscripts/promotion-lib.mjsscripts/promotion-status.mjsscripts/publish-promotion.mjsscripts/run-test-suite.mjsscripts/stable-manifest-lib.mjsscripts/verify-promotion-attestation.mjssite/server.mjssite/stable-manifest.jsontest/fixtures/phase3-blocked/promotion.jsontest/phase3-promotion.mjstest/release-governance.mjstest/site-stable-manifest.mjstest/site.mjs
| the manual `Promote exact candidate to Stable` workflow. Supply the exact | ||
| retained candidate workflow run ID, immutable artifact ID, and intended | ||
| version; run its default dry-run first. Never rebuild in promotion, publish | ||
| a subset, or attach a platform later to a | ||
| version already described as complete. Include a directly distributed | ||
| signed Android APK when applicable. Submit iOS through App Store Connect | ||
| rather than publishing an installable IPA as a generic download. Do not use | ||
| GitHub's generated notes as the sole or primary body. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the earlier manual tag step.
Line 128 still tells the operator to create and push the tag before this workflow. The promotion workflow rejects an existing tag. Following this document in order therefore blocks publication and can strand the version.
Change Step 4 so that it verifies the intended commit without creating the tag. The protected promotion workflow must create the 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 `@docs/release-lifecycle.md` around lines 142 - 149, Update Step 4 in the
release lifecycle instructions to remove the manual tag creation and push step.
Instruct operators to verify the intended commit only, leaving tag creation to
the protected “Promote exact candidate to Stable” workflow.
| const source = resolve(process.env.HELM_CANDIDATE_DOWNLOAD || ""); | ||
| const rehearsalPath = resolve(process.env.HELM_REHEARSAL_EVIDENCE || ""); | ||
| const output = resolve(process.env.HELM_PROMOTION_OUTPUT || ""); | ||
| const project = resolve(process.env.HELM_PROJECT_ROOT || "."); | ||
| const workflowRunId = String(process.env.GITHUB_RUN_ID || "unbound"); | ||
| const ciRunId = String(process.env.HELM_CANDIDATE_CI_RUN_ID || ""); | ||
| if (!source || !rehearsalPath || !output) throw new Error("Candidate, rehearsal, and promotion output paths are required"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The required-path guards in both scripts cannot fail. Both scripts call resolve() on the environment value and then test the resolved string for emptiness. resolve("") returns the current working directory, so the resolved value is always a non-empty string and the guard never throws. An unset variable therefore silently targets the working directory, and the script fails later with an unrelated ENOENT. Validate the raw environment value before you resolve it.
scripts/candidate-promotion-skeleton.mjs#L7-L13: check the rawHELM_CANDIDATE_DOWNLOAD,HELM_REHEARSAL_EVIDENCE, andHELM_PROMOTION_OUTPUTvalues, then resolve them.scripts/prepare-promotion-bundle.mjs#L18-L22: check the rawHELM_PROMOTION_BUNDLE,HELM_PROMOTION_RUN_JSON, andHELM_PROMOTION_ARTIFACT_JSONvalues, then resolve them. Line 21 already uses this pattern forHELM_PROMOTION_CI_JSON.
📍 Affects 2 files
scripts/candidate-promotion-skeleton.mjs#L7-L13(this comment)scripts/prepare-promotion-bundle.mjs#L18-L22
🤖 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 `@scripts/candidate-promotion-skeleton.mjs` around lines 7 - 13, Validate the
raw environment values before calling resolve, so missing required paths cannot
become the current working directory. In
scripts/candidate-promotion-skeleton.mjs lines 7-13, check
HELM_CANDIDATE_DOWNLOAD, HELM_REHEARSAL_EVIDENCE, and HELM_PROMOTION_OUTPUT
before resolving them; apply the same change to HELM_PROMOTION_BUNDLE,
HELM_PROMOTION_RUN_JSON, and HELM_PROMOTION_ARTIFACT_JSON in
scripts/prepare-promotion-bundle.mjs lines 18-22, preserving the existing
raw-value pattern for HELM_PROMOTION_CI_JSON.
| const output = option("--write-verified"); | ||
| if (output) writeVerifiedPromotion(report, resolve(output)); | ||
| process.stdout.write(has("--json") ? `${JSON.stringify(report, null, 2)}\n` : formatPromotionReport(report)); | ||
| if (!report.eligible) process.exitCode = 1; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Print the report before you write the verified output.
writeVerifiedPromotion throws for an ineligible report. Line 48 runs before Line 49, so --write-verified on a blocked bundle produces an unhandled exception and the operator never sees the blocker list. Print the report first, then write the publish inputs.
🐛 Proposed fix
const output = option("--write-verified");
-if (output) writeVerifiedPromotion(report, resolve(output));
process.stdout.write(has("--json") ? `${JSON.stringify(report, null, 2)}\n` : formatPromotionReport(report));
+if (output) writeVerifiedPromotion(report, resolve(output));
if (!report.eligible) process.exitCode = 1;📝 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.
| const output = option("--write-verified"); | |
| if (output) writeVerifiedPromotion(report, resolve(output)); | |
| process.stdout.write(has("--json") ? `${JSON.stringify(report, null, 2)}\n` : formatPromotionReport(report)); | |
| if (!report.eligible) process.exitCode = 1; | |
| const output = option("--write-verified"); | |
| process.stdout.write(has("--json") ? `${JSON.stringify(report, null, 2)}\n` : formatPromotionReport(report)); | |
| if (output) writeVerifiedPromotion(report, resolve(output)); | |
| if (!report.eligible) process.exitCode = 1; |
🤖 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 `@scripts/promotion-status.mjs` around lines 47 - 50, Reorder the execution in
the promotion-status flow so the report is written to stdout before calling
writeVerifiedPromotion when --write-verified is provided. Preserve the existing
resolve(output) argument and exit-code handling, ensuring blocked reports
display their blocker list before writeVerifiedPromotion can throw.
| const expectedAssets = [...stable.artifacts, { name: `1Helm-${version}-stable.json`, sha256: hash(stablePath) }]; | ||
| if (!Array.isArray(release.assets) || release.assets.length !== expectedAssets.length) throw new Error("Draft Release asset matrix is incomplete or contains unexpected assets"); | ||
| for (const expected of expectedAssets) { | ||
| const matches = release.assets.filter((asset) => asset?.name === expected.name && asset?.digest === `sha256:${expected.sha256}`); | ||
| if (matches.length !== 1) throw new Error(`Draft Release bytes do not match ${expected.name}`); | ||
| } | ||
| run("gh", ["release", "edit", tag, "--repo", stable.repository, "--draft=false", "--latest"]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
GitHub REST API release asset digest field availability response
💡 Result:
The digest field for GitHub release assets is available in the GitHub REST API and exposes the SHA256 checksum of the asset [1][2]. This field was introduced to provide better integrity and transparency, allowing users to verify that assets have not been altered since publication [2]. In the REST API, the release asset object includes a digest field, which is defined as a string or null [1][3][4]. Similarly, the GitHub GraphQL API also provides a digest field (as a string) on the ReleaseAsset object [5]. The checksum is automatically computed by GitHub at the time of asset upload and is immutable [2]. You can access this information via the Releases REST API, the GraphQL API, the GitHub CLI, and the GitHub Releases UI [2].
Citations:
- 1: https://docs.github.com/en/rest/releases/assets
- 2: https://github.blog/changelog/2025-06-03-releases-now-expose-digests-for-release-assets/
- 3: https://docs.github.com/en/rest/releases/releases
- 4: https://docs.github.com/en/enterprise-server@3.19/rest/releases/assets?apiVersion=2022-11-28
- 5: https://docs.github.com/en/graphql/reference/releases
🏁 Script executed:
sed -n '1,130p' scripts/publish-promotion.mjs
printf '\n--- relevant symbols and call sites ---\n'
rg -n "publish-promotion|release view|release upload|release\.assets|digest|gh release" scripts package.json .github 2>/dev/null | head -200Repository: gitcommit90/1Helm
Length of output: 16412
🏁 Script executed:
printf '%s\n' '--- stable-manifest-lib.mjs ---'
cat -n scripts/stable-manifest-lib.mjs | sed -n '80,140p'
printf '%s\n' '--- promotion workflow around publish ---'
cat -n .github/workflows/promote-stable.yml | sed -n '175,225p'
printf '%s\n' '--- release-related tests/docs ---'
rg -n -C 3 "stable manifest asset|Draft Release|publish-promotion|release assets|digest-qualified|release.*digest" --glob '!node_modules' --glob '!dist' --glob '!build' .Repository: gitcommit90/1Helm
Length of output: 47077
Handle nullable release-asset digests
release.assets[].digest is nullable in the GitHub REST API. If it is null, this check fails after the immutable tag and draft Release are created, which strands the version. Hash each asset from its download URL when digest is absent, and report malformed or mismatched digests separately.
🤖 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 `@scripts/publish-promotion.mjs` around lines 76 - 82, Update the release asset
validation loop around expectedAssets to handle nullable asset.digest values by
hashing each asset from its download URL when the digest is absent. Validate the
computed hash against expected.sha256, and emit distinct errors for malformed
digests versus content mismatches before invoking the release edit command.
| const version = String(value.version || ""); | ||
| const commit = String(value.commit || ""); | ||
| if (!VERSION.test(version) || value.tag !== `v${version}` || !HEX40.test(commit)) refuse("version, tag, or commit is invalid"); | ||
| if (!ISO_TIME.test(String(value.promoted_at || ""))) refuse("promotion time is invalid"); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline scripts --items all --type function,class \
--match 'promotion|manifest|publish'
rg -n -C4 '\bpromoted_at\b|\.toISOString\s*\(' scripts .githubRepository: gitcommit90/1Helm
Length of output: 4889
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- stable manifest validation and timestamp pattern ---'
sed -n '1,75p' scripts/stable-manifest-lib.mjs
printf '%s\n' '--- all stable-manifest construction and validation sites ---'
rg -n -C3 'validateStableManifest|promoted_at|stableManifest|STABLE_MANIFEST_KIND' --glob '*.mjs' --glob '*.js' --glob '*.json' scripts .github
printf '%s\n' '--- producer format check ---'
python3 - <<'PY'
from pathlib import Path
import re
hits = []
for path in Path("scripts").rglob("*"):
if path.is_file() and path.suffix in {".mjs", ".js", ".json"}:
text = path.read_text(errors="replace")
if "promoted_at" in text:
hits.append((str(path), [
line.strip() for line in text.splitlines()
if "promoted_at" in line or "toISOString" in line
]))
for path, lines in hits:
print(path)
for line in lines:
print(" " + line)
source = Path("scripts/promotion-lib.mjs").read_text()
expected = r'options\.promotedAt \|\| new Date\(\)\.toISOString\(\)\.replace\(/\\\.\\d\{3\}Z\$/, "Z"\)'
print("default producer strips milliseconds:", bool(re.search(expected, source)))
PYRepository: gitcommit90/1Helm
Length of output: 10816
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- promotion function and promotedAt callers ---'
rg -n -C8 'function [A-Za-z0-9_$]*Promotion|promotedAt|verifyPromotion|build.*Promotion|promotionReport' scripts --glob '*.mjs' --glob '*.js'
printf '%s\n' '--- command-line or environment sources for promotedAt ---'
rg -n -C5 'PROMOT|promoted.?at|promotedAt' scripts .github package.json README.md 2>/dev/null || trueRepository: gitcommit90/1Helm
Length of output: 30604
Normalize the supplied promotion time. The fallback strips milliseconds, but options.promotedAt from HELM_PROMOTION_TIME bypasses that step and can be rejected by ISO_TIME.
🤖 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 `@scripts/stable-manifest-lib.mjs` at line 47, Normalize options.promotedAt
through the same promotion-time formatting path used by the fallback before
validating it in the ISO_TIME check. Update the logic around the promoted_at
assignment and validation so HELM_PROMOTION_TIME values are normalized
consistently, while preserving rejection of genuinely invalid promotion times.
| assert.match(source, /numbered acceptance\s+ledger/i); | ||
| } | ||
| assert.match(checklist, /--notes-file "\$RELEASE_NOTES"/); | ||
| assert.match(checklist + read(".github/workflows/promote-stable.yml") + read("scripts/publish-promotion.mjs"), /--notes-file/); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Concatenating the three files weakens the assertion.
assert.match on the joined text passes when only one of the three files contains --notes-file. Assert each file separately so the test detects a missing flag in any one of them.
💚 Proposed fix
- assert.match(checklist + read(".github/workflows/promote-stable.yml") + read("scripts/publish-promotion.mjs"), /--notes-file/);
+ for (const source of [checklist, read(".github/workflows/promote-stable.yml"), read("scripts/publish-promotion.mjs")]) {
+ assert.match(source, /--notes-file/);
+ }📝 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.
| assert.match(checklist + read(".github/workflows/promote-stable.yml") + read("scripts/publish-promotion.mjs"), /--notes-file/); | |
| for (const source of [checklist, read(".github/workflows/promote-stable.yml"), read("scripts/publish-promotion.mjs")]) { | |
| assert.match(source, /--notes-file/); | |
| } |
🤖 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 `@test/release-governance.mjs` at line 19, Update the assertion in the release
governance test to check checklist, .github/workflows/promote-stable.yml, and
scripts/publish-promotion.mjs separately for --notes-file. Remove the
concatenated input so each individual file must contain the flag.
Outcome
Phase 3 turns a Stable release into a guarded promotion of already-built, already-tested bytes rather than another rebuild.
Current safety state
Publication remains intentionally blocked until Phase 4 supplies retained Mac artifacts and Mac/Linux/Windows acceptance evidence, and until the protected Stable environment is deliberately configured. This PR cannot release anything by itself.
Verification
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation