Skip to content

5 carried-skill defects surfaced by a downstream PR review (shell-codestyle, repo-worktree, standup-a-repo, merge-and-release) #1089

Description

@ptr727

Symptom

A CodeRabbit review on ptr727/ESPHome-Config#115
(a hub resync that carried the current .github/skills/ tree via scripts/carry.py, hub commit
3258284) raised 5 findings against hub-owned carried skill content that the downstream repo
cannot fix locally without diverging from the hub canonical. Filing per RESYNC.md section 2's
"file defects in the hub" guidance rather than leaving them only in the PR thread.

Findings

  1. shell-codestyle/SKILL.md: set -Eeuo pipefail is mandated for "an extensionless bash/sh
    shebang script", but -E (errtrace) is Bash-only
    and is not defined by POSIX sh, so a
    script invoked via #!/bin/sh (dash, etc.) that opens with this exact header can fail before
    it executes anything. The skill's own scope line names both bash and sh shebangs as
    in-scope, so either the header needs a POSIX-sh variant or the scope should say Bash only.

  2. repo-worktree/SKILL.md's Python pre-commit setup: uv tool install pre-commit places the
    executable in uv's tool bin directory, and the next instruction (pre-commit install) calls it
    by bare name with no PATH check. If that directory isn't on PATH yet, hook setup silently
    stalls. Worth an explicit PATH note or uv tool update-shell mention.

  3. standup-a-repo/SKILL.md: describes repo-config/configure.sh check as a read-only
    drift check, but the live script (confirmed by fetching it from main) sets the same
    FAILED=1 / exit 1 for both an actual configuration mismatch and a guarded gh api failure,
    with no distinct status for the two. A caller following the skill's "apply only after check
    reports drift" guidance can't actually distinguish "there is drift" from "the check itself
    couldn't run" from the exit code alone.

  4. merge-and-release/SKILL.md: two git fetch origin main steps (around what carries as
    lines 62-63 downstream) have no stated behavior on fetch failure. If either fails silently
    (rather than aborting the whole procedure), a later step can read a stale releaseTrigger or
    install stale hub Skills from whatever origin/main last resolved to locally.

  5. merge-and-release/SKILL.md step 6 (run-correlation polling): says to poll "within a
    bounded interval" with no stated duration, cadence, or attempt limit, so a missing/delayed run
    has no defined timeout distinct from the run's eventual conclusion.

Not filed here

  • The headRefOid / git push origin --delete TOCTOU finding on the same file is already tracked
    as Decide whether merge-and-release's branch cleanup should use compare-and-delete #946 (recurring, awaiting a maintainer decision) - not duplicated.
  • CodeRabbit's SkillSpector tool also flagged agent-conduct/SKILL.md, drive-pr/SKILL.md, and
    resync-a-repo/SKILL.md for filing issues "without a confirmation checkpoint" - declined on the
    PR as by-design: routine defect/conformance issue filing is documented, ordinary fleet-agent
    behavior throughout GOVERNANCE.md, not an unreviewed high-impact action.
  • The release|operational shell-pipe misreading on
    operational-vs-release-workflow/references/branch-protection-and-promotion.md:14 was declined
    as a false positive: it's the same placeholder-alternation notation (X|Y meaning "choose one")
    used throughout the doc set, not a literal shell invocation.

Evidence

Full finding text and the scripts/web queries CodeRabbit ran to support items 3-4 are in the PR
review at ptr727/ESPHome-Config#115 (multiple threads on .github/skills/*), run stamp
2026-08-29.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions