ci(claude): route within-PR learnings to log surface via $RUNNER_TEMP file#438
Merged
ci(claude): route within-PR learnings to log surface via $RUNNER_TEMP file#438
Conversation
… file Adds a structured-notes channel from the review agent to the per-PR ai-review-log issue, with no extra footprint on the PR itself: - `Write` joins the allowedTools list, but the prompt restricts it to a single path (`$RUNNER_TEMP/claude-review-notes.md`). No per-path filter exists in claude-code-action's allowlist; the path-bound is prompt-enforced. Deviations are contained — the runner is ephemeral and has no write credentials beyond `gh pr comment`. - New "## After reviewing: capture learnings for the log surface" prompt section: structured format with sections for surfaces verified, dismissals honored from prior conversation, findings resolved by this push, and new observations. Empty sections are omitted; absent file is a clean no-op. - The log step appends the file's contents to the log-issue comment body before posting, after the existing concise PR-comment block. PR comment stays strictly concise; the log issue gets the full picture. This closes the design-intent gap from #432: "PR is for the opener and reviewers; everything else goes in the related feedback-loop issue." It also gives the future KB MCP server a stable, structured ingestion source — every per-PR log issue carries one notes block per run, formatted the same way. Independent of the in-flight `ai-review-prompts/universal.md` "read prior conversation" change (HarperFast/ai-review-prompts#7); that's a separate one-line pin bump after it merges. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 30, 2026
Merged
kriszyp
pushed a commit
that referenced
this pull request
May 5, 2026
…-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>
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
Adds a structured-notes channel from the review agent to the per-PR
HarperFast/ai-review-logissue, with no extra footprint on the PR itself. Closes the design-intent gap from #432: "PR is for the opener and reviewers; everything else goes in the related feedback-loop issue."Three pieces:
Tool surface —
Writejoins--allowedTools, but the prompt restricts it to a single path:$RUNNER_TEMP/claude-review-notes.md. claude-code-action's allowlist doesn't support per-path filters, so the bound is prompt-enforced. Deviations are contained — the runner is ephemeral and has no write credentials beyondgh pr comment.Prompt section — new "## After reviewing: capture learnings for the log surface" with a structured markdown format:
Empty sections are omitted; absent file is a clean no-op (the agent skips the file when there's nothing structured to capture).
Log step — reads
$RUNNER_TEMP/claude-review-notes.mdif present and appends to the log-issue comment body before posting. PR comment stays strictly concise.Why now
Sets up a stable, structured ingestion source for the future KB MCP server. Every per-PR log issue will carry one well-formed notes block per run, same shape across PRs and repos.
Independent of the universal-layer change
HarperFast/ai-review-prompts#7(the "read prior conversation" prompt change) is in-flight separately. After it merges, harper bumps theai-review-promptsref pin inclaude-review.yml— small one-liner. The two changes reinforce each other (read prior conversation → capture what you learned), but neither blocks the other.Test plan
[harper] PR #<N>: …issue receives a comment whose body contains both the concise PR-comment block AND a## Run notessection.<details>, no run notes block, no recap of what was traced.## Run notessection. Log step output:No run notes file at ... — skipping notes append.Writeis restricted in practice: any file path other than$RUNNER_TEMP/claude-review-notes.mdshould not appear after a clean run.🤖 Generated with Claude Code