From eb0145d314bc8046012dcf425d967d90d99b25b4 Mon Sep 17 00:00:00 2001 From: David Leong <116610336+leongdl@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:05:53 -0700 Subject: [PATCH] ci: enable Claude PR review with RFC-review guidance Opt in to the two-stage Claude PR-review integration hosted in OpenJobDescription/.github (see OpenJobDescription/.github#23): - claude_pr_review_collect.yml (Stage 1): pull_request-triggered no-op whose completion fires the workflow_run event. Fork PRs see no secrets and nothing this stage produces is trusted. - claude_pr_review.yml (Stage 2): workflow_run-triggered caller that forwards the trusted head repo/SHA and the Bedrock OIDC role secret to the reusable review workflow. Add .claude-review.md, the repo-specific guidance the reviewer reads from the base branch. It directs reviews of rfcs/ changes to follow the methodology in skills/openjd-rfc-review/SKILL.md: alignment with the four design tenets (portable, expressive, human readable/writable, tooling parseable), RFC template completeness, and compatibility with the 2023-09 Template Schemas specification. The skill file remains the source of truth; the guidance inlines the methodology because the reviewer must not read review instructions from the PR head, which is fork-controlled. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com> --- .claude-review.md | 89 +++++++++++++++++++ .github/workflows/claude_pr_review.yml | 14 ++- .../workflows/claude_pr_review_collect.yml | 6 +- 3 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 .claude-review.md diff --git a/.claude-review.md b/.claude-review.md new file mode 100644 index 0000000..050307f --- /dev/null +++ b/.claude-review.md @@ -0,0 +1,89 @@ +# Review guidance for openjd-specifications + +This file is read by the automated Claude PR reviewer (see +`OpenJobDescription/.github`'s `reusable_claude_pr_review.yml`) from the base +branch, and tells it what this repository wants reviews to focus on. It is +maintained alongside `skills/openjd-rfc-review/SKILL.md`, which is the source +of truth for the RFC review methodology; keep the two in sync. + +## What this repository contains + +- `rfcs/` — RFC proposals for changes to the Open Job Description + specification. **This is the most important content to review.** +- `wiki/` — the specification itself (2023-09 Template Schemas and related + pages). +- `conformance-tests/` — conformance test suites for OpenJD implementations. +- `samples/` — sample job templates. +- `skills/` — review methodology documents (like the one this guidance is + derived from). + +## Reviewing RFC changes (files under `rfcs/`) + +Apply the methodology from `skills/openjd-rfc-review/SKILL.md`: + +### Design tenets + +Every RFC MUST align with these tenets. Flag concrete violations: + +1. **Portable** — works across operating systems, queuing implementations, + and technology stacks. Flag assumptions that only hold on one OS or one + data-management system. +2. **Expressive** — general features, not narrowly tailored to one use case. +3. **Human readable and writable** — templates using the feature can be + understood and authored in a text editor without boilerplate overload. +4. **Tooling parseable** — automated tooling can process a template using the + feature and unambiguously determine author intent. + +### Template completeness + +Required sections per `rfcs/0000-template.md`: Summary, Basic Examples, +Motivation, Specification, Design Choice Rationale, Prior Art, Rejected +Ideas, Copyright (public domain or CC0-1.0-Universal). Required header +metadata: Feature Name, RFC Tracking Issue URL, Start Date (YYYY-MM-DD), +Specification Version. Comment on missing or hollow sections. + +### Specification compatibility + +The proposed change must fit the formal specification in +`wiki/2023-09-Template-Schemas`: + +- Does the proposed language fit naturally into the specification document? +- Are new sections consistent with similar existing sections? +- Do new concepts apply orthogonally with existing features, or do special + cases limit their applicability? +- Does the combined system feel consistent and whole, not disparate pieces + bolted together? + +### Common RFC issues to call out + +- **Weak motivation**: "users want this" without concrete workflows enabled + and how common they are. +- **Vague specification**: prose where formal specification language is + needed — the Specification section should be droppable into the spec + document as-is. +- **Missing rationale**: significant design choices without the alternatives + considered and why this option won. +- **Tenet violations**: features too specific to one workflow or system + without justification. +- **Examples that don't validate** against the proposed schema, or that fail + to demonstrate the feature succinctly. +- **Edge cases and backward compatibility** not addressed. + +### Tone for RFC reviews + +RFC review comments are about the design and the writing, not code defects. +Anchor comments to the relevant line of the RFC document. Cite the specific +tenet or template section when raising an issue. Distinguish blocking issues +(tenet violations, missing required sections, spec incompatibility) from +suggestions (wording, additional prior art). + +## Reviewing other changes + +- `wiki/` changes: check that wording is precise, consistent with the + surrounding specification text, and does not change normative meaning + unintentionally. +- `conformance-tests/` changes: check test intent matches the specification + behavior being tested, and that assertions are correct for both success and + failure paths. +- `samples/` changes: check templates are valid against the published schemas + and demonstrate what they claim. diff --git a/.github/workflows/claude_pr_review.yml b/.github/workflows/claude_pr_review.yml index 686b262..fc7b4e8 100644 --- a/.github/workflows/claude_pr_review.yml +++ b/.github/workflows/claude_pr_review.yml @@ -7,17 +7,23 @@ # # All the review logic (Bedrock auth, the pinned action, the restricted tool # surface, the read-only PR-head checkout, and per-run limits) lives in the -# reusable workflow in aws-deadline/.github. This caller only forwards the +# reusable workflow in OpenJobDescription/.github. This caller only forwards the # workflow_run identifiers and the role-ARN secret. +# +# Repo-specific focus: the reviewer reads this repository's .claude-review.md +# (from the base branch), which directs it to review RFC documents under rfcs/ +# using the methodology in skills/openjd-rfc-review/SKILL.md -- design tenets, +# template completeness, and specification compatibility. name: Claude PR Review -on: +on: # zizmor: ignore[dangerous-triggers] -- workflow_run is intentional and safe here; see the header comment above (runs from default branch, fork PRs cannot alter behavior/secrets) workflow_run: workflows: ["Claude PR Review (collect)"] types: - completed -# Cancel a superseded review when the PR is pushed again, keyed on the trusted head repo + branch from the workflow_run payload (no pull_request.number here). +# Cancel a superseded review when the PR is pushed again, keyed on the trusted +# head repo + branch from the workflow_run payload (no pull_request.number here). concurrency: group: claude-pr-review-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }} cancel-in-progress: true @@ -39,7 +45,7 @@ jobs: # this prevents a review from firing -- with the base repo's secrets and a # fork-controlled head_sha -- from a non-PR context. if: ${{ github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'pull_request' }} - uses: aws-deadline/.github/.github/workflows/reusable_claude_pr_review.yml@mainline + uses: OpenJobDescription/.github/.github/workflows/reusable_claude_pr_review.yml@mainline permissions: contents: read pull-requests: write diff --git a/.github/workflows/claude_pr_review_collect.yml b/.github/workflows/claude_pr_review_collect.yml index 18418a9..bd15091 100644 --- a/.github/workflows/claude_pr_review_collect.yml +++ b/.github/workflows/claude_pr_review_collect.yml @@ -11,8 +11,8 @@ # fork's copy of this file cannot influence what gets reviewed or where comments # are posted. # -# This is a thin caller -- the (now no-op) collect job lives in the reusable -# workflow in aws-deadline/.github. +# This is a thin caller -- the (no-op) collect job lives in the reusable +# workflow in OpenJobDescription/.github. name: Claude PR Review (collect) on: @@ -29,4 +29,4 @@ permissions: {} jobs: collect: - uses: aws-deadline/.github/.github/workflows/reusable_claude_pr_review_collect.yml@mainline + uses: OpenJobDescription/.github/.github/workflows/reusable_claude_pr_review_collect.yml@mainline