feat(EC-1862): add AI skills for ec-cli - #3434
Conversation
Add 6 skills covering test execution, debugging, build/lint, PR checklist, test authoring, and benchmarking. Jira: EC-1862
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughSix new Claude skill documents provide Conforma CLI guidance for test development, test execution, failure debugging, builds, benchmarks, and pull request preparation. ChangesConforma CLI development skills
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
🤖 Review · |
PR Summary by QodoAdd repo-specific Claude AI skills for ec-cli workflows
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/build-and-lint/SKILL.md:
- Around line 29-30: Update the lint commands in the build-and-lint skill to
invoke golangci-lint through the repository-pinned tooling defined by
tools/go.mod rather than an unqualified global binary. Replace the single-file
example with an appropriate package path that provides reliable coverage, and
keep the gofmt check aligned with that target.
In @.claude/skills/pr-checklist/SKILL.md:
- Around line 21-24: Update the commit-message example fenced block in SKILL.md
to include the text language identifier, preserving its existing example
content.
In @.claude/skills/run-tests/SKILL.md:
- Around line 67-70: Update the fenced /etc/hosts blocks in
.claude/skills/run-tests/SKILL.md lines 67-70 and
.claude/skills/debug-failure/SKILL.md lines 38-41 to declare the text language.
No other content changes are needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 86a00619-00a3-4f91-a273-6a3a0a7bee66
📒 Files selected for processing (6)
.claude/skills/benchmark/SKILL.md.claude/skills/build-and-lint/SKILL.md.claude/skills/debug-failure/SKILL.md.claude/skills/pr-checklist/SKILL.md.claude/skills/run-tests/SKILL.md.claude/skills/write-tests/SKILL.md
| golangci-lint run internal/evaluator/evaluator.go | ||
| gofmt -l internal/evaluator/evaluator.go |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p' | rg 'Makefile|go.mod|SKILL\.md|tools/go.mod|evaluator.go' || true
echo
echo "== relevant skill excerpt =="
if [ -f .claude/skills/build-and-lint/SKILL.md ]; then
nl -ba .claude/skills/build-and-lint/SKILL.md | sed -n '1,80p'
fi
echo
echo "== Makefile lint references =="
for f in Makefile build/*; do
[ -f "$f" ] && echo "--- $f" && rg -n "golangci|lint|tools/go.mod" "$f" || true
done
echo
echo "== tools/go.mod if present =="
if [ -f tools/go.mod ]; then
nl -ba tools/go.mod | sed -n '1,220p'
fi
echo
echo "== golangci references in repo =="
rg -n "golangci|tools/go.mod"|sed -n '1,200p' || trueRepository: conforma/cli
Length of output: 478
🏁 Script executed:
#!/bin/sh
set -euo pipefail
echo "== relevant skill excerpt =="
if [ -f .claude/skills/build-and-lint/SKILL.md ]; then
awk '{printf "%6d\t%s\n", NR, $0}' .claude/skills/build-and-lint/SKILL.md | sed -n '1,80p'
fi
echo
echo "== Makefile lint references =="
awk '{printf "%6d\t%s\n", NR, $0}' Makefile 2>/dev/null | sed -n '1,220p' | rg -n "golangci|lint|tools/go.mod|.PHONY|run:" || true
echo
echo "== tools module files =="
git ls-files | rg '^tools/|go.mod|go.sum' || true
echo
echo "== golangci references in tracked files =="
rg -n "golangci|tools/go\.mod|cmd/golangci-lint|golangci-lint" . 2>/dev/null | sed -n '1,240p' || trueRepository: conforma/cli
Length of output: 10470
🏁 Script executed:
#!/bin/sh
set -euo pipefail
echo "== Makefile target definitions around lint =="
awk '{printf "%6d\t%s\n", NR, $0}' Makefile 2>/dev/null | sed -n '216,245p'
echo
echo "== root go.mod module line =="
awk 'NF && NR <= 6 {print}' go.mod
echo
echo "== tools.go import =="
awk '{printf "%6d\t%s\n", NR, $0}' tools/tools.go 2>/dev/null | sed -n '1,45p'Repository: conforma/cli
Length of output: 3927
🌐 Web query:
golangci-lint v2 single file lint single file supported
💡 Result:
Yes, golangci-lint supports linting a single file [1]. You can execute it by passing the file path directly to the run command, for example: golangci-lint run file1.go [1]. However, there are important limitations to be aware of: 1. Compilation Dependencies: Golangci-lint typically requires the code to compile to perform analysis [2]. If the single file you are linting has dependencies on other files or packages within your project, and those dependencies are not properly resolved or the package does not compile as a whole, the linting may fail [2]. 2. Linter Capability: Not all linters are designed to work on a single file [3]. Some linters require information from the entire package or project to function correctly (e.g., they might need to see all usages of a constant or type) [4][3][5]. If a linter requires this broader scope, it may not produce accurate results or might fail when restricted to a single file [3]. 3. No Mixing: When specifying paths, you cannot mix individual files with package directories in a single command [1]. To ensure correct analysis, it is generally recommended to run golangci-lint on the package or project level (e.g., golangci-lint run ./...) to provide the necessary context for all linters to function as intended [1][2].
Citations:
- 1: https://golangci-lint.run/docs/welcome/quick-start/
- 2: https://golangci-lint.run/docs/welcome/faq/
- 3: Running golangci-lint against single file fails with undeclared name error golangci/golangci-lint#1574
- 4: Would you be open to an
--includeoption? golangci/golangci-lint#420 - 5: Is it possible to exclude a single file in a package golangci/golangci-lint#6593
Use the repository-pinned linter invocation.
The lint targets run golangci-lint through tools/go.mod, but this example uses an unqualified global binary, which can fail on a clean checkout or run a different version. Update it to pinned tooling and document an appropriate package path for reliable coverage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/build-and-lint/SKILL.md around lines 29 - 30, Update the lint
commands in the build-and-lint skill to invoke golangci-lint through the
repository-pinned tooling defined by tools/go.mod rather than an unqualified
global binary. Replace the single-file example with an appropriate package path
that provides reliable coverage, and keep the gofmt check aligned with that
target.
| ``` | ||
| feat(EC-1234): add support for new attestation format | ||
| fix(EC-5678): handle empty predicate in bundle path | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the commit example fence.
This violates markdownlint MD040. Use text (or another appropriate language) for the example block.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 21-21: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/pr-checklist/SKILL.md around lines 21 - 24, Update the
commit-message example fenced block in SKILL.md to include the text language
identifier, preserving its existing example content.
Source: Linters/SAST tools
| ``` | ||
| 127.0.0.1 apiserver.localhost | ||
| 127.0.0.1 rekor.localhost | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to both fenced blocks.
markdownlint MD040 requires every fenced code block to specify a language.
.claude/skills/run-tests/SKILL.md#L67-L70: mark the/etc/hostsblock astext..claude/skills/debug-failure/SKILL.md#L38-L41: mark the/etc/hostsblock astext.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 2 files
.claude/skills/run-tests/SKILL.md#L67-L70(this comment).claude/skills/debug-failure/SKILL.md#L38-L41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/run-tests/SKILL.md around lines 67 - 70, Update the fenced
/etc/hosts blocks in .claude/skills/run-tests/SKILL.md lines 67-70 and
.claude/skills/debug-failure/SKILL.md lines 38-41 to declare the text language.
No other content changes are needed.
Source: Linters/SAST tools
Code Review by Qodo
Context used✅ Compliance rules (platform):
27 rules 1.
|
![qodo-for-conforma[bot]](https://avatars.githubusercontent.com/in/3609154?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Use `go test ./acceptance -args -persist` instead of `make acceptance -- -persist` which doesn't forward args. Jira: EC-1862
|
🤖 Finished Review · ✅ Success · Started 11:18 AM UTC · Completed 11:35 AM UTC |
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Labels: PR adds AI agent skill files under .claude/skills/ |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
|
🤖 Finished Review · ✅ Success · Started 12:31 PM UTC · Completed 12:47 PM UTC |
|
|
||
| ```bash | ||
| # CLI: preserve ec-work-* temp directories | ||
| ec validate image --debug ... |
There was a problem hiding this comment.
[medium] incorrect-technical-claim
The skill claims ec validate image --debug ... preserves ec-work-* temp directories. This is incorrect. The --debug CLI flag only controls log verbosity. Temp directory preservation is controlled exclusively by the EC_DEBUG environment variable. Note: this same error also exists in AGENTS.md.
Suggested fix: Remove the ec validate image --debug ... line or clarify it only affects log verbosity. The EC_DEBUG=1 line is correct. Also fix the same claim in AGENTS.md.
| - Context values for state passing between steps (no global state) | ||
|
|
||
| Run: `make scenario_it_works` | ||
|
|
There was a problem hiding this comment.
[low] incorrect-api-reference
The skill claims the acceptance test framework uses framework.NewFramework() for test setup, but this function does not exist in the codebase. The framework uses Godog's ScenarioContext with AddStepsTo() functions and context-based state passing via the testenv package.
Suggested fix: Replace framework.NewFramework() with an accurate description of the test setup pattern.
| --- | ||
|
|
||
| # PR Definition of Done for Conforma CLI | ||
|
|
There was a problem hiding this comment.
[low] heading-convention
The H1 heading uses a noun phrase while the other 5 skill files use imperative/verb-first phrasing. Minor inconsistency across the skill set.
|
These are all good ideas for skills, but are written more as how-to guides than skills. Skills should be reusable workflows. Here is an example of the ---
name: pr-checklist
description: >
Verify a Conforma CLI pull request is ready for review. Use when users ask
"is this PR ready", "definition of done", "PR checklist", "before merging",
"review checklist", or when preparing a PR for review.
---
# Verify PR Readiness for Conforma CLI
Run through each step below, report pass/fail for each, and summarize what remains.
## Step 1: Check for uncommitted generated code
```bash
make generate
git diff --exit-codeIf there are uncommitted changes after Step 2: Check module tidinessRun go mod tidy
git diff --exit-code go.mod go.sumIf acceptance/ or tools/ files were modified, repeat in those directories: cd acceptance && go mod tidy && git diff --exit-code go.mod go.sum
cd tools && go mod tidy && git diff --exit-code go.mod go.sumStep 3: Run lintmake lintZero warnings required. If there are failures, run Step 4: Run testsmake testIf acceptance-relevant code changed (features/, acceptance/, CLI commands), also run: make acceptanceIf behavior changed, check whether snapshots need updating: git diff features/__snapshots__/If snapshots are stale, update them with Step 5: Verify build tags on new test filesFind any new test files in the diff and verify each starts with a build tag ( git diff --name-only --diff-filter=A HEAD~1 | grep '_test\.go$'Read the first line of each new test file and flag any missing build tags. Step 6: Check PR descriptionIf a PR already exists for this branch, fetch its description and verify it covers:
Compare against the template in Step 7: ReportProduce a summary table:
List any items that need attention before the PR is ready for review. |
|
|
||
| ## Commit Messages | ||
|
|
||
| Use conventional commits with Jira key: |
There was a problem hiding this comment.
We don't want to enforce conventional commits. I see it got this from AGENTS.md, but we should remove it from there also. https://github.com/conforma/cli/blob/main/AGENTS.md?plain=1#L45
2 participants
Add 6 repo-specific AI skills under
.claude/skills/, following the pattern established in conforma/e2e-tests (EC-1929).Skills: run-tests, debug-failure, build-and-lint, pr-checklist, write-tests, benchmark.
Jira: EC-1862