Skip to content

follow-up: address CodeRabbit suggestions deferred from #87 #90

@don-petry

Description

@don-petry

Background

PR #87 centralized 4 workflows into reusables. CodeRabbit's review on that PR raised 4 substantive comments that I intentionally deferred — they all flag pre-existing logic in the original templates, and changing them in a centralization PR would have expanded scope and risked behavioral regressions in downstream repos. They're worth fixing now in isolated PRs.

Items

  • agent-shield-reusable.yml — SKILL.md frontmatter regex column-zero anchor
    Around the SKILL.md validation step, the patterns ^name: and ^description: only match keys in column 0. Indented YAML keys (e.g., under metadata:) are missed. CodeRabbit suggested ^[[:space:]]*name: / ^[[:space:]]*description:. Fix is one-line; verify against any SKILL.md files in the org that use indented frontmatter (or keep the strict pattern and document the assumption).

  • dependabot-rebase-reusable.yml — vacuous-truth on empty statusCheckRollup
    The current jq expression [.statusCheckRollup[]? | ... | .conclusion] | all(. == "SUCCESS" or ...) returns true when the array is empty (all on empty list = true). A PR with no status checks at all would be treated as "all checks pass". Fix: also require [.statusCheckRollup[]? | select(.status == "COMPLETED")] | length > 0. Edge case but worth closing.

  • dependency-audit-reusable.yml — cargo audit re-runs per workspace member
    The cargo audit loop iterates every Cargo.toml and runs cargo audit for each. In a workspace, this audits the same lockfile N times (where N = number of crates). Fix: detect [workspace] in Cargo.toml and only audit at workspace roots; for standalone crates not in any workspace, audit those individually.

  • dependency-audit-reusable.yml — pip-audit requirements.txt branch unreachable when pyproject.toml exists
    The current if/elif skips requirements.txt audit whenever pyproject.toml is also present. Some projects have both (pyproject for tooling, requirements for runtime pin). Fix: replace elif with two independent ifs so both are audited when both exist, and OR the failure status.

Why these were deferred

All four are pre-existing logic in the per-repo templates that #87 lifted into reusables verbatim. The whole point of #87 was a no-behavior-change centralization so downstream repos could adopt the stubs without surprises. Each of these fixes shifts behavior, so they belong in their own PR with its own test plan.

Suggested order

Items 3 and 4 are both dependency-audit-reusable.yml so bundle them in one PR. Items 1 and 2 are independent and small — separate PRs each.

See #87 review for original CodeRabbit comments.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions