feat(ci): add Claude Code GitHub Action for integration testing#517
feat(ci): add Claude Code GitHub Action for integration testing#517MartianGreed wants to merge 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds two Claude-driven GitHub Actions workflows, two new SKILL.md documentation artifacts, and a small benchmark timing declaration change for Bun nanoseconds; no public API or exported declarations were modified. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.claude/skills/dojo-integration-test/SKILL.md:
- Around line 15-18: The `asdf current` invocation in the SKILL.md incorrectly
passes multiple plugin names at once; update the documentation so the `asdf
current` command is used correctly by either showing a chained check for each
plugin (run `asdf current` for sozo, katana, torii, and scarb individually) or
by suggesting `asdf current` with no args to list all plugins; reference the
`asdf current` command and the plugin names sozo, katana, torii, scarb when
editing the example.
In @.github/workflows/claude-assistant.yaml:
- Around line 20-23: The workflow currently checks out code without resolving
the PR head ref (actions/checkout@v4 in the steps block), which causes
issue_comment triggers to use the default branch; add a step that resolves the
PR head ref (when the issue_comment targets a PR) using the GitHub API or
actions/github-script to find the PR for github.event.issue.number and output
the head.ref (e.g., set step id get_pr_head and output ref), then change the
actions/checkout step to use with: ref: ${{ steps.get_pr_head.outputs.ref }}
(falling back to github.ref or default branch when no PR is found).
- Around line 16-19: Add an explicit top-level permissions block to the workflow
so it only requests the minimal rights required (avoid relying on implicit
defaults); modify the workflow definition that contains the if condition for
triggering (the job using the claude-code-action trigger) to include a
permissions: mapping with at least contents: read, issues: write, and
pull-requests: write to ensure external contributors can trigger the action and
to make permissions explicit for reviewers.
In @.github/workflows/integration-test.yaml:
- Around line 23-26: Add an explicit minimal permissions block to the workflow
to restrict GITHUB_TOKEN scope: add a top-level permissions entry with
"contents: read" so the workflow (which uses actions/checkout@v4 and may run
with a pr_number input) only has read access to repository contents and cannot
access or exfiltrate secrets like ANTHROPIC_API_KEY; place this permissions
block at the workflow root (same level as on: and jobs:) to ensure it applies to
the entire workflow.
🧹 Nitpick comments (4)
.github/workflows/integration-test.yaml (1)
33-35: Consider pinning Bun version for reproducible builds.Using
bun-version: latestcan lead to inconsistent CI behavior if Bun releases a breaking change. Consider pinning to a specific version (e.g.,1.1.x) for reproducibility.Suggested fix
- uses: oven-sh/setup-bun@v1 with: - bun-version: latest + bun-version: "1.1.45".claude/skills/dojo-integration-test/SKILL.md (1)
168-197: Quick start workflow uses brittlesleepcommands for service readiness.Using fixed
sleepdurations is unreliable—services may start faster or slower depending on the environment. Consider polling with retries for more robust health checks.Suggested improvement for health checks
# Instead of: sleep 2 # Use polling with timeout: for i in {1..30}; do curl -s localhost:5050 > /dev/null && break sleep 1 done # Similarly for torii: for i in {1..30}; do curl -s localhost:8080/health > /dev/null && break sleep 1 done.github/workflows/claude-assistant.yaml (2)
8-9: Unused trigger eventsopenedandassignedon issues.The job condition only checks for
@claudein comments or thetest-integrationlabel. Theopenedandassignedevents on issues will never satisfy the condition (unless the issue body/title contains@claude, which isn't checked). Consider removing these unused triggers to avoid confusion.Suggested fix
issues: - types: [opened, assigned, labeled] + types: [labeled]
30-32: Same concern: Consider pinning Bun version.Consistent with the feedback on
integration-test.yaml, consider pinning the Bun version for reproducible builds.
| 1. **Required asdf plugins**: sozo, katana, torii, scarb | ||
| ```bash | ||
| asdf current sozo katana torii scarb | ||
| ``` |
There was a problem hiding this comment.
Incorrect asdf current command syntax.
asdf current accepts only one plugin name at a time, not multiple. This command will fail.
Suggested fix
1. **Required asdf plugins**: sozo, katana, torii, scarb
```bash
- asdf current sozo katana torii scarb
+ asdf current sozo && asdf current katana && asdf current torii && asdf current scarb
```Alternatively, use asdf current without arguments to list all plugins, or check each individually.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1. **Required asdf plugins**: sozo, katana, torii, scarb | |
| ```bash | |
| asdf current sozo katana torii scarb | |
| ``` | |
| 1. **Required asdf plugins**: sozo, katana, torii, scarb |
🤖 Prompt for AI Agents
In @.claude/skills/dojo-integration-test/SKILL.md around lines 15 - 18, The
`asdf current` invocation in the SKILL.md incorrectly passes multiple plugin
names at once; update the documentation so the `asdf current` command is used
correctly by either showing a chained check for each plugin (run `asdf current`
for sozo, katana, torii, and scarb individually) or by suggesting `asdf current`
with no args to list all plugins; reference the `asdf current` command and the
plugin names sozo, katana, torii, scarb when editing the example.
641cc9f to
785b258
Compare
785b258 to
1084615
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.claude/skills/dojo-integration-test/SKILL.md:
- Around line 244-246: The Quick Start snippet uses an invalid multi-plugin call
"asdf current sozo katana torii scarb"; update it to invoke asdf current once
per plugin (e.g., replace that single line with four separate commands calling
"asdf current" for each plugin: sozo, katana, torii, and scarb) so the
documentation shows the correct usage and will work when copy-pasted.
♻️ Duplicate comments (1)
.claude/skills/dojo-integration-test/SKILL.md (1)
25-28: Incorrectasdf currentcommand syntax was addressed per past review.The fix from the previous review was applied here.
🧹 Nitpick comments (1)
packages/grpc/src/benchmarks/effect-functional.bench.ts (1)
1-7: Move thedeclare const Bunstatement after all imports.The global declaration is placed between the
bun:testimport and local imports, breaking the import grouping convention. Per coding guidelines, imports should be grouped: external dependencies → workspace packages → relative imports, with no interleaving declarations.Suggested reordering
import { describe, test, expect } from "bun:test"; - -declare const Bun: { nanoseconds: () => number }; - import { ToriiGrpcClient } from "../torii-client"; import { makeToriiClient } from "../torii-client-functional"; import type { Query } from "@dojoengine/torii-wasm"; + +declare const Bun: { nanoseconds: () => number };
| ```bash | ||
| # 1. Check prerequisites | ||
| asdf current sozo katana torii scarb |
There was a problem hiding this comment.
Same asdf current syntax issue in Quick Start section.
Line 246 uses the same incorrect multi-plugin syntax. The asdf current command only accepts one plugin name at a time.
Suggested fix
# 1. Check prerequisites
-asdf current sozo katana torii scarb
+asdf current sozo && asdf current katana && asdf current torii && asdf current scarb🤖 Prompt for AI Agents
In @.claude/skills/dojo-integration-test/SKILL.md around lines 244 - 246, The
Quick Start snippet uses an invalid multi-plugin call "asdf current sozo katana
torii scarb"; update it to invoke asdf current once per plugin (e.g., replace
that single line with four separate commands calling "asdf current" for each
plugin: sozo, katana, torii, and scarb) so the documentation shows the correct
usage and will work when copy-pasted.
Summary
integration-test.yamlworkflow for manual dispatch integration testing with Claude Codeclaude-assistant.yamlworkflow for interactive@claudementions andtest-integrationlabel triggersdojo-integration-testskill for running the full Dojo stack (katana, torii, sozo)Setup Required
ANTHROPIC_API_KEYto repository secretstest-integrationlabelUsage
@claudeon any PR/issuetest-integrationlabel to triggerSummary by CodeRabbit
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.