Skip to content

Conversation

@matin-deriv
Copy link
Contributor

  • Adding Claude code review & Claude MD to the repo

@github-actions
Copy link
Contributor

Dependency Review

The following issues were found:
  • ✅ 0 vulnerable package(s)
  • ✅ 0 package(s) with incompatible licenses
  • ✅ 0 package(s) with invalid SPDX license definitions
  • ⚠️ 2 package(s) with unknown licenses.
See the Details below.

License Issues

.github/workflows/claude.yml

PackageVersionLicenseIssue Type
anthropics/claude-code-action1.*.*NullUnknown License
deriv-com/shared-actions/.github/actions/verify_user_in_organization3.*.*NullUnknown License

OpenSSF Scorecard

PackageVersionScoreDetails
actions/actions/checkout 4.*.* 🟢 6.5
Details
CheckScoreReason
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Maintained🟢 56 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 5
Code-Review🟢 10all changesets reviewed
Binary-Artifacts🟢 10no binaries found in the repo
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Packaging⚠️ -1packaging workflow not detected
Signed-Releases⚠️ -1no releases found
Pinned-Dependencies🟢 3dependency not pinned by hash detected -- score normalized to 3
Security-Policy🟢 9security policy file detected
Vulnerabilities🟢 91 existing vulnerabilities detected
Branch-Protection🟢 5branch protection is not maximal on development and all release branches
SAST🟢 8SAST tool detected but not run on all commits
actions/anthropics/claude-code-action 1.*.* UnknownUnknown
actions/deriv-com/shared-actions/.github/actions/verify_user_in_organization 3.*.* UnknownUnknown

Scanned Manifest Files

.github/workflows/claude.yml
  • actions/checkout@4.*.*
  • anthropics/claude-code-action@1.*.*
  • deriv-com/shared-actions/.github/actions/verify_user_in_organization@3.*.*

@github-actions
Copy link
Contributor

github-actions bot commented Dec 29, 2025

Claude finished @matin-deriv's task in 1m 25s —— View job


Code Review Complete ✅

I've reviewed both files added in this PR. Overall, this is a valuable addition to the repository that will improve AI-assisted development and code review processes. Below are my findings organized by priority.


🔴 High Priority Issues

1. Security: Missing ANTHROPIC_API_KEY Secret (.github/workflows/claude.yml:55)

The workflow references secrets.ANTHROPIC_API_KEY but this secret may not be configured in the repository settings yet.

Action Required: Ensure the ANTHROPIC_API_KEY secret is added to the repository secrets before merging this PR, otherwise the workflow will fail.

Location: .github/workflows/claude.yml:55

2. Security: User Verification Step (.github/workflows/claude.yml:28-32)

The user verification step uses PERSONAL_ACCESS_TOKEN which appears to be a shared secret. This is good for security, but:

  • Ensure this token has minimal required permissions (only org member read access)
  • Consider documenting who can trigger Claude reviews (org members only? specific teams?)
  • The verification happens before checkout, which is good practice

Recommendation: Add a comment in the workflow explaining the security model (e.g., "Only deriv-com org members can trigger Claude reviews").


🟡 Medium Priority Issues

3. Workflow Trigger Mismatch (.github/workflows/claude.yml:6-12)

The workflow has a logical inconsistency:

on:
    pull_request:
        types: [opened, synchronize, reopened, ready_for_review]

jobs:
    claude-assistant:
        # Only run on PR-related events that contain @claude
        if: github.event_name == 'pull_request'

Issues:

  • The comment says "that contain @claude" but there's no actual check for the @claude mention in the if condition
  • The workflow will run on every PR event (opened, synchronize, etc.), not just when @claude is mentioned
  • This could burn through API credits unnecessarily

Recommended Fix:

if: github.event_name == 'pull_request' && contains(github.event.pull_request.body, '@claude')

Or if you want to check PR comments too, you may need to add issue_comment trigger and more complex logic. The current setup will trigger Claude on every commit push to any PR.

Location: .github/workflows/claude.yml:6-12

4. Concurrency Group Could Be More Specific (.github/workflows/claude.yml:23-25)

concurrency:
    group: pr-${{ github.event.pull_request.number }}
    cancel-in-progress: true

Issue: This will cancel ANY workflow with the same PR number, not just Claude workflows. If you have multiple workflows, this could cause unexpected cancellations.

Recommended Fix:

concurrency:
    group: claude-pr-${{ github.event.pull_request.number }}
    cancel-in-progress: true

Location: .github/workflows/claude.yml:23-25

5. Fork Safety Consideration (.github/workflows/claude.yml:36-41)

The workflow checks out the PR head from the fork:

repository: ${{ github.event.pull_request.head.repo.full_name }}

Potential Issue: If a fork is deleted after PR creation, the checkout will fail. While this is rare, consider adding error handling or a fallback.

Also: Using GITHUB_TOKEN (line 41) for fork checkouts should work, but be aware that the user verification step (line 28-32) already restricts who can run this, which is good.

6. Timeout is Long (.github/workflows/claude.yml:52)

timeout-minutes: 60

Question: Is 60 minutes necessary? Claude reviews typically take 5-15 minutes. A 60-minute timeout could mask issues or waste runner time if something goes wrong.

Recommendation: Consider reducing to 30 minutes unless you have very large PRs that need longer review times.

Location: .github/workflows/claude.yml:52


🟢 Low Priority / Suggestions

7. CLAUDE.md: Minor Inconsistency in Version Documentation

In CLAUDE.md:10, the license year is 2024:

-   **License**: MIT License (Copyright 2024 deriv.com)

Suggestion: Since we're now in 2025, consider updating to "Copyright 2024-2025 deriv.com" or checking the actual LICENSE file for the correct copyright year.

Location: CLAUDE.md:10

8. CLAUDE.md: Excellent Documentation Overall

The CLAUDE.md file is comprehensive and well-structured. It covers:

  • Technology stack with specific versions
  • Architecture patterns (MobX stores, bot engine, etc.)
  • Common commands and workflows
  • Key design decisions
  • Important caveats

Strengths:

  • Clear explanation of the 22 MobX stores
  • Detailed bot engine architecture
  • Path aliases documented
  • Git workflow conventions specified
  • Honest about technical debt (Webpack → Rsbuild transition)

Minor Enhancement: Consider adding a "Troubleshooting" section for common development issues (e.g., HTTPS cert issues, port conflicts, etc.).

9. Workflow: Consider Adding Workflow Dispatch (.github/workflows/claude.yml)

For testing and manual triggers, consider adding workflow_dispatch to the triggers:

on:
    pull_request:
        types: [opened, synchronize, reopened, ready_for_review]
    workflow_dispatch:  # Allows manual triggering

This would make it easier to test the workflow without creating PRs.

10. Missing Documentation in Workflow File

Consider adding more inline documentation in the workflow about:

  • What happens when Claude finishes (does it post a comment? create review comments?)
  • Cost implications (API usage)
  • How to debug failures

📋 Summary

Must Fix Before Merge (High Priority)

  1. ✅ Confirm ANTHROPIC_API_KEY secret exists in repository settings
  2. ⚠️ Fix workflow trigger logic to only run when @claude is mentioned (or clarify if you want it to run on every PR)
  3. 📝 Document the security model (who can trigger, token permissions)

Should Fix (Medium Priority)

  1. Make concurrency group more specific (claude-pr-...)
  2. Consider reducing timeout from 60 to 30 minutes
  3. Add error handling for fork deletion edge case

Nice to Have (Low Priority)

  1. Update copyright year in CLAUDE.md to 2024-2025
  2. Add troubleshooting section to CLAUDE.md
  3. Add workflow_dispatch trigger for easier testing
  4. Add more inline documentation about costs and debugging

✨ What I Like About This PR

  1. Proactive Security: User verification step before checkout is excellent
  2. Comprehensive Documentation: CLAUDE.md is thorough and will be very helpful for AI assistants
  3. Proper Permissions: Workflow has minimal required permissions
  4. Good Git Practices: Fetch depth of 20 for proper diff analysis
  5. Clear Review Focus: The prompt in the workflow is well-structured and specific

🎯 Recommended Next Steps

  1. Before merging:

    • Add ANTHROPIC_API_KEY to repository secrets
    • Decide on trigger logic (every PR vs @claude mention only)
    • Fix concurrency group name
  2. After merging:

    • Monitor first few Claude runs to ensure workflow behaves as expected
    • Document usage guidelines for team (when to use @claude, what to expect)
    • Consider cost tracking/budgeting for API usage
  3. Future enhancements:

    • Add metrics/logging for Claude review effectiveness
    • Consider rate limiting or cost controls
    • Add workflow status badges to README

@github-actions
Copy link
Contributor

Preview Link: https://matin-claude-files.bot-65f.pages.dev

Name Result
Build status Completed ✅
Preview URL Visit Preview
Action URL Visit Action

@matin-deriv matin-deriv merged commit 32c39f9 into master Dec 29, 2025
8 checks passed
@matin-deriv matin-deriv deleted the matin/claude-files branch December 29, 2025 03:12
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.

2 participants