Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 37 additions & 6 deletions .github/workflows/claude-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,51 @@ concurrency:

jobs:
review:
# Skip forks, skip non-claude bots (e.g. dependabot, renovate),
# but allow claude-created PRs through for self-review
# Skip forks and third-party bots. Allow human-authored PRs plus PRs created
# by our triage workflow on same-repository claude/issue-* branches.
if: |
github.event.pull_request.head.repo.fork == false &&
(
!endsWith(github.event.pull_request.user.login, '[bot]') ||
contains(github.event.pull_request.user.login, 'claude')
contains(github.event.pull_request.user.login, 'claude') ||
(
github.event.pull_request.user.login == 'github-actions[bot]' &&
startsWith(github.event.pull_request.head.ref, 'claude/issue-')
)
)
runs-on: ubuntu-latest
permissions:
actions: read
contents: read
pull-requests: write
id-token: write
steps:
- name: Checkout repository
uses: actions/checkout@v4
with:
fetch-depth: 1
fetch-depth: 0

- name: Detect review workflow changes
id: review-workflow
shell: bash
run: |
if git diff --quiet \
'${{ github.event.pull_request.base.sha }}' \
'${{ github.event.pull_request.head.sha }}' \
-- .github/workflows/claude-review.yml; then
echo "changed=false" >> "$GITHUB_OUTPUT"
else
echo "changed=true" >> "$GITHUB_OUTPUT"
fi

- name: Claude Code Review
uses: anthropics/claude-code-action@v1.0.93
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
# Claude's app token gives ordinary PRs a distinct reviewer identity.
# A workflow-token fallback is required when this workflow itself changes,
# because the Claude app validates it against the default-branch version.
github_token: ${{ steps.review-workflow.outputs.changed == 'true' && github.token || '' }}
allowed_bots: '*'
track_progress: true
classify_inline_comments: 'true'
Expand All @@ -44,6 +66,8 @@ jobs:
prompt: |
REPO: ${{ github.repository }}
PR NUMBER: ${{ github.event.pull_request.number }}
PR AUTHOR: ${{ github.event.pull_request.user.login }}
REVIEW ACTOR: ${{ steps.review-workflow.outputs.changed == 'true' && 'github-actions[bot]' || 'claude[bot]' }}

**Before doing anything else, read and clean up stale Claude feedback:**
1. Fetch all previous Claude comments on this PR and read them to retain context:
Expand All @@ -68,10 +92,17 @@ jobs:
Leave inline comments on specific lines and a top-level summary.
Skip nitpicks; prioritize substantive feedback.

**When you are done, submit a formal GitHub review using one of:**
**When you are done, submit a GitHub review using one of:**
- `gh pr review ${{ github.event.pull_request.number }} --approve --body "..."` — if no substantive issues found
- `gh pr review ${{ github.event.pull_request.number }} --request-changes --body "..."` — if there are blocking problems

Do not leave the PR without a formal APPROVE or REQUEST_CHANGES review state.
Exception when PR AUTHOR and REVIEW ACTOR are both `github-actions[bot]`: GitHub
prevents that workflow identity from approving or requesting changes on its own PR.
In that case, use
`gh pr review ${{ github.event.pull_request.number }} --comment --body "..."`
and begin the body with either `PASS —` or `BLOCKING —`. The successful/failed
review workflow check is the merge gate for that self-review case.

Do not leave a human-authored PR without a formal APPROVE or REQUEST_CHANGES state.
claude_args: |
--model sonnet --allowedTools "mcp__github__pull_request_review_write,mcp__github__add_comment_to_pending_review,mcp__github__add_issue_comment,mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr review:*),Bash(gh api repos/*/issues/*/comments:*),Bash(gh api repos/*/issues/comments/*:*),Bash(gh api repos/*/pulls/*/reviews:*),Bash(gh api repos/*/pulls/*/comments:*)"
38 changes: 38 additions & 0 deletions src/services/checkmarx/client.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { describe, expect, it, vi } from 'vitest';
import type { HttpClient } from '../../lib/http.js';
import { CheckmarxClient } from './client.js';

function makeClient(response: unknown): {
client: CheckmarxClient;
checkmarx: ReturnType<typeof vi.fn>;
} {
const checkmarx = vi.fn().mockResolvedValue(response);
const http = { checkmarx } as unknown as HttpClient;
return { client: new CheckmarxClient(http), checkmarx };
}

describe('CheckmarxClient', () => {
it('returns an empty project list when Checkmarx responds with null', async () => {
const { client, checkmarx } = makeClient({
projects: null,
totalCount: 0,
filteredTotalCount: 0
});

await expect(client.listProjects()).resolves.toEqual([]);
expect(checkmarx).toHaveBeenCalledWith('/api/projects', { params: { limit: 100 } });
});

it('returns an empty scan list when Checkmarx responds with null', async () => {
const { client, checkmarx } = makeClient({
scans: null,
totalCount: 0,
filteredTotalCount: 0
});

await expect(client.listScans({ projectId: 'project-id', last: 25 })).resolves.toEqual([]);
expect(checkmarx).toHaveBeenCalledWith('/api/scans', {
params: { limit: 25, 'project-id': 'project-id' }
});
});
});
14 changes: 8 additions & 6 deletions src/services/checkmarx/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ export class CheckmarxClient {

async listProjects(): Promise<CxOneProject[]> {
const res = await this.http.checkmarx<CxOneProjectsResponse>('/api/projects', { params: { limit: 100 } });
if (res.filteredTotalCount > res.projects.length) {
process.stderr.write(`warning: ${res.filteredTotalCount} projects found; only showing first ${res.projects.length}\n`);
const projects = res.projects ?? [];
if (res.filteredTotalCount > projects.length) {
process.stderr.write(`warning: ${res.filteredTotalCount} projects found; only showing first ${projects.length}\n`);
}
return res.projects;
return projects;
}

async getProject(id: string): Promise<CxOneProject> {
Expand All @@ -26,10 +27,11 @@ export class CheckmarxClient {
const params: Record<string, string | number> = { limit: opts.last ?? 100 };
if (opts.projectId) params['project-id'] = opts.projectId;
const res = await this.http.checkmarx<CxOneScansResponse>('/api/scans', { params });
if (res.filteredTotalCount > res.scans.length) {
process.stderr.write(`warning: ${res.filteredTotalCount} scans found; only showing first ${res.scans.length}\n`);
const scans = res.scans ?? [];
if (res.filteredTotalCount > scans.length) {
process.stderr.write(`warning: ${res.filteredTotalCount} scans found; only showing first ${scans.length}\n`);
}
return res.scans;
return scans;
}

async getScan(id: string): Promise<CxOneScan> {
Expand Down
4 changes: 2 additions & 2 deletions src/types/checkmarx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ export interface CxOneResultsSummary {
}

export interface CxOneProjectsResponse {
projects: CxOneProject[];
projects: CxOneProject[] | null;
totalCount: number;
filteredTotalCount: number;
}

export interface CxOneScansResponse {
scans: CxOneScan[];
scans: CxOneScan[] | null;
totalCount: number;
filteredTotalCount: number;
}
Loading