fix(distribution): preserve exact releases and make publication retryable - #283
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: VeraTools/vera/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughThe release workflow accepts a manually selected tag or uses the pushed tag. It supports GitHub-hosted runners for build, release, and Docker jobs. It uses the selected tag for checkout, version extraction, release preparation, publication, and Docker image versioning. ChangesRelease workflow
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Merge Risk: ⚪ Minimal · up to No actionable release-workflow issue is established; the change is ready to merge subject to normal checks. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The recovery path preserves existing publishing permissions, but its newest-tag restriction is only documented: selecting an older tag can roll back shared Docker image tags. Publication also resolves the tag again rather than using one verified commit throughout. These risks depend on trusted release operations; dispatch authorization and tag protections could not be confirmed. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
1292ebc to
b814c8b
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| workflow_dispatch: | ||
| inputs: | ||
| tag: | ||
| description: 'Existing release tag to publish (never moves the tag)' | ||
| required: true | ||
| type: string | ||
| use_github_runners: | ||
| description: 'Use GitHub-hosted runners when Blacksmith is unavailable' | ||
| type: boolean | ||
| default: false |
There was a problem hiding this comment.
🔴 Older recovery replaces current Docker images
Dispatching an older tag makes docker push its binaries to the mutable image tags. Those tags then serve the older release instead of the latest one.
Learn more
The manual dispatch accepts any existing tag, while Build and push and the daemon retry both publish the mutable variant tags. The release job already determines whether the requested tag is latest, but that result is not available to the Docker job. An older release can therefore replace the current images even though its versioned image tags are correct.
Example: After v1.4.2 publishes ghcr.io/veratools/vera:cpu, dispatch v1.4.1 for recovery. Its Docker job republishes :cpu with v1.4.1, so consumers pulling :cpu get the older binary.
Recommended fix: Determine whether RELEASE_TAG is the latest eligible release before Docker publication. Publish mutable variant tags only for the latest release in both the Buildx and daemon paths; continue publishing version-specific tags for older releases.
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/release.yml">
<violation number="1" location=".github/workflows/release.yml:86">
P2: The hosted-runner recovery now uses the historical tag’s Dockerfiles, so a tag created before a Dockerfile or base-image fix cannot use that fix and may fail recovery. Keep this checkout on `${{ github.ref }}`; the downloaded binary remains pinned by `RELEASE_TAG`.</violation>
<violation number="2" location=".github/workflows/release.yml:92">
P2: This regex rejects valid hyphenated prerelease tags such as `v1.2.3-rc-1`, so those releases now fail before building. Allow hyphens in prerelease identifiers.</violation>
<violation number="3" location=".github/workflows/release.yml:93">
P3: The commit comparison in `Verify release tag` is tautological: since `actions/checkout@v4` was told to check out `ref: refs/tags/${{ env.RELEASE_TAG }}`, `HEAD` is by construction the tag's commit, so `git rev-parse HEAD` always equals `refs/tags/...^{commit}`. The check cannot detect a moved tag or an incorrect commit, which is the integrity failure the PR description claims it guards against. Keep the version-shape regex (that is the effective validation) and either drop the equality test or document it as a defense-in-depth check against a checkout malfunction rather than as commit validation.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| ref: refs/tags/${{ env.RELEASE_TAG }} |
There was a problem hiding this comment.
P2: The hosted-runner recovery now uses the historical tag’s Dockerfiles, so a tag created before a Dockerfile or base-image fix cannot use that fix and may fail recovery. Keep this checkout on ${{ github.ref }}; the downloaded binary remains pinned by RELEASE_TAG.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/release.yml, line 86:
<comment>The hosted-runner recovery now uses the historical tag’s Dockerfiles, so a tag created before a Dockerfile or base-image fix cannot use that fix and may fail recovery. Keep this checkout on `${{ github.ref }}`; the downloaded binary remains pinned by `RELEASE_TAG`.</comment>
<file context>
@@ -28,39 +43,54 @@ jobs:
steps:
- uses: actions/checkout@v4
+ with:
+ ref: refs/tags/${{ env.RELEASE_TAG }}
+ fetch-depth: 0
+
</file context>
| - name: Verify release tag | ||
| shell: bash | ||
| run: | | ||
| [[ "$RELEASE_TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]] |
There was a problem hiding this comment.
P2: This regex rejects valid hyphenated prerelease tags such as v1.2.3-rc-1, so those releases now fail before building. Allow hyphens in prerelease identifiers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/release.yml, line 92:
<comment>This regex rejects valid hyphenated prerelease tags such as `v1.2.3-rc-1`, so those releases now fail before building. Allow hyphens in prerelease identifiers.</comment>
<file context>
@@ -28,39 +43,54 @@ jobs:
+ - name: Verify release tag
+ shell: bash
+ run: |
+ [[ "$RELEASE_TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]
+ test "$(git rev-parse HEAD)" = "$(git rev-parse "refs/tags/$RELEASE_TAG^{commit}")"
</file context>
| [[ "$RELEASE_TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]] | |
| [[ "$RELEASE_TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9-]+(\.[a-zA-Z0-9-]+)*)?$ ]] |
| shell: bash | ||
| run: | | ||
| [[ "$RELEASE_TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]] | ||
| test "$(git rev-parse HEAD)" = "$(git rev-parse "refs/tags/$RELEASE_TAG^{commit}")" |
There was a problem hiding this comment.
P3: The commit comparison in Verify release tag is tautological: since actions/checkout@v4 was told to check out ref: refs/tags/${{ env.RELEASE_TAG }}, HEAD is by construction the tag's commit, so git rev-parse HEAD always equals refs/tags/...^{commit}. The check cannot detect a moved tag or an incorrect commit, which is the integrity failure the PR description claims it guards against. Keep the version-shape regex (that is the effective validation) and either drop the equality test or document it as a defense-in-depth check against a checkout malfunction rather than as commit validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/release.yml, line 93:
<comment>The commit comparison in `Verify release tag` is tautological: since `actions/checkout@v4` was told to check out `ref: refs/tags/${{ env.RELEASE_TAG }}`, `HEAD` is by construction the tag's commit, so `git rev-parse HEAD` always equals `refs/tags/...^{commit}`. The check cannot detect a moved tag or an incorrect commit, which is the integrity failure the PR description claims it guards against. Keep the version-shape regex (that is the effective validation) and either drop the equality test or document it as a defense-in-depth check against a checkout malfunction rather than as commit validation.</comment>
<file context>
@@ -28,39 +43,54 @@ jobs:
+ shell: bash
+ run: |
+ [[ "$RELEASE_TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]
+ test "$(git rev-parse HEAD)" = "$(git rev-parse "refs/tags/$RELEASE_TAG^{commit}")"
- name: Install Rust toolchain
</file context>
There was a problem hiding this comment.
3 issues found across 27 reviewed files. 2 files intentionally excluded from review.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/docker_publication.py">
<violation number="1" location="scripts/docker_publication.py:14">
P1: This helper cannot be imported on Python 3.9 because `str | None` is evaluated at definition time and PEP 604 unions require Python 3.10. Use a quoted annotation or `Optional[str]` so the supported Python 3.9 environment can run the release helper.</violation>
</file>
<file name="scripts/release_versions.py">
<violation number="1" location="scripts/release_versions.py:10">
P2: This annotation makes the new release helper fail on Python 3.9 before it can query releases. Quote the union annotation or add `from __future__ import annotations` so the helper keeps the repository's Python 3.9 floor.</violation>
</file>
<file name="packages/python-cli/tests/test_wrapper.py">
<violation number="1" location="packages/python-cli/tests/test_wrapper.py:107">
P3: `read_json` of the self-redirecting URL is expected to fail only after urllib's redirect cap, but `assertRaises(Exception)` would also pass if the request fails earlier for an unrelated reason. Assert `HTTPError` (urllib's `too many redirects` error) or `RuntimeError` instead so the test verifies the redirect bound rather than any failure.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
|
|
||
|
|
||
| def image_digest(image: str) -> str | None: |
There was a problem hiding this comment.
P1: This helper cannot be imported on Python 3.9 because str | None is evaluated at definition time and PEP 604 unions require Python 3.10. Use a quoted annotation or Optional[str] so the supported Python 3.9 environment can run the release helper.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/docker_publication.py, line 14:
<comment>This helper cannot be imported on Python 3.9 because `str | None` is evaluated at definition time and PEP 604 unions require Python 3.10. Use a quoted annotation or `Optional[str]` so the supported Python 3.9 environment can run the release helper.</comment>
<file context>
@@ -0,0 +1,56 @@
+
+
+
+def image_digest(image: str) -> str | None:
+ result = subprocess.run(
+ ["docker", "buildx", "imagetools", "inspect", image, "--format", "{{json .Manifest.Digest}}"],
</file context>
| def image_digest(image: str) -> str | None: | |
| def image_digest(image: str) -> "str | None": |
| import sys | ||
|
|
||
|
|
||
| def newest_stable(releases: list[dict]) -> str | None: |
There was a problem hiding this comment.
P2: This annotation makes the new release helper fail on Python 3.9 before it can query releases. Quote the union annotation or add from __future__ import annotations so the helper keeps the repository's Python 3.9 floor.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/release_versions.py, line 10:
<comment>This annotation makes the new release helper fail on Python 3.9 before it can query releases. Quote the union annotation or add `from __future__ import annotations` so the helper keeps the repository's Python 3.9 floor.</comment>
<file context>
@@ -0,0 +1,32 @@
+import sys
+
+
+def newest_stable(releases: list[dict]) -> str | None:
+ versions = {}
+ for release in releases:
</file context>
| def newest_stable(releases: list[dict]) -> str | None: | |
| def newest_stable(releases: list[dict]) -> "str | None": |
| self.assertEqual(len(self.seen), 2) | ||
|
|
||
| def test_requested_version_never_falls_back_to_latest(self): | ||
| with self.assertRaises(Exception): |
There was a problem hiding this comment.
P3: read_json of the self-redirecting URL is expected to fail only after urllib's redirect cap, but assertRaises(Exception) would also pass if the request fails earlier for an unrelated reason. Assert HTTPError (urllib's too many redirects error) or RuntimeError instead so the test verifies the redirect bound rather than any failure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/python-cli/tests/test_wrapper.py, line 107:
<comment>`read_json` of the self-redirecting URL is expected to fail only after urllib's redirect cap, but `assertRaises(Exception)` would also pass if the request fails earlier for an unrelated reason. Assert `HTTPError` (urllib's `too many redirects` error) or `RuntimeError` instead so the test verifies the redirect bound rather than any failure.</comment>
<file context>
@@ -0,0 +1,285 @@
+ self.assertEqual(len(self.seen), 2)
+
+ def test_requested_version_never_falls_back_to_latest(self):
+ with self.assertRaises(Exception):
+ wrapper.ensure_binary_installed()
+ self.assertEqual(self.seen, ["/releases/download/v1.4.0/release-manifest.json"])
</file context>
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/python-cli/pyproject.toml">
<violation number="1" location="packages/python-cli/pyproject.toml:30">
P3: The file this config references, `release-version.txt`, is generated by release.yml into `packages/python-cli/src/vera_ai_wrapper/` at build time and is not tracked by git or covered by `.gitignore` (git check-ignore confirms). Any local or CI-dev `python -m build` leaves it in the working tree, polluting `git status` and risking an accidental commit of a stale exact tag that `package_version()` would then prefer. Add it to `.gitignore`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| where = ["src"] | ||
|
|
||
| [tool.setuptools.package-data] | ||
| vera_ai_wrapper = ["release-version.txt"] |
There was a problem hiding this comment.
P3: The file this config references, release-version.txt, is generated by release.yml into packages/python-cli/src/vera_ai_wrapper/ at build time and is not tracked by git or covered by .gitignore (git check-ignore confirms). Any local or CI-dev python -m build leaves it in the working tree, polluting git status and risking an accidental commit of a stale exact tag that package_version() would then prefer. Add it to .gitignore.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/python-cli/pyproject.toml, line 30:
<comment>The file this config references, `release-version.txt`, is generated by release.yml into `packages/python-cli/src/vera_ai_wrapper/` at build time and is not tracked by git or covered by `.gitignore` (git check-ignore confirms). Any local or CI-dev `python -m build` leaves it in the working tree, polluting `git status` and risking an accidental commit of a stale exact tag that `package_version()` would then prefer. Add it to `.gitignore`.</comment>
<file context>
@@ -25,3 +25,6 @@ package-dir = {"" = "src"}
where = ["src"]
+
+[tool.setuptools.package-data]
+vera_ai_wrapper = ["release-version.txt"]
</file context>
Release retries could replace published assets, downgrade rolling tags, or block PyPI behind npm. This change preserves existing release assets and versioned images, verifies Docker archives against the exact release manifest, serializes each publication channel, and promotes only the newest published stable release. npm and PyPI now have separate retryable jobs. Manual recovery uses explicit tag checkout and can select GitHub-hosted runners; a nonpublishing mode rehearses all six binary targets and package builds from an exact commit.
The dependency refresh uses Rust 1.88 compatibility fallback. It updates 79 compatible version families, including Tantivy 0.26.2, tree-sitter 0.26.13, reqwest 0.13.5 and console 0.16.6. Release compiler, Zig, build tools, Actions, grammar revisions/checksums, and existing Docker base families are pinned. HCL uses its existing audited Git revision. model2vec, ONNX Runtime/ort, tokenizers and GPU runtime floors remain unchanged because their newer versions require behavior or hardware migration evidence. Tantivy 0.26.2 still uses lru 0.16.4, so its narrowly justified advisory waiver remains, alongside the two existing unmaintained transitive dependency waivers.
Both wrappers now run verified caches offline, select the exact requested release, publish downloads atomically, bound requests, validate archives without unsafe bulk extraction, quote shims safely, and preserve child exit/signal behavior. No runtime dependency was added. Node 18 and Python 3.9 remain supported, with Linux and Windows regression jobs.
Validation: locked workspace tests, all-target Clippy/check, formatting, dependency/security/usage checks; 15 release-tool fixtures and local wrapper fixtures; Actionlint with ShellCheck. The test run exposed a pre-existing process-global hydration counter race; the counter now belongs to its test-only metadata store. The full 1,251-task Semble comparison has exact equality for all 12,510 ordered paths, line spans and scores, with unchanged retrieval metrics; frozen fixture JSON/content/raw scores also match. CI36670853279 passed stable, actual Rust1.88, Windows and Linux/Windows Node18+Python3.9 floors. Nonpublishing rehearsal36670324320 passed all13 jobs and all6 archives: exact manifest hashes/layout verified, GNUglibc2.28 and staticmusl checked, npm package and Python wheel/sdist validated. A final prerelease fix preserves the raw GitHub tag in the Python package despite PyPI normalization; an actual prerelease wheel/sdist build and Twine check pass. Final candidate82382c0 passed all6CI jobs in36671953456, including actualRust1.88 and Linux/Windows runtimefloors.
Release tooling deliberately runs on pinned Python3.11 or the Ubuntu24.04 system Python; its type unions and hashlib.file_digest do not belong to the Python3.9 wrapper runtime. No committed package version bump is introduced. The release helper and workflow accept hyphenated prerelease tags. Docker recovery uses current workflow recipes with the verified historical binary. Push builds compare checkout against the event commit, while publication retries independently reject changed published assets.