Skip to content

guardrails: shared install-git-hooks copies fail consumer CI (mode 100644, linted vendored scripts) #9426

Open

Description

@kyle-sexton

Problem

/guardrails:setup apply install-git-hooks scope=shared (guardrails 0.55.4, lib/git-hooks/install.mjs copy --scope shared) produces copies that fail a consuming repository's own CI, so every repository adopting the shared entries needs the same manual fix-up. Seen on melodic-software/standards#852:

  1. Executable bit. The installer stages the copies as mode 100644. Seven of them carry a shebang (check-lib.sh, cli-flag-verify.sh, resolve-convention-pattern.sh, run-guards.sh, skill-reference-verify.sh, stale-path-verify.sh, verify-cli-flag.sh), and standards' check-repo-hygiene rejects that: has a shebang but git index mode is 100644; run: git update-index --chmod=+x.
  2. Lint of vendored copies. The repository's ShellCheck lanes (lefthook pre-commit and the ci-workflows shellcheck action) lint .githooks/guardrails/ and fail on the copies (SC2218 and others under that repo's .shellcheckrc). The pilot added .githooks/guardrails/** to the pre-commit exclude and .githooks/guardrails/ to the action's exclude; nothing in plan tells the operator to do this.
  3. Cost of the first commit. guardrails-pre-commit-content scanned the ~10.7k vendored lines for 70 s on the install commit (0.15 s on a normal commit afterwards).

Expected

  • copy stages shebang files as 100755 (or plan returns a decision to).
  • plan returns a decision naming the repository's linters that will see .githooks/guardrails/ (or the copies pass ShellCheck under a default rc), so the exclusion is part of the install, not discovered in CI.
  • pre-commit-content skips its own plugin-owned copies.

Related

Rolling the shared entries out across the melodic-software repositories; each would hit 1 and 2.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Bp5vTm7VLLUuUBumknREY5

Activity

  1. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 23, 2026
  2. github-actions commented on Oct 23, 2026

    @github-actions
    Contributor

    Proposed work class: work-class: scoped
    Basis: automated intake triage; the lanes App applied this class.

    Size: Medium
    Complexity: Medium
    Uncertainty: Known
    Flags: none

    Understood: the guardrails install-git-hooks shared-scope copy step stages shebang scripts as mode 100644, which fails consumer repo hygiene checks. The vendored .githooks/guardrails/ copies are also linted by the consumer's ShellCheck lanes, and the pre-commit-content scan is slow on the install commit because it scans the plugin's own copies. Expected: copy stages shebang files as 100755, plan names the linters that will see the copies, and pre-commit-content skips plugin-owned copies. Nothing essential is missing. I found no duplicates among the open issues.

    Automated triage, applied by the lanes App.

  3. added
    priority: mediumReal value, no hard deadline; normal backlog flow.
    and removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 23, 2026
  4. added
    status: readyTriaged, unblocked, and fully specified; eligible to pick up.
    work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.
    status: needs-decisionAwaiting a human or maintainer judgment call.
    and removed
    status: readyTriaged, unblocked, and fully specified; eligible to pick up.
    on Oct 23, 2026
  5. melodic-automation-lanes commented on Oct 23, 2026

    @melodic-automation-lanes
    Contributor

    Not admitted to the issue lane: it has no - [ ] task list to check. A recorded decision does not change these rules (.github/actions/resolve-config/admission-rules.json); a person who wants it worked adds agent-ready.

  6. kyle-sexton commented on Oct 23, 2026

    @kyle-sexton
    MemberAuthor

    A third failure from the same copies: the repo's check-machine-paths CI step flags the example paths in comments of .githooks/guardrails/machine-path-patterns.sh:41 and hardcoded-path-patterns.sh:37 (they self-match, like a pattern source). The pilot added ':(exclude).githooks/guardrails/**' to that step in standards#852. plan should name this exclusion too, or the comments should use placeholders.

  7. kyle-sexton commented on Oct 23, 2026

    @kyle-sexton
    MemberAuthor

    Two more from the standards#852 pilot:

    1. reference-verify.sh silently skips files over the per-argument limit. It builds each payload with jq -n --arg c "$text" (lines ~123 and ~131 of the 0.55.4 copy). A file whose content passes the Linux per-argument limit (128 KiB) fails with Argument list too long before jq starts, the payload is empty, no finding is produced, and the hook exits 0, so all three reference checks are skipped for that file. Found by the Codex review on the pilot, reproduced on the 238,555-byte hook-utils.sh. Fix: feed the content through stdin or a temp file (jq --rawfile), in both the Write and Edit branches, and fail on a payload-construction error.
    2. No repo-side exemption for a pattern source. In the observation replay of standards' last 100 commits, the only pre-commit-content finding was components/cloud-environment/pages-publish (a scanner whose pattern bodies document /home/<x> and /Users/<x> forms): it self-matches, as the plugin's own pattern libraries do (path_exempt lists them). path_exempt is hard-coded in the copy, so a consumer cannot add its own pattern source without editing a copy. A tracked exemption list read from the repo would cover it.
  8. kyle-sexton commented on Oct 23, 2026

    @kyle-sexton
    MemberAuthor

    More from the rollout in this repository (#9462), guardrails 0.55.4:

    1. cli-flag-verify reads flags inside a quoted prompt. The reference-verify replay of origin/main~200..origin/main reported claude on-demand --scheduled (not in 'claude on-demand --help') for plugins/machine-health/CHANGELOG.md and plugins/machine-health/skills/setup/SKILL.md. The text is claude -p "/machine-health:audit on-demand --scheduled": --scheduled is a skill argument inside the -p prompt string, not a CLI flag. The finding's line number is the file's first changed line (first_line in reference-verify.sh), not the line holding the flag (6 and 2 here, against 29 and 345), so the reader has to search for it.
    2. Item 4 seen live on the install push. The push that adds the copies sends .githooks/guardrails/hook-utils.sh (238,555 bytes) whole, as a new file; reference-verify.sh printed line 125: .../jq: Argument list too long and the job still passed. Every repository's install push skips that file this way.
    3. On-disk mode. copy writes seven shebang copies 0644 on disk as well as in the index; after git update-index --chmod=+x they show as mode changes in git status until chmod +x, and scripts/check-affected.sh here skips every check while they do.
    4. Consumer lint mirrors. This repository's local scripts/check-affected.sh ShellCheck list does not read the workflow's exclude:, so the pre-push job blocks any push that changes the copies; and scripts/affected-tests.sh reports the copies as UNMAPPED. Both are repo-side follow-ups here, noted for the plan decision that names linters (item 2).
  9. kyle-sexton commented on Oct 23, 2026

    @kyle-sexton
    MemberAuthor

    Item 2 follow-up from the dotfiles rollout (dotfiles#1293): a consumer that runs ShellCheck through the ci-workflows pr-run-checks.yml reusable cannot exclude the copies. The reusable's Lint shell scripts step passes no inputs to the shellcheck composite (at ab83b01, v0.39.3, and on ci-workflows main as of 2026-10-23), so .githooks/guardrails/ is linted and fails on SC2218 (run-guards.sh:569, error level), SC2329 (run-guards.sh:744) and SC1091. The repo-side workaround the pilot used (the composite's own exclude:) only reaches a direct call of the composite. Fix options: make the copies ShellCheck-clean under a strict rc (removes the need for any exclusion), or add a shellcheck-exclude input to the reusable.

  10. 1 remaining item

  11. kyle-sexton commented on Oct 23, 2026

    @kyle-sexton
    MemberAuthor

    More from the Codex review threads on the guardrails rollout PRs (copies are plugin-owned and not edited in place):

    1. reference-verify.sh turns the stale-path verifier's advisory findings into push-blocking failures. A document that intentionally records a deletion (for example docs/old.md was deleted) makes the hook exit 1, although the verifier's own output says the deletion record is correct as written. There is no per-finding acknowledgement. Evidence: medley#2206, ci-runner#493, autonomy-demo-scratch#71 (the last is in kyle-sexton). Expected: judgment-dependent findings stay advisory (exit 0), or a scoped acknowledgement lets an author record a valid deletion.
    2. skill-reference-verify.sh finds no manifests in the Codex and Cursor marketplaces, so it never checks references there. It searches only .claude-plugin/plugin.json; those repositories hold .codex-plugin/plugin.json and .cursor-plugin/plugin.json, so the gate exits before checking anything. Evidence: codex-plugins#148, cursor-plugins#126. Expected: manifest discovery and reference resolution cover each marketplace's native manifest layout, or the job is not installed where it cannot run.

    Item 5 (repo-side exemption list), more evidence: ci-runner's worker image ships fixed /home/runner/... paths that the repo's machine-path CI lane already exempts in pr-require-checks.yml, but pre-commit-content rejects them (staging set -- /home/runner/run.sh in worker/entrypoint.sh exits 1): ci-runner#493.

    Item 4 (jq --arg argv limit), more evidence from eight threads, each reproduced on the 238,555-byte hook-utils.sh or a 140-150 KB file: medley#2206, agent-automations#33, .github#211, agent-plugins#108, ci-workflows#870, azure-iac#86, codex-plugins#148, autonomy-demo-scratch#71.

  12. kyle-sexton commented on Oct 23, 2026

    @kyle-sexton
    MemberAuthor

    Found in the melodic-software/knowledge-corpus rollout (guardrails 0.55.4): reference-verify.sh fails open on a large LFS repository when TMPDIR is small.

    When the pushed tip is not the current clean checkout, the hook runs git worktree add --detach "$CHECKS_TMP/tip" "$TIP" with CHECKS_TMP under ${TMPDIR:-/tmp} (check-lib.sh, mktemp -d "${TMPDIR:-/tmp}/checks.XXXXXX"). On WSL with a 4 GB tmpfs /tmp, the LFS smudge of the 4 GB corpus fills it, the worktree add fails with unable to write file ... / Could not reset index file to revision 'HEAD', and the hook prints NOTICE: could not check out <sha>, so it was not checked. and exits 0. The range is not checked at all.

    Possible fixes: create the tip checkout with GIT_LFS_SKIP_SMUDGE=1 (the verifiers read text, not media), or with --no-checkout plus a sparse checkout of the files under check; and treat a failed checkout as a finding rather than a notice.

    Also: the cli-flag finding in that run reported file.md:1 while the text was on line 196.

  13. kyle-sexton commented on Oct 23, 2026

    @kyle-sexton
    MemberAuthor

    Slowness from the melodic-software/knowledge-corpus rollout (#54): replaying pre-commit-content.sh over 51 commits took 2,208 s (standards#852: 20 s for 100 commits), and reference-verify.sh over 50 commits / 672 .md files took 331 s. A normal commit of the 10.7k vendored lines took 67 s. The corpus commits are large transcript and digest imports (up to 5,492 files); per-commit times were not recorded. Expected: seconds per commit.

  14. kyle-sexton commented on Oct 23, 2026

    @kyle-sexton
    MemberAuthor
    1. The commit-msg rejection text names .claude/source-control.md even when the pattern comes from docs/conventions/source-control/commit-convention.yml (standards#855). It should name the file the pattern came from.
  15. kyle-sexton commented on Oct 24, 2026

    @kyle-sexton
    MemberAuthor

    Owner decision recorded for #9504 (2026-10-23 session): guardrails git hooks reach consumer repositories through one lefthook remotes: entry with ref: main and refetch_frequency: 24h, and that is the default for scope=shared with lefthook or no hook manager. Per-repo copies were dropped (they drift each release and need lint exclusions). A pinned tag or a release branch moved by a workflow was considered and rejected as overengineering: main's rulesets and the PR-time end-to-end test are the gate on what consumers run. The README states the trust consequence (whoever can merge to main runs code in every consuming clone within a day).

  16. kyle-sexton commented on Oct 24, 2026

    @kyle-sexton
    MemberAuthor

    Rollout finding from architecture#8: repositories outside the standards sync (architecture, azure-iac, knowledge-corpus, pr-pipeline-sandbox, songwriting, autonomy-demo-scratch) have no docs/conventions/source-control/commit-convention.yml, so each carries a hand copy of the subject regex in .claude/source-control.md ## subject_pattern. That copy restates standards' pr-convention-policy types and drifts when standards changes them. Proposed: resolve-convention-pattern.sh falls back to the owning policy (for example commit-convention.yml fetched from melodic-software/standards, or shipped with the lefthook remote) when the repository has neither source, so non-synced repositories need no copy.

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

    priority: mediumReal value, no hard deadline; normal backlog flow.status: needs-decisionAwaiting a human or maintainer judgment call.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

    Type

    Fields

    No fields configured for Bug.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions