Skip to content

Fix code review workflow trigger and hardening#7

Merged
xdanger merged 4 commits intomainfrom
xdanger-codex/review-codereview-workflow
Apr 2, 2026
Merged

Fix code review workflow trigger and hardening#7
xdanger merged 4 commits intomainfrom
xdanger-codex/review-codereview-workflow

Conversation

@xdanger
Copy link
Copy Markdown
Member

@xdanger xdanger commented Mar 30, 2026

Summary

  • add explicit workflow_call inputs so the review job works for both direct PR runs and reusable callers
  • make the Copilot reviewer request best-effort instead of blocking the main review job
  • tighten workflow permissions and Claude's allowed tool surface to reduce unnecessary write access
  • deepen checkout history, add review guidance, cap turns, and pin third-party actions to SHAs

Testing

  • Not run (not requested)

@github-actions github-actions bot requested a review from Copilot March 30, 2026 20:43
@xdanger xdanger review requested due to automatic review settings March 30, 2026 20:43
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 30, 2026

Greptile Summary

This PR hardens the reusable code-review.yml workflow by adding explicit workflow_call inputs for PR context, removing excess permissions (id-token: write, issues: write), pinning third-party actions to SHAs, simplifying the Claude prompt, and making the Copilot reviewer request non-blocking. The changes are well-reasoned and move in a clear positive direction, but one issue needs attention before merge.

Key changes:

  • workflow_call inputs (pr_number, head_repo_full_name) added so reusable callers can pass PR context explicitly
  • Concurrency group now scoped to github.repository to prevent cross-repo key collisions
  • id-token: write and issues: write permissions removed — only pull-requests: write is retained
  • Third-party actions pinned to full commit SHAs (actions/checkout, anthropics/claude-code-action)
  • --max-turns 30 cap added; WebSearch/WebFetch and the trailing comma removed from --allowedTools
  • Copilot reviewer step marked continue-on-error: true
  • ANTHROPIC_BASE_URL env var dropped (aligns with commit f6a9c53 which removed the corresponding secret input)

Issues found:

  • P1 — claude_args comment line will be passed literally to the CLI (line 83): The # Keep the tool surface narrow… line sits inside a YAML literal block scalar (|) and is therefore part of the string value, not a YAML comment. If claude-code-action expands args without shell eval, this unrecognised token will likely cause the step to fail on every run.
  • P2 — No draft-PR guard on the workflow_call path: Unlike the pull_request branch, the workflow_call condition does not check for draft status, leaving callers to enforce this themselves.
  • P2 — fetch-depth: 0 clones full history: Switching from a shallow clone to an unbounded full history adds unnecessary time and bandwidth for large repos; a shallow depth of 10–50 would satisfy git log/git blame needs.

Confidence Score: 3/5

Not safe to merge as-is — the literal comment line in claude_args will likely break the review step on every run.

The overall direction and structural changes are solid (pinned SHAs, tighter permissions, correct workflow_call wiring). However, the # comment line embedded inside the claude_args YAML block scalar is a runtime defect that would silently or noisily break the primary review step. Fixing that one line is a trivial change that would bring this to a clean 4 or 5.

.github/workflows/code-review.yml — specifically line 83 (comment in claude_args block scalar)

Important Files Changed

Filename Overview
.github/workflows/code-review.yml Adds workflow_call inputs, tightens permissions, pins action SHAs, simplifies the Claude prompt, and caps turns — but a literal # comment line inside the claude_args block scalar will likely break the step at runtime.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([Trigger]) --> B{Event type?}
    B -- pull_request --> C{Is draft?}
    B -- workflow_call --> D{pr_number set?}
    C -- Yes --> Z([Skip — draft PR])
    C -- No --> E{Same-repo fork?}
    D -- No --> Z2([Skip — no PR context])
    D -- Yes --> F{head_repo_full_name match or unset?}
    E -- No --> Z3([Skip — fork])
    E -- Yes --> G[review job]
    F -- No --> Z4([Skip — fork])
    F -- Yes --> G
    G --> H[Request Copilot review
continue-on-error: true]
    H --> I[Checkout @ SHA
fetch-depth: 0]
    I --> J[claude-code-action @ SHA
--model opus
--max-turns 30]
    J --> K([Post inline review comments])
Loading

Reviews (1): Last reviewed commit: "🐛 fix: harden reusable code review work..." | Re-trigger Greptile

Comment on lines +41 to +48
) || (
github.event_name == 'workflow_call' &&
inputs.pr_number &&
(
!inputs.head_repo_full_name ||
inputs.head_repo_full_name == github.repository
)
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 workflow_call path has no draft-PR guard

The pull_request branch of the if condition correctly checks !github.event.pull_request.draft, but the workflow_call branch has no equivalent. A caller that triggers this workflow for a draft PR will not be blocked.

This is likely intentional (delegating the draft check to callers), but it's an implicit contract that is easy to miss and could lead to unnecessary review runs on in-progress work. Consider either documenting this assumption clearly in the workflow comment, or adding an optional is_draft input so callers can pass the flag and this workflow can enforce it uniformly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8bd03579fd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions github-actions bot requested a review from Copilot April 2, 2026 10:17
@xdanger xdanger review requested due to automatic review settings April 2, 2026 10:17
- 🐛 move claude args note outside the literal block
- 🐛 skip reusable runs for draft pull requests
- 🐛 cap checkout history to enough review context
@github-actions github-actions bot requested a review from Copilot April 2, 2026 10:22
@xdanger xdanger review requested due to automatic review settings April 2, 2026 10:22
- 🐛 restore id-token for claude action auth
@github-actions github-actions bot requested a review from Copilot April 2, 2026 10:24
@xdanger xdanger review requested due to automatic review settings April 2, 2026 10:24
@xdanger xdanger merged commit fcdf298 into main Apr 2, 2026
3 of 4 checks passed
@xdanger xdanger deleted the xdanger-codex/review-codereview-workflow branch April 2, 2026 10:25
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