ci(claude): extract inline workflow scripts to .github/scripts/#447
Merged
ci(claude): extract inline workflow scripts to .github/scripts/#447
Conversation
The inline `run: |` blocks in claude-review.yml and claude-mention.yml have grown past the point of being reviewable inside YAML — the log step alone was 142 lines. Pulls them out to standalone bash scripts: - .github/scripts/compose-review-scope.sh (was 36-line inline) - .github/scripts/find-prior-review-comment.sh (was 10-line inline) - .github/scripts/log-review-to-ai-review-log.sh (was 142-line inline) - .github/scripts/parse-claude-mention.sh (was 17-line inline) Each script documents its inputs, outputs, and the rationale for non-obvious mechanics inline. Workflows invoke via `bash .github/scripts/<name>.sh` rather than direct path execution — sidesteps the +x bit being dropped on Windows checkouts and the "forgot to chmod the new script" footgun. The `#!/usr/bin/env bash` shebangs are now informational only. Stacked on top of #444 (marker-based review-comment edit), which introduces the "Find prior review comment" step and the marker'd log-step lookup. Will rebase to main once #444 merges. Tests for these scripts are deliberately out of scope — Nathan's preference per discussion. A separate PR will add coverage via `npm run test:workflows` (or similar) once we settle on the test runner shape. #417 (auth-gate work) carries its own inline scripts and is not touched here. When that branch updates, its scripts adopt the same .github/scripts/ structure as part of that PR's review fixup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
heskew
force-pushed
the
workflow/extract-inline-scripts
branch
from
dd61acc to
6bed023
Compare
kriszyp
pushed a commit
that referenced
this pull request
May 5, 2026
…heck Symptom: harper PR #411 from a real org member was silently skipped — `Claude PR Review` evaluated its job-level `if:` to false and ran zero steps. Cause: GitHub's webhook `author_association` is unreliable. It reports `CONTRIBUTOR` (or `NONE`) for org members with private membership AND for users whose repo access comes via team membership rather than direct collaborator status. Real HarperFast team members fall into both buckets. Forcing visibility changes is hostile UX, and the collaborators API would admit a broader population (read-only collaborators, default-org-permission users) than we want. Fix: two-job pattern with team-membership check. Each workflow has an `authorize` job that runs first, mints an installation token from a HarperFast-org-owned GitHub App (Members:Read scope), and checks team membership. The work/review job has ONE `if: needs.authorize.outputs.authorized == 'true'`. No step-level guards, no individual user list to maintain. The App token lives in the authorize job ONLY — the work job uses the default GITHUB_TOKEN, so the org-read capability never reaches the agent step. CODEOWNERS-driven trust set: - The auth check reads `.github/CODEOWNERS` via the default token and extracts every `@HarperFast/<team>` handle as the trust set. - Same set as people we trust to review code; alignment by construction. New owner team in CODEOWNERS automatically extends trigger trust. New consumer repo inherits its own CODEOWNERS. - External-org handles are deliberately ignored — only HarperFast teams. - If CODEOWNERS is missing, empty, or has no @HarperFast handles, falls back to @HarperFast/developers. Per-workflow specifics: - claude-review.yml: checks BOTH the PR author (`pull_request.user.login`) AND the event actor (`github.actor`). A non-trusted user pushing to a trusted user's PR branch changes the actor without changing the PR author; refusing those events closes that loophole. claude[bot] is admitted explicitly so AI-authored PRs from the issue-to-PR pipeline get reviewed (ADMIT_CLAUDE_BOT=true). - claude-mention.yml: checks the commenter. claude[bot] not admitted here (only humans trigger mentions). - claude-issue-to-pr.yml: checks the LABELER (github.actor), not the issue author. The labeler must already have at least triage permission; a maintainer labeling an external-author issue is a legitimate way to invoke the agent on community reports. Per the post-#447 convention, the auth-check bash lives in `.github/scripts/authorize-claude-workflow.sh` (shared across all three workflows; parameterized by env vars). Workflows invoke via `bash .github/scripts/...`. Defense-in-depth lint: - New `.github/workflows/auth-gate-invariants.yml` runs on any PR touching a `claude-*.yml` workflow file. Validates structurally via `bash .github/scripts/validate-auth-gate-invariants.sh`: * `authorize` job exists. * `authorize.outputs.authorized` wired to a step output. * `actions/create-github-app-token` present and pinned to a SHA. * `authorize.permissions` has no `write` scopes. * `HARPERFAST_AI_CLIENT_ID` and `HARPERFAST_AI_APP_PRIVATE_KEY` secrets referenced. * Every non-authorize job has `needs: authorize` and an exact `if: needs.authorize.outputs.authorized == 'true'` (no compound expressions, no tautologies). Make this a REQUIRED status check on `main` via branch protection. Subtle attacks on the bash logic are caught by CODEOWNERS review on `.github/`. Required (organization-level) secrets — must be set on the HarperFast org for any consumer repo to authorize a Claude run: - HARPERFAST_AI_CLIENT_ID (the App's Client ID, like Iv23li…) - HARPERFAST_AI_APP_PRIVATE_KEY (.pem file contents) Replaces #417's two earlier commits (which were on a stale base that pre-dated #432, #437, #438, #439, #442, #444, #447). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The inline
run: |blocks inclaude-review.ymlandclaude-mention.ymlhad grown past the reviewable-in-YAML threshold — the log step alone was 142 lines. Pulls them out to standalone scripts:.github/scripts/compose-review-scope.sh.github/scripts/find-prior-review-comment.sh.github/scripts/log-review-to-ai-review-log.sh.github/scripts/parse-claude-mention.shEach script documents inputs, outputs, and the rationale for non-obvious mechanics in its own header.
Mechanics
Workflows invoke via
bash .github/scripts/<name>.shrather than direct path execution. Sidesteps the +x bit being dropped on Windows checkouts and the "forgot to chmod the new script" footgun. The#!/usr/bin/env bashshebangs are informational only — bash invocation reads the file directly.Out of scope (intentional)
npm run test:workflowscoverage. The pure-logic parts (title generation, marker detection, mention parsing) are the natural test targets; gh/curl integration aspects need a fixture story..github/scripts/structure as part of that PR's review fixup.Test plan
@claudemention a PR → confirmparse-claude-mention.shcorrectly gatesproceedand selects the right model.no blockers(not0 finding(s)) and the comment threads into the existing log issue.🤖 Generated with Claude Code