fix(workflows): address CodeRabbit suggestions deferred from #87#93
fix(workflows): address CodeRabbit suggestions deferred from #87#93
Conversation
Closes #90. 1) agent-shield-reusable.yml — SKILL.md frontmatter regex now allows optional leading whitespace (`^[[:space:]]*name:` / `^[[:space:]]*description:`), so indented YAML keys (e.g. under a `metadata:` parent) are recognised as present. Previously the strict column-zero anchor missed them. 2) dependabot-rebase-reusable.yml — fix vacuous-truth merge gate. `[].statusCheckRollup[]? | ... | all(...)` returns true on an empty list (logical convention), which made a PR with no status checks appear "all green" and trigger an auto-merge. New gate also requires at least one COMPLETED check before merging, in addition to the existing all-pass and zero-pending requirements. Also collapses the three `gh pr view` calls into one round-trip via a shared $ROLLUP. 3) dependency-audit-reusable.yml — cargo audit no longer re-runs per workspace member. The new logic finds workspace roots (Cargo.toml files containing `[workspace]`) and audits them once each, then audits standalone crates whose dir is not under any workspace root. For a workspace with N members, that's 1 audit instead of N+1. 4) dependency-audit-reusable.yml — pip-audit now audits both pyproject.toml AND requirements.txt when both exist in the same directory (some projects ship pyproject for tooling and requirements.txt for pinned runtime deps). Previously the elif branch made requirements.txt unreachable. All four were originally raised by CodeRabbit on #87 and intentionally deferred to keep that PR no-behavior-change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR applies four deferred behavioral fixes to the org’s centralized reusable workflows to improve correctness and reduce unnecessary work in downstream repos.
Changes:
- Improve SKILL.md frontmatter validation to recognize indented YAML keys in
agent-shield-reusable.yml. - Fix Dependabot “merge-ready” gating to avoid vacuous-truth on empty status checks and reduce
gh pr viewcalls. - Optimize
dependency-audit-reusable.ymlby deduplicating Cargo workspace audits and auditing bothpyproject.tomlandrequirements.txtwhen both exist.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/dependency-audit-reusable.yml | Dedup Cargo audits at workspace roots; audit both pyproject and requirements when present. |
| .github/workflows/dependabot-rebase-reusable.yml | Require at least one completed check before treating rollup as passing; consolidate gh pr view calls. |
| .github/workflows/agent-shield-reusable.yml | Relax frontmatter key matching to allow leading whitespace. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| for dir in "${WORKSPACES[@]}" "${STANDALONE[@]}"; do | ||
| echo "::group::cargo audit $dir" | ||
| if ! (cd "$dir" && cargo generate-lockfile 2>/dev/null; cargo audit); then |
There was a problem hiding this comment.
In the cargo audit subshell, cargo audit is separated by ; so it will run even if cd "$dir" fails (and also regardless of cargo generate-lockfile success). This can cause auditing the wrong directory. Chain commands so cargo audit only runs after a successful cd (and decide whether a lockfile generation failure should be fatal or explicitly ignored).
| if ! (cd "$dir" && cargo generate-lockfile 2>/dev/null; cargo audit); then | |
| if ! (cd "$dir" && { cargo generate-lockfile 2>/dev/null || true; } && cargo audit); then |
| mapfile -t WORKSPACES < <( | ||
| find . -name Cargo.toml -not -path '*/target/*' -print0 \ | ||
| | xargs -0 grep -l '^\[workspace\]' 2>/dev/null \ | ||
| | xargs -n1 dirname 2>/dev/null \ | ||
| | sort -u | ||
| ) | ||
|
|
||
| mapfile -t NON_WORKSPACES < <( | ||
| find . -name Cargo.toml -not -path '*/target/*' -print0 \ | ||
| | xargs -0 grep -L '^\[workspace\]' 2>/dev/null \ | ||
| | xargs -n1 dirname 2>/dev/null \ | ||
| | sort -u |
There was a problem hiding this comment.
The workspace discovery pipelines convert NUL-delimited find -print0 output into newline/whitespace-delimited paths before dirname (via grep then xargs -n1 dirname). That will mis-handle Cargo.toml paths containing spaces/tabs/newlines. Consider keeping NUL delimiters end-to-end (e.g., grep -Z + xargs -0) or using find ... -exec sh -c ... to emit workspace/dir names without whitespace splitting.
| mapfile -t WORKSPACES < <( | |
| find . -name Cargo.toml -not -path '*/target/*' -print0 \ | |
| | xargs -0 grep -l '^\[workspace\]' 2>/dev/null \ | |
| | xargs -n1 dirname 2>/dev/null \ | |
| | sort -u | |
| ) | |
| mapfile -t NON_WORKSPACES < <( | |
| find . -name Cargo.toml -not -path '*/target/*' -print0 \ | |
| | xargs -0 grep -L '^\[workspace\]' 2>/dev/null \ | |
| | xargs -n1 dirname 2>/dev/null \ | |
| | sort -u | |
| mapfile -d '' -t WORKSPACES < <( | |
| find . -name Cargo.toml -not -path '*/target/*' -print0 \ | |
| | xargs -0 grep -Z -l '^\[workspace\]' 2>/dev/null \ | |
| | xargs -0 -n1 dirname -z 2>/dev/null \ | |
| | sort -zu | |
| ) | |
| mapfile -d '' -t NON_WORKSPACES < <( | |
| find . -name Cargo.toml -not -path '*/target/*' -print0 \ | |
| | xargs -0 grep -Z -L '^\[workspace\]' 2>/dev/null \ | |
| | xargs -0 -n1 dirname -z 2>/dev/null \ | |
| | sort -zu |
…stub The canonical stub had three spaces aligning the comment after the cron expression. Repos that run prettier in their lint chain (e.g. google-app-scripts) hit a `prettier --check` failure on every fresh adoption — see petry-projects/google-app-scripts#151. Bringing the template in line with prettier defaults so future adopters don't drift.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 43 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Closes #90.
Four small behavioral fixes to the centralized reusable workflows that CodeRabbit raised on #87 and I deferred to avoid expanding scope. Each is independent.
1.
agent-shield-reusable.yml— SKILL.md frontmatter regexAllow optional leading whitespace so indented YAML keys (e.g. under a
metadata:parent) are recognised. Was:^name:/^description:. Now:^[[:space:]]*name:/^[[:space:]]*description:.2.
dependabot-rebase-reusable.yml— vacuous-truth merge gateThe previous
jq | all(...)returnedtrueon an empty status-check rollup, so a PR with no checks at all would be treated as "all green" and auto-merged. New gate also requires at least one COMPLETED check. Also collapsed threegh pr viewcalls into one round-trip.3.
dependency-audit-reusable.yml— cargo workspace dedupFind workspace roots (
Cargo.tomlcontaining[workspace]) and audit each once, then audit standalone crates whose directory is not under any workspace root. For a workspace with N members, this is 1 audit instead of N+1.4.
dependency-audit-reusable.yml— pip-audit pyproject + requirementsAudit both
pyproject.tomlandrequirements.txtwhen both exist in a directory. Previouselifmade the requirements.txt branch unreachable when pyproject was present, but some projects ship pyproject for tooling and requirements.txt for pinned runtime deps.Test plan
actionlintcleanv1tag🤖 Generated with Claude Code