feat: implement issue #478 — deploy standard workflows via PRs, not direct pushes#480
Conversation
…irect pushes `deploy-standard-workflows.sh` direct-pushed stubs to each repo's default branch via the Contents API. That 409s on repos whose ruleset enforces required status checks (the Contents API doesn't honor the org-admin ruleset bypass), and bypasses review/CI on the repos where it does succeed — inconsistent with the org's PR/ruleset standards (surfaced during the add-to-project rollout, #415). - Add `scripts/lib/standards-deploy.sh` — a single, sourceable PR-based deploy primitive (`sd_deploy_via_pr`): idempotent open-PR check → create/reuse a sync branch off the default branch → PUT the verbatim file onto the branch → open a labeled PR. Never pushes to the default branch, never merges, never --admin. - Rewire `deploy-standard-workflows.sh` to source the lib and open PRs instead of direct-pushing. Idempotent skip-if-compliant and `--dry-run` preserved (dry-run now reports "Would open PR …"). Header/--help updated; dead `upsert_file` removed. PRs are labeled `standards-sync` and left for the normal review/auto-merge pipeline. - Tests: `test/scripts/lib/standards-deploy.bats` (11 cases — happy path + call sequence, idempotent skip, branch reuse, drifted-update blob SHA, and every failure mode) and `test/scripts/deploy-standard-workflows/dry-run.bats` (2 cases — plan-to-create vs already-compliant), both via a fake `gh` on PATH. - Add `standards-deploy-tests.yml` CI gate (shellcheck + bats) on the tooling. Fixes #478. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the standard workflow deployment process from direct pushes to a PR-based model to prevent failures on protected branches. It introduces a shared deployment utility scripts/lib/standards-deploy.sh along with comprehensive BATS tests. The review feedback highlights several improvement opportunities to increase robustness and portability: separating the PR creation from labeling to prevent failures when labels do not exist, using the optional object index operator in jq to avoid errors on empty arrays, and adopting a more portable, POSIX-compliant method for base64 encoding without line wrapping across different platforms.
| local pr_url | ||
| pr_url=$(gh pr create --repo "$repo" --head "$branch" --base "$default_branch" \ | ||
| --title "$title" --body "$body" --label "$label" 2>/dev/null || true) | ||
| if [ -z "$pr_url" ]; then | ||
| echo "FAILED pr-create-failed" | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
If the specified label (e.g., standards-sync) does not exist in the target repository, gh pr create --label will fail completely, preventing the pull request from being opened.
To make the deployment robust across all repositories (even those where the label hasn't been created yet), it is safer to open the PR first and then apply the label in a separate step, ignoring any failures.
| local pr_url | |
| pr_url=$(gh pr create --repo "$repo" --head "$branch" --base "$default_branch" \ | |
| --title "$title" --body "$body" --label "$label" 2>/dev/null || true) | |
| if [ -z "$pr_url" ]; then | |
| echo "FAILED pr-create-failed" | |
| return 1 | |
| fi | |
| local pr_url | |
| pr_url=$(gh pr create --repo "$repo" --head "$branch" --base "$default_branch" \ | |
| --title "$title" --body "$body" 2>/dev/null || true) | |
| if [ -z "$pr_url" ]; then | |
| echo "FAILED pr-create-failed" | |
| return 1 | |
| fi | |
| if [ -n "$label" ]; then | |
| gh pr edit "$pr_url" --add-label "$label" 2>/dev/null || true | |
| fi |
| existing_pr=$(gh pr list --repo "$repo" --head "$branch" --state open \ | ||
| --json number --jq '.[0].number // ""' 2>/dev/null || true) |
There was a problem hiding this comment.
When there are no open PRs, gh pr list returns an empty array []. Running .[0].number on an empty array causes jq to fail with jq: error: Cannot index null with string "number". Although the error is silenced with 2>/dev/null, it causes the pipeline to exit with a non-zero status.
Using the optional object index operator ? (e.g., .[0].number?) avoids indexing null and prevents jq from throwing an error.
| existing_pr=$(gh pr list --repo "$repo" --head "$branch" --state open \ | |
| --json number --jq '.[0].number // ""' 2>/dev/null || true) | |
| existing_pr=$(gh pr list --repo "$repo" --head "$branch" --state open \ | |
| --json number --jq '.[0].number? // ""' 2>/dev/null || true) |
| local branch_sha encoded | ||
| branch_sha=$(gh api "repos/${repo}/contents/${path}?ref=${branch}" \ | ||
| --jq '.sha // ""' 2>/dev/null || true) | ||
| encoded=$(base64 -w 0 "$local_file" 2>/dev/null || base64 -b 0 "$local_file") |
There was a problem hiding this comment.
The -w 0 (GNU) and -b 0 (BSD/macOS) flags for base64 are platform-specific and can fail on environments with minimal or alternative base64 implementations (such as Alpine Linux or busybox).
A fully portable, POSIX-compliant way to encode a file to base64 without line wrapping is to pipe the file content to base64 and strip newlines using tr -d '\n'.
| encoded=$(base64 -w 0 "$local_file" 2>/dev/null || base64 -b 0 "$local_file") | |
| encoded=$(base64 < "$local_file" | tr -d '\n') |
| } | ||
|
|
||
| @test "dry-run skips a stub that is already compliant" { | ||
| GH_CONTENT_B64="$(base64 -w 0 "$TEMPLATE" 2>/dev/null || base64 -b 0 "$TEMPLATE")" |
There was a problem hiding this comment.
Use the portable base64 < file | tr -d '\n' pattern here as well to ensure the tests run reliably across different platforms (e.g., macOS and Linux CI runners) without relying on platform-specific flags.
GH_CONTENT_B64="$(base64 < "$TEMPLATE" | tr -d '\n')"
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughReplaces the direct Contents API push in ChangesPR-based workflow stub deployment
Sequence Diagram(s)sequenceDiagram
actor Operator
participant deploy_script as deploy-standard-workflows.sh
participant sd_deploy_via_pr as sd_deploy_via_pr (lib)
participant GitHub as GitHub API
Operator->>deploy_script: --workflow add-to-project.yml --repo org/repo
deploy_script->>GitHub: gh api contents/<path> (check existing stub)
deploy_script->>deploy_script: compute sync branch, PR title/body
deploy_script->>sd_deploy_via_pr: repo, path, local_file, branch, label, title, body
sd_deploy_via_pr->>GitHub: gh pr list --head <branch>
alt PR already open
sd_deploy_via_pr-->>deploy_script: SKIP_PR_OPEN <id>
deploy_script-->>Operator: skip (duplicate PR)
else No open PR
sd_deploy_via_pr->>GitHub: gh api repos/<repo> → default branch + base SHA
sd_deploy_via_pr->>GitHub: gh api POST git/refs (create sync branch)
sd_deploy_via_pr->>GitHub: gh api PUT contents/<path> (upload template)
sd_deploy_via_pr->>GitHub: gh pr create --label standards-sync
sd_deploy_via_pr-->>deploy_script: OPENED <url>
deploy_script-->>Operator: ok (PR opened)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
✨ 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 |
… standard (#481) fix(deploy): ensure standards-sync label exists before opening PR; document deploy standard The PR-based deploy (#480) passed `gh pr create --label standards-sync`, which fails outright on any target repo lacking that label — and none of the consumer repos carry it (only .github-private does). Live verification on a ruleset- protected repo confirmed branch+PUT+PR creation works (no 409) but PR creation failed on the missing label. - sd_deploy_via_pr now creates the label if missing (create-if-missing, no --force so an existing curated label is never clobbered) before opening the PR. - Add a regression test asserting the label is ensured before `gh pr create`. - Document the deploy/sync standard in ci-standards.md ("Deploying & syncing stubs"): PR-based via deploy-standard-workflows.sh, standards-sync label, never direct-push / never --admin, with the ruleset-protected-repo rationale. Refs #478, #480. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
Fixes #478. Replaces
deploy-standard-workflows.sh's direct Contents-API push with a PR-based deploy path, extracted into a shared, sourceable primitive.Why: the direct push to each repo's default branch 409s on repos whose ruleset enforces required status checks (the Contents API doesn't honor the org-admin ruleset bypass), and bypasses review/CI on the repos where it does succeed — inconsistent with the org's PR/ruleset standards. Surfaced during the add-to-project rollout (#415): 3 of 7 target repos rejected the direct push and had to be deployed by hand.
Changes
scripts/lib/standards-deploy.sh(new) —sd_deploy_via_pr(): idempotent open-PR check → create/reuse a sync branch off the default branch → PUT the verbatim file onto the branch → open a labeled PR. Never pushes to the default branch, never merges, never--admin. Mirrors the proven branch→PUT→gh pr createsequence already used by.github-private'saw-standards-sync.sh, as a single reusable primitive.scripts/deploy-standard-workflows.sh— sources the lib and opens PRs instead of direct-pushing. Idempotent skip-if-compliant and--dry-runpreserved (dry-run now reportsWould open PR …). Header +--helpupdated; deadupsert_fileremoved. PRs labeledstandards-sync, left for the normal review/auto-merge pipeline.test/scripts/lib/standards-deploy.bats(11 cases) +test/scripts/deploy-standard-workflows/dry-run.bats(2 cases), via a fakeghon PATH. Covers happy-path call sequence, idempotent skip (no mutating calls), branch reuse, drifted-update blob SHA, and every failure mode..github/workflows/standards-deploy-tests.yml(new) — shellcheck + bats gate scoped to the deploy tooling.Validation
bats— 13/13 pass locally.shellcheck --severity=warning -x— clean on both scripts.--dry-run:add-to-project.yml→already compliant×7 +.githubexempt; a drifted stub →Would open PR to update … (branch standards-sync/…).Note
Implemented manually after the dev-lead agent failed to complete #478 (engine error on the first attempt, and the retry path doesn't cover failed initial issue implementations — filed as
petry-projects/.github-private#781). The implementation follows the plan the agent itself posted on #478.Fixes #478.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores