Skip to content

ci(claude): bump ai-review-prompts pin to pick up concise-PR + within-PR memory#439

Merged
heskew merged 1 commit intomainfrom
workflow/bump-ai-review-prompts-pin
Apr 30, 2026
Merged

ci(claude): bump ai-review-prompts pin to pick up concise-PR + within-PR memory#439
heskew merged 1 commit intomainfrom
workflow/bump-ai-review-prompts-pin

Conversation

@heskew
Copy link
Copy Markdown
Member

@heskew heskew commented Apr 30, 2026

Summary

One-line bump of the ai-review-prompts ref pin in claude-review.yml from 752c5da (2026-04-23 seed) to 14c79a1 (2026-04-30 main). Picks up two universal-layer changes:

  • ai-review-prompts#6 — keep PR comments concise; route calibration tracing to the workflow's log surface when one is wired.
  • ai-review-prompts#7 — read prior PR conversation before reviewing; don't re-raise dismissed findings; mark resolved findings as resolved.

Why now (no buffer)

Internal HarperFast-owned repo. The buffer discipline (wait several days after release before bumping) applies to third-party actions where supply-chain compromise is the threat we're hedging against. Internal config repos that we control on both sides ship without delay.

How it interacts with existing harper changes

Test plan

  • Push to a PR with prior claude activity → confirm the new review reads prior comments / inline threads and doesn't re-raise dismissed findings.
  • Confirm the related ai-review-log issue's run-notes block populates with at least the "Surfaces verified" / "Dismissals honored" sections when relevant.
  • PR comment stays strictly concise — single sentence on "no blockers" runs.

🤖 Generated with Claude Code

…-PR memory

Bumps the `ai-review-prompts` ref from `752c5da` (2026-04-23 seed) to
`14c79a1` (2026-04-30 main). Two universal-layer changes land:

- HarperFast/ai-review-prompts#6 — keep PR comments concise; route
  the calibration tracing to the workflow's log surface when one is
  wired (which harper now has, via #432).
- HarperFast/ai-review-prompts#7 — read prior PR conversation
  before reviewing (top-level comments, inline review threads,
  commit messages since last review). Don't re-raise dismissed
  findings, mark resolved findings as resolved.

Both reinforce the harper-side mechanisms already in place:

- The concise-PR override in #437 was overriding a stale universal
  clause; that clause is now fixed at source. The override stays as
  in-tree documentation but no longer needs to fight the layer.
- The learnings-to-log channel in #438 is the structured target the
  universal "read prior conversation" instruction implicitly relies
  on — capturing what was learned for future runs.

Internal-repo bump, no buffer needed (HarperFast-owned source). The
buffer discipline applies to third-party actions, not our own
prompt-config repo.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@heskew heskew requested review from a team as code owners April 30, 2026 22:44
@heskew heskew merged commit b6b4ada into main Apr 30, 2026
9 of 12 checks passed
@heskew heskew deleted the workflow/bump-ai-review-prompts-pin branch April 30, 2026 22:45
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant