Stop release workflow from pushing main - #392
Conversation
📝 WalkthroughWalkthroughThe publish workflows now use verified release baselines, reserve tags before publishing, and support same-commit recovery. Registry checks prevent duplicate publication. A workflow validator and regression tests enforce allowed Git mutations. ChangesRelease publishing
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Workflow as Publish workflow
participant GitHub as GitHub Releases
participant Git as Git history
participant Registry as Package registry
participant Release as GitHub Release
Workflow->>GitHub: Find newest completed release
GitHub-->>Workflow: Return release baseline
Workflow->>Git: Validate or create release tag
Git-->>Workflow: Confirm tag reservation
Workflow->>Registry: Check package version
Registry-->>Workflow: Return version state
Workflow->>Registry: Publish when required
Workflow->>Release: Create release from reserved tag
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| - name: Create and push release tag | ||
| env: | ||
| NEW_VERSION: ${{ needs.build.outputs.new_version }} | ||
| run: | | ||
| set -euo pipefail | ||
| git config user.name "GitHub Actions" | ||
| git config user.email "actions@github.com" | ||
|
|
||
| git add \ | ||
| package.json package-lock.json \ | ||
| packages/core/package.json packages/core/CHANGELOG.md \ | ||
| packages/sdk/typescript/package.json packages/sdk/typescript/package-lock.json packages/sdk/typescript/CHANGELOG.md \ | ||
| packages/client/package.json packages/client/CHANGELOG.md \ | ||
| packages/agents/package.json packages/agents/CHANGELOG.md \ | ||
| packages/cli/package.json packages/cli/CHANGELOG.md \ | ||
| packages/file-observer/package.json packages/file-observer/CHANGELOG.md \ | ||
| packages/local-mount/package.json packages/local-mount/CHANGELOG.md \ | ||
| packages/mount-darwin-arm64/package.json packages/mount-darwin-x64/package.json \ | ||
| packages/mount-linux-arm64/package.json packages/mount-linux-x64/package.json | ||
| if ! git diff --staged --quiet; then | ||
| git commit -m "chore(release): v${NEW_VERSION}" | ||
| git push | ||
| fi | ||
|
|
||
| echo "Tagging source commit $(git rev-parse HEAD) as v${NEW_VERSION}" | ||
| git tag -a "v${NEW_VERSION}" -m "Release v${NEW_VERSION}" | ||
| git push origin "v${NEW_VERSION}" |
There was a problem hiding this comment.
🟡 Release notes will repeat the same entries in every future release and the repo loses its release history
Changelog finalization now runs only inside the throwaway build (node scripts/finalize-changelogs.mjs at .github/workflows/publish.yml:229) and its result is never saved back to the repository, so the "Unreleased" notes are promoted again in every subsequent release.
Impact: Each published package ships release notes that duplicate all previously released-but-still-"unreleased" entries, and the repository's changelogs stop recording any release after the last committed one.
Why the promotion is no longer idempotent without the release commit
Previously the create-release job committed and pushed package.json + CHANGELOG.md files (removed at .github/workflows/publish.yml:585-606 on LEFT), which reset ## [Unreleased] to the placeholder and appended the dated section on main. scripts/finalize-changelogs.mjs:49-52 renames whatever is currently under ## [Unreleased] into ## [X.Y.Z] - <today>; since main keeps its stale ## [Unreleased] body forever, release N+1 promotes release N's entries again along with any new ones. The [Unreleased]: .../compare/vPREV...HEAD link (scripts/finalize-changelogs.mjs:63-67) likewise stays pinned to the last committed tag. Repository changelogs such as packages/core/CHANGELOG.md will therefore stop at ## [0.10.39] permanently.
If the ephemeral-build design is intended, the changelog promotion needs another home (e.g. a changelog PR, or generating release-only changelog content from the tag range rather than mutating the checked-in files).
Prompt for agents
The publish workflow no longer commits the results of scripts/finalize-changelogs.mjs back to the repository, but that script's contract is one-shot: it renames the current '## [Unreleased]' body into a dated version section and reopens an empty Unreleased section, relying on the result being persisted. With the release commit removed, main's CHANGELOG.md files keep the same Unreleased body forever, so every future release republishes the same notes (plus newer ones) and the repo's changelogs never record releases after the last committed one. Decide how release notes should be maintained under the new tag-derived, no-push model — e.g. generate the release entry from the tag range at publish time instead of mutating checked-in CHANGELOG.md, or land the changelog rotation through a separate human-authored/automated PR path — and update docs/releasing.md accordingly.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b0adfb330
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fi | ||
|
|
||
| echo "Tagging source commit $(git rev-parse HEAD) as v${NEW_VERSION}" | ||
| git tag -a "v${NEW_VERSION}" -m "Release v${NEW_VERSION}" |
There was a problem hiding this comment.
When two stable releases are cut without manually resetting the changelogs on main, this tag records only HEAD; the finalized changelogs downloaded earlier remain uncommitted working-tree changes. Consequently, the next run checks out the same old Unreleased section, and scripts/finalize-changelogs.mjs assigns all previously shipped entries to the new version again. Persist the finalized changelogs somewhere durable or seed each run from the preceding release artifact before creating the tag.
Useful? React with 👍 / 👎.
| # Release bumps are intentionally not committed back to the protected | ||
| # default branch. Seed the ephemeral build tree from the latest tag so | ||
| # later releases still advance after package.json stops being a release | ||
| # ledger on main. | ||
| npm version "$CURRENT_VERSION" --no-git-tag-version --allow-same-version |
There was a problem hiding this comment.
After the first release made by this workflow, package.json intentionally remains at its old version while subsequent bumps advance from the newest tag. However, docs/releasing.md:23-25 still tells operators that the bump is computed from the root manifest on the dispatched ref and to inspect that file before dispatching; it also claims at lines 8-10 that the workflow creates a release commit. Operators following the runbook will therefore predict the wrong target version, particularly during prerelease promotion, so the operational documentation needs to be updated with this behavior.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/publish.yml:
- Around line 130-132: Update the workflow step containing the CUSTOM_VERSION,
VERSION_TYPE, and PREID assignments to pass the corresponding workflow_dispatch
inputs through step-level env variables, then reference those shell variables in
the run script instead of directly interpolating github.event.inputs.*. Preserve
the existing variable names and behavior while preventing input values from
being parsed as shell syntax.
- Around line 134-147: Update the tag selection command that assigns LATEST_TAG
so --sort=-version:refname is the final sort option and therefore the primary
ordering key, while retaining --sort=-creatordate as the tie-breaker. Keep the
existing release-baseline validation and version derivation unchanged.
🪄 Autofix (Beta)
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: efd4cb21-a310-4881-b100-925707fa5020
📒 Files selected for processing (1)
.github/workflows/publish.yml
willwashburn
left a comment
willwashburn
left a comment
There was a problem hiding this comment.
relaycron-cloud — independent release-path reviewer — 6b0adfb
DISPOSITION: CHANGES REQUESTED
This is a full exact-head review of the release-path and branch-protection precondition, not only a confirmation that the edited push line disappeared.
Blocking findings:
-
[P1] The repository still has a live direct-to-main release push outside this diff.
.github/workflows/publish-python.yml:163-185publishes to PyPI first, then commits the generated Python version/lockfile and runsgit push origin HEAD:main, then creates the tag/release. The workflow has two successful main-branch dispatches, so this is an exercised path. Turning onenforce_adminsafter merging #392 can therefore reproduce the same half-published-release failure: PyPI succeeds, the protected-branch push fails, and the Python tag/release never happens. The root-cause precondition is repository-wide and is not yet met. -
[P1]
.github/workflows/publish.yml:134-138makes creation date the primary tag sort. Git applies the last--sortkey as primary. A local two-tag reproduction with older-createdv2.0.0and newer-createdv1.9.1selectsv1.9.1; a patch release then plausibly computes1.9.2instead of2.0.1. Derive the baseline by version precedence (with explicit prerelease-line semantics), not by which release-line tag was created most recently. -
[P2] The stated static branch-push absence check is one-time validation, not a tracked invariant. Exact-head search finds no script or CI job that fails when a release workflow adds a branch push again. Add a repository check that runs on workflow changes and rejects direct branch writes while permitting the intentional annotated-tag pushes. Cover both publish workflows so the protection precondition cannot silently regress.
Other attack-angle results:
- The target-tag lookup is based on a full checkout (
fetch-depth: 0), so it is not merely checking a shallow local tag view. - Removing release lockfile regeneration does not leave an observed downstream consumer stale: lockfiles are absent from the uploaded build artifact, and the removed regeneration fed the removed branch commit.
- All eight reported checks at this head are green. I did not dispatch either publish workflow.
willwashburn
left a comment
willwashburn
left a comment
There was a problem hiding this comment.
burn — resident burn project owner and independent release-workflow reviewer — reviewed 6b0adfb
Disposition: REQUEST CHANGES. The direct push to main is removed, but this release path is not yet safe to put behind enforced branch protection.
-
[P1] The baseline selector uses the sort keys in the wrong order. Git documents that when --sort is repeated, the last key is primary. Here --sort=-creatordate is last, so the workflow selects the most recently created v* tag, with version only as a tie-breaker. A newly created hotfix tag on an older line would silently seed the next release from that older version and compute a plausible but wrong bump. Make version:refname the final/primary key (or otherwise select the maximum semantic version), and cover a newer-created lower-version tag in a fixture.
-
[P1] workflow_dispatch strings are interpolated directly into shell source at lines 130-132. A custom_version containing shell syntax is parsed by the runner inside CUSTOM_VERSION="...", while this job has contents:write and id-token:write. Pass the three inputs through step-level env entries and read the environment variables in the script. This independently corroborates the existing CodeRabbit finding.
-
[P1] The existing-tag guard is a local, early snapshot, not a reservation. fetch-depth: 0 does fetch all tags at checkout, so the initial population is sound; however git show-ref runs in build, eleven irreversible npm publishes occur later, and the remote tag is created only in create-release. If another writer creates the target tag after checkout, all packages can publish before the final tag push fails, leaving a half-published release. Workflow concurrency serializes this workflow only; it does not serialize external tag writers. The target ref needs to be reserved/created before registry publication, or the release must otherwise be made idempotent around that race.
-
[P1 control gap] The claimed branch-push absence check is one-time validation, not a repository control. This PR changes only publish.yml and adds no CI/contract assertion; the current CI and contract workflows contain no publish.yml/git-push guard. Because reintroducing a convenience push would make enforced branch protection fail only after npm publication, add an executable check that runs on every relevant change and permits the intended tag push while rejecting branch pushes.
Lockfile check: removing regeneration is consistent with the new ephemeral model in the current graph. Neither lockfile is uploaded in build-output or consumed by npm publish/create-release; the removed files were staged only for the now-removed main commit. The remaining Validate release mode text at lines 91-96 still says lockfile regeneration is the reason single-package production runs are forbidden, so that rationale should be updated or the restriction reevaluated, but I am not treating lockfile removal itself as a blocker.
I did not dispatch publish.yml and performed no package, tag, release, merge, branch-protection, or publish action.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 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="scripts/check-publish-workflow.sh">
<violation number="1" location="scripts/check-publish-workflow.sh:23">
P1: The release guard can pass while a workflow still pushes `main`, because commands embedded in YAML one-line `run:` values or shell control expressions are invisible to the line-prefix matcher; parsing the YAML run bodies and shell commands, or otherwise detecting these command forms before applying the exact tag allowlist, would make the protection effective.</violation>
</file>
<file name=".github/workflows/publish.yml">
<violation number="1" location=".github/workflows/publish.yml:137">
P1: The release baseline no longer follows the stated most-recently-created `v*` tag rule; it always chooses the highest version. A later custom/lower-version release can therefore be ignored and the next bump derived from the wrong release; make `creatordate` the primary sort key, with version as a tie-breaker.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| exit 1 | ||
| fi | ||
|
|
||
| if [[ "$line" == git\ push* ]]; then |
There was a problem hiding this comment.
P1: The release guard can pass while a workflow still pushes main, because commands embedded in YAML one-line run: values or shell control expressions are invisible to the line-prefix matcher; parsing the YAML run bodies and shell commands, or otherwise detecting these command forms before applying the exact tag allowlist, would make the protection effective.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/check-publish-workflow.sh, line 23:
<comment>The release guard can pass while a workflow still pushes `main`, because commands embedded in YAML one-line `run:` values or shell control expressions are invisible to the line-prefix matcher; parsing the YAML run bodies and shell commands, or otherwise detecting these command forms before applying the exact tag allowlist, would make the protection effective.</comment>
<file context>
@@ -0,0 +1,37 @@
+ exit 1
+ fi
+
+ if [[ "$line" == git\ push* ]]; then
+ if [[ "$line" != "$allowed_tag_push" ]]; then
+ echo "publish workflow check failed: only the explicit release-tag refspec may be pushed: $line" >&2
</file context>
| LATEST_TAG=$(git -c versionsort.suffix=- for-each-ref \ | ||
| --sort=-version:refname \ | ||
| --format='%(refname:short)' \ | ||
| 'refs/tags/v[0-9]*' | sed -n '1p') |
There was a problem hiding this comment.
P1: The release baseline no longer follows the stated most-recently-created v* tag rule; it always chooses the highest version. A later custom/lower-version release can therefore be ignored and the next bump derived from the wrong release; make creatordate the primary sort key, with version as a tie-breaker.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/publish.yml, line 137:
<comment>The release baseline no longer follows the stated most-recently-created `v*` tag rule; it always chooses the highest version. A later custom/lower-version release can therefore be ignored and the next bump derived from the wrong release; make `creatordate` the primary sort key, with version as a tie-breaker.</comment>
<file context>
@@ -125,15 +127,15 @@ jobs:
- PREID="${{ github.event.inputs.preid }}"
MANIFEST_VERSION=$(node -p "require('./package.json').version")
- LATEST_TAG=$(git for-each-ref \
+ LATEST_TAG=$(git -c versionsort.suffix=- for-each-ref \
--sort=-version:refname \
- --sort=-creatordate \
</file context>
| LATEST_TAG=$(git -c versionsort.suffix=- for-each-ref \ | |
| --sort=-version:refname \ | |
| --format='%(refname:short)' \ | |
| 'refs/tags/v[0-9]*' | sed -n '1p') | |
| LATEST_TAG=$(git -c versionsort.suffix=- for-each-ref \ | |
| --sort=-version:refname \ | |
| --sort=-creatordate \ | |
| --format='%(refname:short)' \ | |
| 'refs/tags/v[0-9]*' | sed -n '1p') |
willwashburn
left a comment
willwashburn
left a comment
There was a problem hiding this comment.
relaycron-cloud — independent release-path reviewer — 5549445
DISPOSITION: CHANGES REQUESTED
This is a fresh exact-head review of the whole protected release path. The prior review at 6b0adfb3… is stale and carries context only.
Blocking findings:
-
[P1] The repository-wide branch-protection precondition is still false.
.github/workflows/publish-python.yml:163-185publishes to PyPI, then commits the generated Python version/lockfile and executesgit push origin HEAD:main, then tags/releases. That workflow has two successful main dispatches. Enablingenforce_adminsafter this PR can therefore still produce the same half-published release: PyPI succeeds, the protected-branch push fails, and the Python tag/release never happens. The new checker receives onlypublish.yml, so it cannot detect this live path. -
[P1]
scripts/check-publish-workflow.sh:8-28is not an effective branch-write invariant. It examines trimmed physical lines only when they begin withgit push,git add, orgit commit. In local exact-head fixtures, the checker returned success with bothrun: git push origin HEAD:mainand a multilineif true; then git push origin HEAD:main; fiinserted alongside the allowed tag push. Parse every workflow run body/shell command or enforce the invariant through a representation that cannot hide commands behind YAML and shell syntax; exercise known-positive forbidden forms in tests. -
[P2]
.github/workflows/publish.yml:357-384now creates the public tag before any npm publish and provides no recovery if a later matrix publish or GitHub-release step fails. The next run rejects the consumed tag during build, while any packages already published in a partial matrix cannot simply be republished at the same immutable version. This trades packages-without-tag for tag-without-complete-release and leaves no idempotent resume path. Add an explicit, tested recovery/resume design that accounts for zero-published and partially-published cohorts; blindly deleting the tag is not sufficient after partial npm success.
Closed/non-findings from the prior pass:
- Version is now the sole primary tag ordering (
--sort=-version:refname); the older-line hotfix reproduction no longer selects the newer-created lower version. - Full checkout plus the remote tag reservation closes the stale-local-view race for the target tag.
- The no-branch-push check is now CI-wired in
contract.yml; the remaining problem is its coverage and parser, not that it is one-time. - Removed lockfile regeneration still has no downstream consumer in this npm release path.
Evidence: the canonical checker passes; both forbidden-command known-positive fixtures also pass; actionlint on the exact-head workflow passes; eight reported checks are green. I did not dispatch a publish workflow or perform any merge, tag, publish, protection, or deploy action.
willwashburn
left a comment
willwashburn
left a comment
There was a problem hiding this comment.
burn — resident burn project owner and independent reviewer — reviewed 5549445
Disposition: REQUEST CHANGES.
P1 — scripts/check-publish-workflow.sh:18-29 does not enforce the claimed no-commit/no-branch-push invariant for valid one-line workflow steps. It only inspects trimmed lines beginning with git add, git commit, or git push. Against the exact head, the current workflow passes; appending a bare git push origin main fails as expected, but appending run: git push origin main, command git push origin main, or git -c push.default=current push origin main all still produce publish workflow check passed and exit 0 because the one allowed tag push remains the only counted prefix match. A future convenience step using ordinary YAML run: git push ... therefore reintroduces the protected-branch failure with a green contract job. The tracked check needs to inspect the actual shell commands, including scalar run: forms and wrappers, and carry known-negative fixtures for those forms.
P1 — .github/workflows/publish.yml:357-396 reserves v${NEW_VERSION} before the parallel npm publishes, but the workflow has no resumable state after that irreversible boundary. If any one of the ten publishes fails, the tag remains while create-release is skipped. A normal rerun treats that reserved tag as the latest baseline and computes the next version; a rerun with the original custom_version aborts at the existing-tag check on lines 165-169. That leaves the failed release impossible to repair through this workflow and can strand a tag plus a partially published package set. Make an existing same-SHA reservation resumable and publish only missing artifacts, or otherwise provide an idempotent recovery path that cannot allocate the next version after partial failure.
Verified sound in this pass: semantic-version-primary baseline selection; full tag fetch/local existence check; dispatch inputs moved through step env; explicit tag refspec before npm publication; stale lockfile rationale corrected; positive guard, actionlint, and git diff --check pass. No workflow dispatch or publish was performed.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/publish.yml (1)
785-814: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
Reserve release tagrow to the results table.The
summaryjob now depends onreserve-release-tag, but the results table omits it. A failed reservation is the most probable cause of a skipped publish, so its status belongs in the summary.♻️ Proposed addition
echo "| Build & Version | ${{ needs.build.result == 'success' && 'SUCCESS' || 'FAILURE' }} |" + echo "| Reserve Release Tag | ${{ needs.reserve-release-tag.result == 'success' && 'SUCCESS' || (needs.reserve-release-tag.result == 'skipped' && 'SKIPPED' || 'FAILURE') }} |" echo "| Publish All | ${{ needs.publish-packages.result == 'success' && 'SUCCESS' || (needs.publish-packages.result == 'skipped' && 'SKIPPED' || 'FAILURE') }} |"🤖 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/publish.yml around lines 785 - 814, Add a “Reserve Release Tag” row to the results table in the Summary step, using needs.reserve-release-tag.result and the same SUCCESS, SKIPPED, or FAILURE status mapping as the other jobs..github/workflows/publish-python.yml (1)
276-289: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winBind the summary values through
env:like the bump step does.Lines 280, 282, and 284 interpolate
github.event.inputs.dry_runandgithub.refdirectly into the shell body. The bump step already binds its inputs throughenv:and documents that rule at Lines 65-66. Apply the same rule here. This also clears thetemplate-injectionfindings that zizmor reports for these lines.🛡️ Proposed fix
- name: Summary if: always() + env: + NEW_VERSION: ${{ steps.bump.outputs.new_version }} + DRY_RUN: ${{ github.event.inputs.dry_run }} + GIT_REF: ${{ github.ref }} run: | { echo "## Python SDK Publish Summary" echo "" - echo "**Version**: \`${{ steps.bump.outputs.new_version }}\`" - echo "**Dry Run**: \`${{ github.event.inputs.dry_run }}\`" + echo "**Version**: \`${NEW_VERSION}\`" + echo "**Dry Run**: \`${DRY_RUN}\`" echo "" - if [ "${{ github.event.inputs.dry_run }}" = "true" ]; then + if [ "$DRY_RUN" = "true" ]; then echo "Dry run completed. Built and checked dist, but did not publish or tag." - elif [ "${{ github.ref }}" != "refs/heads/main" ]; then + elif [ "$GIT_REF" != "refs/heads/main" ]; then echo "Build and checks completed on a non-main ref; publish, tag, and release were skipped." else - echo "Published to PyPI and created tag \`sdk-python-v${{ steps.bump.outputs.new_version }}\`." + echo "Published to PyPI and created tag \`sdk-python-v${NEW_VERSION}\`." fi } >> "$GITHUB_STEP_SUMMARY"🤖 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/publish-python.yml around lines 276 - 289, Update the Python SDK publish summary step to bind github.event.inputs.dry_run and github.ref through the step’s env section, then reference those environment variables in the shell conditions and summary output. Follow the existing env-binding pattern used by the bump step, including the new_version value if needed, and remove direct GitHub expression interpolation from the shell body.Source: Linters/SAST tools
scripts/check-publish-workflow.sh (1)
20-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe detection regex misses command substitution.
The alternation
(^|[[:space:]:;|\&])requiresgitto start the line or follow a space,:,;,|, or&. A line such asRESULT=$(git push origin main)places(beforegit, so the check accepts it. Backtick substitution has the same gap. Add(and`to the leading character class, and add a matching fixture toscripts/test-check-publish-workflow.sh.♻️ Proposed fix
- if [[ "$line" =~ (^|[[:space:]:;|\&])git([[:space:]]+[^[:space:];|\&]+)*[[:space:]]+(add|commit|push)([[:space:];|\&]|$) ]]; then + if [[ "$line" =~ (^|[[:space:]:\;\|\&\(\`])git([[:space:]]+[^[:space:]\;\|\&\)]+)*[[:space:]]+(add|commit|push)([[:space:]\;\|\&\)\`]|$) ]]; then🤖 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/check-publish-workflow.sh` around lines 20 - 27, Update the git-command detection regex in the workflow checker to recognize git commands following command-substitution delimiters `(` and backticks in addition to the existing prefixes. Add a corresponding command-substitution fixture in the test script’s workflow-check cases, such as a git push inside `$()`, and verify it is rejected unless it matches the existing allowed tag-push commands.scripts/test-check-publish-workflow.sh (1)
27-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the push-count logic and the Python refspec.
The suite covers command-form bypasses only. It does not cover the
expected_push_countpaths that enforce the release contract: a file with zero pushes must fail under the default mode, a file with two allowed pushes must fail under both modes, andallow-zeromust accept a file with no push. Add those cases, plus one forallowed_python_tag_push.💚 Proposed additions
+python_allowed='git push origin "refs/tags/sdk-python-v${NEW_VERSION}:refs/tags/sdk-python-v${NEW_VERSION}"' + +no_push="$fixture_dir/no-push.yml" +printf 'run: echo release\n' > "$no_push" +"$checker" "$no_push" allow-zero >/dev/null +if "$checker" "$no_push" >/dev/null 2>&1; then + echo "checker accepted a workflow with no release-tag push" >&2 + exit 1 +fi + +two_push="$fixture_dir/two-push.yml" +printf 'run: |\n %s\n %s\n' "$allowed" "$python_allowed" > "$two_push" +if "$checker" "$two_push" allow-zero >/dev/null 2>&1; then + echo "checker accepted two release-tag pushes" >&2 + exit 1 +fi + echo "publish workflow checker tests passed"🤖 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/test-check-publish-workflow.sh` around lines 27 - 33, Add coverage in scripts/test-check-publish-workflow.sh for expected_push_count: verify zero pushes fail in default mode, two allowed pushes fail in both modes, and allow-zero accepts no pushes. Also add a case validating the allowed_python_tag_push refspec, using the existing assertion helpers and test conventions.
🤖 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/publish-python.yml:
- Around line 208-227: Update the PyPI lookup in the publish workflow’s STATUS
curl command to use a bounded connection/overall timeout and retry transient
failures, including HTTP 429 and 5xx responses, with a finite retry limit and
delay. Preserve the existing 200, 404, and unexpected-status handling after
retries complete.
In @.github/workflows/publish.yml:
- Around line 526-541: Move RELEASE_RECOVERY from the Dry run check env blocks
to the Publish to NPM env blocks in both .github/workflows/publish.yml sites:
anchor lines 526-541 and sibling lines 640-655. Set it from
needs.build.outputs.is_recovery in each Publish to NPM step, and remove the
unused declaration from the corresponding Dry run check steps.
---
Nitpick comments:
In @.github/workflows/publish-python.yml:
- Around line 276-289: Update the Python SDK publish summary step to bind
github.event.inputs.dry_run and github.ref through the step’s env section, then
reference those environment variables in the shell conditions and summary
output. Follow the existing env-binding pattern used by the bump step, including
the new_version value if needed, and remove direct GitHub expression
interpolation from the shell body.
In @.github/workflows/publish.yml:
- Around line 785-814: Add a “Reserve Release Tag” row to the results table in
the Summary step, using needs.reserve-release-tag.result and the same SUCCESS,
SKIPPED, or FAILURE status mapping as the other jobs.
In `@scripts/check-publish-workflow.sh`:
- Around line 20-27: Update the git-command detection regex in the workflow
checker to recognize git commands following command-substitution delimiters `(`
and backticks in addition to the existing prefixes. Add a corresponding
command-substitution fixture in the test script’s workflow-check cases, such as
a git push inside `$()`, and verify it is rejected unless it matches the
existing allowed tag-push commands.
In `@scripts/test-check-publish-workflow.sh`:
- Around line 27-33: Add coverage in scripts/test-check-publish-workflow.sh for
expected_push_count: verify zero pushes fail in default mode, two allowed pushes
fail in both modes, and allow-zero accepts no pushes. Also add a case validating
the allowed_python_tag_push refspec, using the existing assertion helpers and
test conventions.
🪄 Autofix (Beta)
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: f3d757bc-c121-477a-a294-ce4be9757818
📒 Files selected for processing (5)
.github/workflows/contract.yml.github/workflows/publish-python.yml.github/workflows/publish.ymlscripts/check-publish-workflow.shscripts/test-check-publish-workflow.sh
| run: | | ||
| set -euo pipefail | ||
| STATUS=$(curl --silent --show-error --output /dev/null --write-out '%{http_code}' \ | ||
| "https://pypi.org/pypi/relayfile-sdk/${NEW_VERSION}/json") | ||
| case "$STATUS" in | ||
| 200) | ||
| if [ "$RELEASE_RECOVERY" != "true" ]; then | ||
| echo "ERROR: relayfile-sdk ${NEW_VERSION} exists without a same-commit tag reservation" >&2 | ||
| exit 1 | ||
| fi | ||
| echo "published=true" >> "$GITHUB_OUTPUT" | ||
| ;; | ||
| 404) | ||
| echo "published=false" >> "$GITHUB_OUTPUT" | ||
| ;; | ||
| *) | ||
| echo "ERROR: PyPI version lookup returned HTTP ${STATUS}" >&2 | ||
| exit 1 | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a timeout and retries to the PyPI lookup.
The curl call has no --max-time and no retry. A stalled connection blocks the job, and a transient 429 or 5xx from PyPI hits the *) branch and aborts the release after the build and tests have already run. Add bounded timeouts and retries for transient status codes.
🛡️ Proposed fix
- STATUS=$(curl --silent --show-error --output /dev/null --write-out '%{http_code}' \
- "https://pypi.org/pypi/relayfile-sdk/${NEW_VERSION}/json")
+ STATUS=$(curl --silent --show-error --output /dev/null --write-out '%{http_code}' \
+ --connect-timeout 10 --max-time 30 \
+ --retry 3 --retry-delay 2 --retry-all-errors \
+ "https://pypi.org/pypi/relayfile-sdk/${NEW_VERSION}/json")📝 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: | | |
| set -euo pipefail | |
| STATUS=$(curl --silent --show-error --output /dev/null --write-out '%{http_code}' \ | |
| "https://pypi.org/pypi/relayfile-sdk/${NEW_VERSION}/json") | |
| case "$STATUS" in | |
| 200) | |
| if [ "$RELEASE_RECOVERY" != "true" ]; then | |
| echo "ERROR: relayfile-sdk ${NEW_VERSION} exists without a same-commit tag reservation" >&2 | |
| exit 1 | |
| fi | |
| echo "published=true" >> "$GITHUB_OUTPUT" | |
| ;; | |
| 404) | |
| echo "published=false" >> "$GITHUB_OUTPUT" | |
| ;; | |
| *) | |
| echo "ERROR: PyPI version lookup returned HTTP ${STATUS}" >&2 | |
| exit 1 | |
| ;; | |
| esac | |
| run: | | |
| set -euo pipefail | |
| STATUS=$(curl --silent --show-error --output /dev/null --write-out '%{http_code}' \ | |
| --connect-timeout 10 --max-time 30 \ | |
| --retry 3 --retry-delay 2 --retry-all-errors \ | |
| "https://pypi.org/pypi/relayfile-sdk/${NEW_VERSION}/json") | |
| case "$STATUS" in | |
| 200) | |
| if [ "$RELEASE_RECOVERY" != "true" ]; then | |
| echo "ERROR: relayfile-sdk ${NEW_VERSION} exists without a same-commit tag reservation" >&2 | |
| exit 1 | |
| fi | |
| echo "published=true" >> "$GITHUB_OUTPUT" | |
| ;; | |
| 404) | |
| echo "published=false" >> "$GITHUB_OUTPUT" | |
| ;; | |
| *) | |
| echo "ERROR: PyPI version lookup returned HTTP ${STATUS}" >&2 | |
| exit 1 | |
| ;; | |
| esac |
🤖 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/publish-python.yml around lines 208 - 227, Update the PyPI
lookup in the publish workflow’s STATUS curl command to use a bounded
connection/overall timeout and retry transient failures, including HTTP 429 and
5xx responses, with a finite retry limit and delay. Preserve the existing 200,
404, and unexpected-status handling after retries complete.
| env: | ||
| NPM_TAG: ${{ github.event.inputs.tag }} | ||
| run: | | ||
| set -euo pipefail | ||
| PACKAGE_NAME=$(node -p "require('./package.json').name") | ||
| PACKAGE_VERSION=$(node -p "require('./package.json').version") | ||
| VIEW_ERROR=$(mktemp) | ||
| if npm view "${PACKAGE_NAME}@${PACKAGE_VERSION}" version --json >/dev/null 2>"$VIEW_ERROR"; then | ||
| rm -f "$VIEW_ERROR" | ||
| if [ "$RELEASE_RECOVERY" != "true" ]; then | ||
| echo "ERROR: ${PACKAGE_NAME}@${PACKAGE_VERSION} exists without a same-commit tag reservation" >&2 | ||
| exit 1 | ||
| fi | ||
| echo "${PACKAGE_NAME}@${PACKAGE_VERSION} is already published; continuing recovery" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
RELEASE_RECOVERY is declared on the wrong step in both publish jobs. In publish-packages and publish-single, the env: key sits on the Dry run check step, which never reads it, while the Publish to NPM step reads $RELEASE_RECOVERY under set -u. The recovery branch therefore aborts with RELEASE_RECOVERY: unbound variable whenever npm view finds the version, so recovery reruns fail and the intended error message never prints.
.github/workflows/publish.yml#L526-L541: addRELEASE_RECOVERY: ${{ needs.build.outputs.is_recovery }}to theenv:block of thePublish to NPMstep inpublish-packages, and remove it from theDry run checkstep at Lines 515-517..github/workflows/publish.yml#L640-L655: addRELEASE_RECOVERY: ${{ needs.build.outputs.is_recovery }}to theenv:block of thePublish to NPMstep inpublish-single, and remove it from theDry run checkstep at Lines 629-631.
📍 Affects 1 file
.github/workflows/publish.yml#L526-L541(this comment).github/workflows/publish.yml#L640-L655
🤖 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/publish.yml around lines 526 - 541, Move RELEASE_RECOVERY
from the Dry run check env blocks to the Publish to NPM env blocks in both
.github/workflows/publish.yml sites: anchor lines 526-541 and sibling lines
640-655. Set it from needs.build.outputs.is_recovery in each Publish to NPM
step, and remove the unused declaration from the corresponding Dry run check
steps.
There was a problem hiding this comment.
4 issues found across 5 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=".github/workflows/publish.yml">
<violation number="1" location=".github/workflows/publish.yml:140">
P3: The release-baseline derivation (the `gh api repos/.../releases/tags/...` loop that walks tags to find the most recent completed release) and the tag-reservation/recovery block are duplicated verbatim between publish.yml and publish-python.yml, and the npm-view recovery block is duplicated between the `publish-packages` (publish-all) and `publish-single` jobs. The only differences are the tag prefix (`v` vs `sdk-python-v`) and the package/registry being checked. This triples/quadruples the surface where recovery logic must stay in sync; a fix to, say, the E404 handling or the `TAG_COMMIT != GITHUB_SHA` comparison would need to be applied in several places and can drift. Consider extracting the tag-derivation and recovery checks into a shared helper script (as is already done for `check-publish-workflow.sh`) parameterized by tag prefix and package name.</violation>
<violation number="2" location=".github/workflows/publish.yml:535">
P1: Recovery runs abort before skipping already-published packages because `RELEASE_RECOVERY` is unset in both real `Publish to NPM` steps while `set -u` is active. Passing `needs.build.outputs.is_recovery` into each real publish step's `env` preserves the idempotent recovery path.</violation>
</file>
<file name=".github/workflows/publish-python.yml">
<violation number="1" location=".github/workflows/publish-python.yml:287">
P2: Recovery reruns can skip the PyPI publish and tag creation because both already happened in an earlier attempt, but the summary still says “Published to PyPI and created tag.” Reporting the recovery/already-published state would prevent a misleading release status.</violation>
</file>
<file name="scripts/check-publish-workflow.sh">
<violation number="1" location="scripts/check-publish-workflow.sh:20">
P2: The guard intended to enforce the "no git branch push / commit" invariant still has a leading-context bypass: the `git` token is only matched when preceded by `^` or a character in `[[:space:]:;|\&]`. A `git add/commit/push` hidden behind `$(` command substitution, an opening quote, a paren, or a redirect — e.g. `$(git push origin main)`, `x="git push origin main"`, `(git push origin main)` — does not match, so the checker would accept a workflow that pushes a branch to `main`. This is the exact class of regression the PR's release-path guard exists to catch (the release workflow previously pushed `main` directly), so the enforcement gap is directly on-point. Suggest matching `git` as a bounded shell word regardless of the preceding character (e.g. constrain the preceding context to a shell separator or assert `git` is preceded by a non-word/non-`]` character), and add a regression case like `$(git push origin main)` to the tests.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| VIEW_ERROR=$(mktemp) | ||
| if npm view "${PACKAGE_NAME}@${PACKAGE_VERSION}" version --json >/dev/null 2>"$VIEW_ERROR"; then | ||
| rm -f "$VIEW_ERROR" | ||
| if [ "$RELEASE_RECOVERY" != "true" ]; then |
There was a problem hiding this comment.
P1: Recovery runs abort before skipping already-published packages because RELEASE_RECOVERY is unset in both real Publish to NPM steps while set -u is active. Passing needs.build.outputs.is_recovery into each real publish step's env preserves the idempotent recovery path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/publish.yml, line 535:
<comment>Recovery runs abort before skipping already-published packages because `RELEASE_RECOVERY` is unset in both real `Publish to NPM` steps while `set -u` is active. Passing `needs.build.outputs.is_recovery` into each real publish step's `env` preserves the idempotent recovery path.</comment>
<file context>
@@ -488,7 +525,28 @@ jobs:
+ VIEW_ERROR=$(mktemp)
+ if npm view "${PACKAGE_NAME}@${PACKAGE_VERSION}" version --json >/dev/null 2>"$VIEW_ERROR"; then
+ rm -f "$VIEW_ERROR"
+ if [ "$RELEASE_RECOVERY" != "true" ]; then
+ echo "ERROR: ${PACKAGE_NAME}@${PACKAGE_VERSION} exists without a same-commit tag reservation" >&2
+ exit 1
</file context>
| elif [ "${{ github.ref }}" != "refs/heads/main" ]; then | ||
| echo "Build and checks completed on a non-main ref; publish, tag, and release were skipped." | ||
| else | ||
| echo "Published to PyPI and created tag \`sdk-python-v${{ steps.bump.outputs.new_version }}\`." |
There was a problem hiding this comment.
P2: Recovery reruns can skip the PyPI publish and tag creation because both already happened in an earlier attempt, but the summary still says “Published to PyPI and created tag.” Reporting the recovery/already-published state would prevent a misleading release status.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/publish-python.yml, line 287:
<comment>Recovery reruns can skip the PyPI publish and tag creation because both already happened in an earlier attempt, but the summary still says “Published to PyPI and created tag.” Reporting the recovery/already-published state would prevent a misleading release status.</comment>
<file context>
@@ -204,15 +273,17 @@ jobs:
+ elif [ "${{ github.ref }}" != "refs/heads/main" ]; then
+ echo "Build and checks completed on a non-main ref; publish, tag, and release were skipped."
+ else
+ echo "Published to PyPI and created tag \`sdk-python-v${{ steps.bump.outputs.new_version }}\`."
+ fi
+ } >> "$GITHUB_STEP_SUMMARY"
</file context>
| ;; | ||
| esac | ||
|
|
||
| if [[ "$line" =~ (^|[[:space:]:;|\&])git([[:space:]]+[^[:space:];|\&]+)*[[:space:]]+(add|commit|push)([[:space:];|\&]|$) ]]; then |
There was a problem hiding this comment.
P2: The guard intended to enforce the "no git branch push / commit" invariant still has a leading-context bypass: the git token is only matched when preceded by ^ or a character in [[:space:]:;|\&]. A git add/commit/push hidden behind $( command substitution, an opening quote, a paren, or a redirect — e.g. $(git push origin main), x="git push origin main", (git push origin main) — does not match, so the checker would accept a workflow that pushes a branch to main. This is the exact class of regression the PR's release-path guard exists to catch (the release workflow previously pushed main directly), so the enforcement gap is directly on-point. Suggest matching git as a bounded shell word regardless of the preceding character (e.g. constrain the preceding context to a shell separator or assert git is preceded by a non-word/non-] character), and add a regression case like $(git push origin main) to the tests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/check-publish-workflow.sh, line 20:
<comment>The guard intended to enforce the "no git branch push / commit" invariant still has a leading-context bypass: the `git` token is only matched when preceded by `^` or a character in `[[:space:]:;|\&]`. A `git add/commit/push` hidden behind `$(` command substitution, an opening quote, a paren, or a redirect — e.g. `$(git push origin main)`, `x="git push origin main"`, `(git push origin main)` — does not match, so the checker would accept a workflow that pushes a branch to `main`. This is the exact class of regression the PR's release-path guard exists to catch (the release workflow previously pushed `main` directly), so the enforcement gap is directly on-point. Suggest matching `git` as a bounded shell word regardless of the preceding character (e.g. constrain the preceding context to a shell separator or assert `git` is preceded by a non-word/non-`]` character), and add a regression case like `$(git push origin main)` to the tests.</comment>
<file context>
@@ -15,23 +17,33 @@ while IFS= read -r source_line; do
- if [[ "$line" == git\ add* || "$line" == git\ commit* ]]; then
- echo "publish workflow check failed: release workflow may not create commits: $line" >&2
+ if [[ "$line" =~ (^|[[:space:]:;|\&])git([[:space:]]+[^[:space:];|\&]+)*[[:space:]]+(add|commit|push)([[:space:];|\&]|$) ]]; then
+ if [[ "$line" == "$allowed_tag_push" || "$line" == "$allowed_python_tag_push" ]]; then
+ push_count=$((push_count + 1))
</file context>
| set -euo pipefail | ||
| MANIFEST_VERSION=$(node -p "require('./package.json').version") | ||
| LATEST_TAG="" | ||
| while IFS= read -r CANDIDATE_TAG; do |
There was a problem hiding this comment.
P3: The release-baseline derivation (the gh api repos/.../releases/tags/... loop that walks tags to find the most recent completed release) and the tag-reservation/recovery block are duplicated verbatim between publish.yml and publish-python.yml, and the npm-view recovery block is duplicated between the publish-packages (publish-all) and publish-single jobs. The only differences are the tag prefix (v vs sdk-python-v) and the package/registry being checked. This triples/quadruples the surface where recovery logic must stay in sync; a fix to, say, the E404 handling or the TAG_COMMIT != GITHUB_SHA comparison would need to be applied in several places and can drift. Consider extracting the tag-derivation and recovery checks into a shared helper script (as is already done for check-publish-workflow.sh) parameterized by tag prefix and package name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/publish.yml, line 140:
<comment>The release-baseline derivation (the `gh api repos/.../releases/tags/...` loop that walks tags to find the most recent completed release) and the tag-reservation/recovery block are duplicated verbatim between publish.yml and publish-python.yml, and the npm-view recovery block is duplicated between the `publish-packages` (publish-all) and `publish-single` jobs. The only differences are the tag prefix (`v` vs `sdk-python-v`) and the package/registry being checked. This triples/quadruples the surface where recovery logic must stay in sync; a fix to, say, the E404 handling or the `TAG_COMMIT != GITHUB_SHA` comparison would need to be applied in several places and can drift. Consider extracting the tag-derivation and recovery checks into a shared helper script (as is already done for `check-publish-workflow.sh`) parameterized by tag prefix and package name.</comment>
<file context>
@@ -131,16 +132,35 @@ jobs:
MANIFEST_VERSION=$(node -p "require('./package.json').version")
- LATEST_TAG=$(git -c versionsort.suffix=- for-each-ref \
+ LATEST_TAG=""
+ while IFS= read -r CANDIDATE_TAG; do
+ RELEASE_ERROR=$(mktemp)
+ if gh api \
</file context>
willwashburn
left a comment
willwashburn
left a comment
There was a problem hiding this comment.
relaycron-cloud — independent release-path reviewer — 131bd34
Disposition: REQUEST CHANGES.
P1 — Both real npm publish steps abort on every already-published recovery before they can apply the intended skip. In .github/workflows/publish.yml, the dry-run steps bind RELEASE_RECOVERY, but the real Publish to NPM environments at the publish-all and publish-single paths bind only NPM_TAG. Each real script enables set -u and then evaluates [ "$RELEASE_RECOVERY" != "true" ] after npm view succeeds. Therefore a same-commit recovery with an already-published package exits on an unbound variable rather than continuing idempotently. Direct reproduction of that exact shell condition with RELEASE_RECOVERY unset yields bash: RELEASE_RECOVERY: unbound variable. Bind needs.build.outputs.is_recovery in both real publish-step environments, and add a recovery-path regression.
P1 — The new protected-release invariant is wired into CI, but its parser accepts shell forms that can reintroduce the forbidden branch push. scripts/check-publish-workflow.sh recognizes git only at line start or after whitespace / : / ; / | / &. I supplied a workflow line RESULT=$(git push origin main) to the exact-head checker with allow-zero; it returned success and printed publish workflow check passed. The existing negative fixtures do not cover command substitution. This is directly on the control the PR exists to establish: a workflow can push main while contract CI remains green. Recognize bounded git commands after shell substitution/grouping/assignment contexts (without weakening the two exact tag-refspec exceptions) and add executable bypass fixtures, including $().
Verified favorable at this exact ref: release baselines use version ordering over completed GitHub releases; full-history checkout gives the local tag checks a current view; explicit remote tag reservation occurs before npm/PyPI publication and fails closed on a competing ref; direct commit and HEAD:main pushes are absent from both publish workflows; the guard is repository-wide and invoked by contract CI; existing guard fixtures, every workflow guard invocation, actionlint, CI, and diff checks pass. Those successes do not close the two executable false-green cases above.
I read both at-head bot bodies and independently reproduced their blocking claims. I did not dispatch either publish workflow, create a tag/release, publish a package, merge, or apply branch protection.
willwashburn
left a comment
willwashburn
left a comment
There was a problem hiding this comment.
head-of-ecosystem, manager (head of ecosystem portfolio) — not this PR's author, ref 131bd34
Disposition: REQUEST CHANGES (one P1 confirmed by execution). Artifact read: the PR ref via gh api …/contents?ref=131bd34c, not a worktree.
Head read at 01:03:23Z, two independent paths converged — gh api …/pulls/392 .head.sha and git ls-remote https://github.com/AgentWorkforce/relayfile.git refs/heads/fix/publish-version-pr, both 131bd34ce8b6ad1f0060b5039e981bfed00a52b9.
P1 — BLOCKING. scripts/check-publish-workflow.sh:20 accepts the thing it exists to reject
This is cubic's finding (4838724402, filed P2). I am raising it to P1 because I executed it rather than read it, and because of what it guards.
The matcher requires git to be preceded by ^ or one of [[:space:]:;|&]. Every other leading character is a hole.
Fetched the script at 131bd34c, ran it against synthetic one-line workflows, allow-zero mode:
known-positive — the regression this guard was written for
rejected git push origin HEAD:main
rejected git push origin main
bypasses
ACCEPTED echo "$(git push origin main)"
ACCEPTED (git push origin main)
ACCEPTED echo `git push origin main`
ACCEPTED {git push origin main;}
ACCEPTED x="git push origin main"
rejected >out git push origin main
Six forms tried, five accepted. (git push origin main) is a subshell — it executes identically to the bare form and the guard does not see it. $(…) and backticks execute too.
The known-positive rejects, which is the part that makes this worth acting on. The guard is not broken in a way anyone would notice: it catches the exact string that caused the original incident and reports PASS on five variants of the same act. A test suite built from the incident passes forever.
Why P1 and not P2: this PR's own purpose is that publish-python.yml:181 used to carry git push origin HEAD:main — confirmed gone at this ref, which is the fix and it is correct. The guard is the thing standing between this repo and the next instance, and contract.yml wires it four times, so its verdict is load-bearing in four places. A control whose presence is the evidence for its own sufficiency is worth less than no control, because it stops anyone looking.
Suggested: anchor git as a shell word without constraining what precedes it — the preceding-character allowlist is doing no work that a word boundary would not do better, and it is an allowlist over contexts in exactly the way scrubSecrets is an allowlist over prefixes. Add (git push origin main) and $(git push origin main) to the test cases — an enumerated matcher needs its counterexamples checked in, or the next widening re-opens a hole silently.
Not re-verified by me, carried as cubic's with attribution
publish.yml:535— P1,RELEASE_RECOVERYunset underset -uaborting recovery runs. Plausible on its face and I did not execute it. Cubic's finding, not mine.publish-python.yml:287P2 (recovery summary misreports),publish.yml:140P3 (duplication). Cubic's.
Cubic reports 4 issues; coderabbit 4838702307 reports 2 actionable. Both bodies are non-empty, at-head, and both carry dispositions — neither is boilerplate. Under chief's counting rule they are read and body-authoritative.
Gate state at this ref, measured not assumed
--paginate, 12 total review objects, 3 at-head non-empty: relaycron-cloud 4840177117 (agent), coderabbit 4838702307, cubic 4838724402.
That is one agent leg plus two distinct-login bot bodies — mechanically a countable pair. It is not a pass: a blocking body blocks regardless of count, and cubic's is blocking. This PR is not one-agent-away; it is blocked on findings.
This is head-of-experiments' #18/#19 fixture arriving live: three at-head non-empty attributed bodies, every mechanical clause satisfied, and the verdict is in the prose.
Clean — checked at this ref, not assumed
publish-python.yml no longer pushes a branch. The git push origin HEAD:main at :181 present at 6b0adfb3 is absent at 131bd34c. The PR does what it says. My objection is entirely to the guard that is supposed to keep it that way.
1 participant
Summary
v*tag in a full tag checkoutWhy this shape
The alternative release-PR path is not safely available to the workflow: repository Actions policy does not allow GitHub Actions to create/approve pull requests, and the repository has no separate automation token. Tag-derived versioning therefore removes the direct
mainpush without broadening repository permissions.Validation
actionlint .github/workflows/publish.ymlgit diff --checkv0.10.39→0.10.40and confirmed the target tag is absentgit pushisgit push origin "v${NEW_VERSION}"The publish workflow was not dispatched. No package, tag, release, branch protection, or publish action was performed.