Skip to content

feat(EC-1862): add AI skills for ec-cli - #3434

Open
BohdanMar wants to merge 1 commit into
conforma:mainfrom
BohdanMar:EC-1862-ai-skills
Open

feat(EC-1862): add AI skills for ec-cli#3434
BohdanMar wants to merge 1 commit into
conforma:mainfrom
BohdanMar:EC-1862-ai-skills

Conversation

@BohdanMar

@BohdanMar BohdanMar commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Six new Claude skill documents provide Conforma CLI guidance for test development, test execution, failure debugging, builds, benchmarks, and pull request preparation. AGENTS.md also clarifies the behavior of EC_DEBUG=1 and --debug.

Changes

Conforma CLI development skills

Layer / File(s) Summary
Test development, execution, and debugging workflows
.claude/skills/write-tests/SKILL.md, .claude/skills/run-tests/SKILL.md, .claude/skills/debug-failure/SKILL.md, AGENTS.md
Documents test conventions, test commands, focused acceptance execution, environment setup, failure diagnosis, artifact preservation, and the distinction between EC_DEBUG=1 and --debug.
Build, lint, benchmark, and PR readiness workflows
.claude/skills/build-and-lint/SKILL.md, .claude/skills/benchmark/SKILL.md, .claude/skills/pr-checklist/SKILL.md
Documents build and generation checks, linting, module tidiness, benchmark and profiling commands, and pull request readiness requirements.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the addition of AI skills for ec-cli.
Description check ✅ Passed The description is mostly complete and includes the change summary and Jira link, but it should use the template headings and expand the Why section.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 10:58 AM UTC · Ended 11:18 AM UTC
Commit: 87c4a29 · View workflow run →

@qodo-for-conforma

Copy link
Copy Markdown

PR Summary by Qodo

Add repo-specific Claude AI skills for ec-cli workflows

📝 Documentation ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Add six Claude skills under .claude/skills/ for common ec-cli workflows.
• Document standardized commands for tests, build/lint, debugging, and benchmarking.
• Provide a PR definition-of-done checklist to align contributions with CI expectations.
Diagram

graph TD
  Dev["Developer"] --> Claude["Claude (AI)"] --> Skills[".claude/skills/*.md"] --> Guidance["Workflow guidance"] --> Commands["make/go/ec commands"] --> Env["Docker/Podman & CI"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Single consolidated skill ("ec-cli-workflows")
  • ➕ Less duplication and fewer files to maintain
  • ➕ Easier to ensure consistent phrasing and avoid drift
  • ➖ Harder for the AI to pick the right narrow skill
  • ➖ Longer prompt context; lower signal-to-noise for specific asks
2. Reference canonical CONTRIBUTING/README docs instead of duplicating steps
  • ➕ Reduces documentation drift by keeping one source of truth
  • ➕ Encourages improving existing onboarding docs
  • ➖ Skill prompts become less self-contained
  • ➖ Existing docs may not be structured for quick, task-oriented retrieval

Recommendation: Keep the current split-by-task skills (better routing and focused guidance), but consider adding lightweight cross-links to the canonical docs (if/when they exist) to reduce future drift, especially for CI behaviors and Make targets that tend to change.

Files changed (6) +446 / -0

Other (6) +446 / -0
SKILL.mdAdd skill for running simple and stress benchmarks with profiling hints +68/-0

Add skill for running simple and stress benchmarks with profiling hints

• Introduces a benchmarking skill describing two benchmark modes (simple vs stress), required data preparation, and environment prerequisites. Documents standard Go benchmark output and how to collect runtime traces/pprof profiles via '--trace'.

.claude/skills/benchmark/SKILL.md

SKILL.mdAdd skill for build, lint, codegen, and multi-module dependency hygiene +60/-0

Add skill for build, lint, codegen, and multi-module dependency hygiene

• Adds build and lint instructions centered on Make targets (build/dist/lint/lint-fix/tekton-lint) plus single-file lint examples. Documents code generation expectations in CI, multi-module 'go mod tidy' guidance, and image/task-bundle workflows.

.claude/skills/build-and-lint/SKILL.md

SKILL.mdAdd skill for debugging test failures and common environment issues +76/-0

Add skill for debugging test failures and common environment issues

• Provides a structured troubleshooting playbook: preserving temp dirs/containers, interpreting outputs, and resolving common failures (DNS, Podman, inotify/key limits, Go checksum mismatch, snapshot updates). Includes notes about CI-specific behaviors that affect acceptance tests.

.claude/skills/debug-failure/SKILL.md

SKILL.mdAdd PR definition-of-done checklist and contribution conventions +45/-0

Add PR definition-of-done checklist and contribution conventions

• Defines a pre-submit checklist aligned to repo CI expectations ('make ci', codegen cleanliness, correct module tidying, snapshot updates). Captures commit message conventions with Jira keys and reiterates PR template expectations and code-quality guardrails.

.claude/skills/pr-checklist/SKILL.md

SKILL.mdAdd skill for running unit/integration/generative and acceptance tests +82/-0

Add skill for running unit/integration/generative and acceptance tests

• Documents core test entry points ('make test', 'make acceptance', 'make ci'), tag-based timeouts, and how to run focused tests/features/scenarios. Includes acceptance debugging options, macOS Podman setup guidance, and key environment variables.

.claude/skills/run-tests/SKILL.md

SKILL.mdAdd skill with testing patterns: build tags, snapshots, acceptance, and table tests +115/-0

Add skill with testing patterns: build tags, snapshots, acceptance, and table tests

• Adds guidance for authoring unit and integration tests (including required build tags), snapshot testing with go-snaps, and acceptance tests structure (features + Godog step definitions). Recommends naming conventions and table-driven test patterns consistent with Go practices.

.claude/skills/write-tests/SKILL.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 4ea11df and 1e119d0.

📒 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

Comment thread .claude/skills/build-and-lint/SKILL.md Outdated
Comment thread .claude/skills/pr-checklist/SKILL.md
Comment thread .claude/skills/run-tests/SKILL.md Outdated
@qodo-for-conforma

qodo-for-conforma Bot commented Jul 23, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
✅ Compliance rules (platform): 27 rules

Grey Divider


Action required

1. Broken acceptance persist command ✓ Resolved 🐞 Bug ≡ Correctness
Description
The skills instruct make acceptance -- -persist/-restore, but the acceptance Makefile target
does not forward arguments to the go test invocation, so -persist/-restore will be treated as
extra make goals and fail. This breaks the documented workflow for persisting/restoring acceptance
test environments.
Code

.claude/skills/run-tests/SKILL.md[R46-51]

+```bash
+# Keep test containers running after failure for debugging
+make acceptance -- -persist
+
+# Reattach to persisted containers
+make acceptance -- -restore
Relevance

●●● Strong

Team often accepts fixes aligning docs/commands with real behavior; this is a concrete
broken-workflow doc bug.

PR-#3331
PR-#3021

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The skills suggest passing -persist/-restore via make, but make acceptance does not
accept/forward extra flags; the repo docs for acceptance explicitly require using `go test ...
-args` for these options.

.claude/skills/run-tests/SKILL.md[46-55]
.claude/skills/debug-failure/SKILL.md[13-24]
Makefile[121-152]
acceptance/README.md[29-51]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The skills documentation tells users to run `make acceptance -- -persist` / `make acceptance -- -restore`, but the Makefile does not support passing these flags through to the acceptance test binary.

## Issue Context
- `make acceptance` runs `cd acceptance && go test -timeout ... ./...` without any forwarding mechanism for custom test flags.
- The acceptance module explicitly documents that `-persist`/`-restore` must be passed via `go test ... -args ...` and that `./...` can’t be used with `-args`.

## Fix Focus Areas
- .claude/skills/run-tests/SKILL.md[46-55]
- .claude/skills/debug-failure/SKILL.md[19-24]

## Proposed fix
Update both skills to use a working command, e.g.:

```bash
cd acceptance
# keep env
E2E_INSTRUMENTATION=true go test ./acceptance -args -persist

# restore env
E2E_INSTRUMENTATION=true go test ./acceptance -args -restore
```

(Alternatively, add a Makefile mechanism to forward acceptance test args and then document that supported interface.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread .claude/skills/run-tests/SKILL.md Outdated
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
acceptance 54.32% <ø> (+0.05%) ⬆️
generative 16.78% <ø> (-0.02%) ⬇️
integration 27.94% <ø> (-0.03%) ⬇️
unit 71.76% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:18 AM UTC · Completed 11:35 AM UTC
Commit: 87c4a29 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [incorrect-command] .claude/skills/debug-failure/SKILL.md:27 — The command cd acceptance && go test ./acceptance -args -persist will fail. After cd acceptance, the module root is github.com/conforma/cli/acceptance and ./acceptance resolves to a nonexistent acceptance/acceptance/ subdirectory. The Makefile's focus-acceptance target confirms the correct form: cd acceptance && go test . -args .... Same issue at line 33 with -restore.
    Remediation: Change ./acceptance to . in both commands (lines 27 and 33).

  • [incorrect-command] .claude/skills/run-tests/SKILL.md:62 — Same incorrect go test ./acceptance path after cd acceptance. Affects lines 62 (-persist), 66 (-restore), and 69 (-tags=@focus).
    Remediation: Replace ./acceptance with . in all three commands.

  • [protected-path] .claude/ and AGENTS.md — All 7 files in this PR are under protected paths (6 under .claude/skills/ and AGENTS.md). The PR links to Jira EC-1862 and explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Previous run

Review

Findings

High

  • [protected-path] .claude/skills/, AGENTS.md — All files in this PR are under protected paths (.claude/ and AGENTS.md). No linked GitHub issue provides authorization for these changes. The PR body references an external Jira ticket (EC-1862) but this is not verifiable within GitHub. Human approval is required for all protected-path changes.
    Remediation: Link a GitHub issue authorizing these changes, or ensure a CODEOWNER approves this PR.

Medium

  • [api-contract] .claude/skills/benchmark/SKILL.md:78 — The --trace flag examples use space-separated syntax (--trace perf, --trace cpu, --trace mem), but this flag has NoOptDefVal set in cmd/root/root_cmd.go. With pflag's NoOptDefVal, space-separated values are consumed as positional arguments, not flag values. The correct syntax is --trace=perf, --trace=cpu, --trace=mem.
    Remediation: Change all three examples to use equals syntax.

Low

  • [edge-case] .claude/skills/run-tests/SKILL.md:19git diff --name-only HEAD~1 only checks the most recent commit. On multi-commit branches, earlier changes will be missed.
    Remediation: Use git diff --name-only origin/main...HEAD.

  • [edge-case] .claude/skills/pr-checklist/SKILL.md:62 — Same HEAD~1 issue: git diff --name-only --diff-filter=A HEAD~1 misses new test files from earlier commits.
    Remediation: Use git diff --name-only --diff-filter=A origin/main...HEAD.

  • [scope-creep] AGENTS.md:44 — The "Conventional commits with Jira key encouraged" line was removed. This change is unrelated to adding AI skills and removes project guidance without justification.
    Remediation: Restore the line or provide explicit justification for its removal.

  • [missing-authorization] — No GitHub issue is linked to this PR. Authorization traces to an external Jira ticket (EC-1862), not verifiable within GitHub.

  • [naming-consistency] .claude/skills/debug-failure/SKILL.md:15 — Uses EC_DEBUG=1 while some test code uses EC_DEBUG=true. Both work (any non-empty value triggers the behavior), but consistency would reduce confusion.

  • [naming-consistency] .claude/skills/run-tests/SKILL.md:75 — The -persist/-restore acceptance test instructions are duplicated verbatim from the debug-failure skill. Consider cross-referencing instead.

  • [structure-consistency] .claude/skills/benchmark/SKILL.md — Final step naming varies across skills: "Report results", "Report", "Report findings".

  • [code-organization] .claude/skills/pr-checklist/SKILL.md:39 — Steps 1–3 duplicate the build-and-lint skill's Steps 2–4 nearly verbatim.

  • [naming-consistency] .claude/skills/write-tests/SKILL.md:11 — Title uses singular form ("Write a Test") while other skills use plural/general forms.

Previous run (2)

Review

Findings

Medium

  • [protected-path] .claude/skills/* — All 6 added files are under the .claude/ protected path (.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). The PR links to Jira EC-1862 and explains the rationale (adding AI skills following the pattern from conforma/e2e-tests EC-1929). Human approval is always required for protected-path changes, regardless of context.

  • [incorrect-technical-claim] .claude/skills/debug-failure/SKILL.md:15 — The skill claims ec validate image --debug ... preserves ec-work-* temp directories. This is incorrect. The --debug CLI flag (defined in cmd/root/root_cmd.go) only controls log verbosity. Temp directory preservation is controlled exclusively by the EC_DEBUG environment variable, checked in internal/evaluator/conftest_evaluator.go and internal/evaluator/base_evaluator.go. Note: this same incorrect claim also exists in AGENTS.md line 44, so the skill author likely copied from there.
    Remediation: Remove the ec validate image --debug ... line or change its comment to clarify it only increases log verbosity. The EC_DEBUG=1 line below it is correct and sufficient. Also fix the same claim in AGENTS.md.

Low

  • [incorrect-api-reference] .claude/skills/write-tests/SKILL.md:82 — The skill claims the acceptance test framework uses framework.NewFramework() for test setup, but this function does not exist anywhere in the codebase. The framework uses Godog's ScenarioContext with AddStepsTo() functions and context-based state passing via the testenv package.
    Remediation: Replace framework.NewFramework() with an accurate description of the test setup pattern.

  • [heading-convention] .claude/skills/pr-checklist/SKILL.md:10 — The H1 heading uses a noun phrase ('PR Definition of Done for Conforma CLI') while the other 5 skill files use imperative/verb-first phrasing. Minor inconsistency across the skill set.

  • [content-duplication] .claude/skills/run-tests/SKILL.md — The skill files reproduce content already present in AGENTS.md without a cross-reference or synchronization mechanism. CLAUDE.md explicitly states all future updates should be made to AGENTS.md. The --debug finding above demonstrates this maintenance risk — the incorrect claim was copied from AGENTS.md into the skill file.
    Remediation: Add a note in AGENTS.md flagging the skill files as downstream consumers that must be updated in parallel, or restructure skills to reference AGENTS.md sections rather than duplicating content.

Previous run (3)

Review

Findings

Medium

  • [protected-path] .claude/skills/* — All 6 files are under .claude/, a protected path requiring human approval. The PR references Jira EC-1862 and explains the rationale (adding AI skills following the pattern from conforma/e2e-tests). Human approval is required for protected-path changes regardless of review outcome.

  • [content-duplication-with-AGENTS.md] .claude/skills/run-tests/SKILL.md — Significant portions of the skill files duplicate content already in AGENTS.md (test tags/timeouts table, build commands, lint commands, troubleshooting steps, CGO/DNS info, multi-module notes). CLAUDE.md designates AGENTS.md as the single source of truth ("All future updates should be made to AGENTS.md"). Claude Code skills may need to be self-contained for independent loading, but this creates a dual-maintenance burden. Consider documenting the duplication as intentional for skill self-containment, or referencing AGENTS.md sections where possible.

  • [incorrect command invocation] .claude/skills/run-tests/SKILL.md:46 — The command cd acceptance && go test ./acceptance -args -persist is inconsistent with the Makefile's focus-acceptance target, which uses cd acceptance && go test . -args .... However, the skill copies this command from acceptance/README.md (line 44), which documents the same ./acceptance form. This is a pre-existing documentation inconsistency, not introduced by this PR. Ideally both the README and skill should align with the Makefile's go test . form.

Low

  • [incorrect API reference] .claude/skills/write-tests/SKILL.md:79 — Claims the acceptance test framework uses framework.NewFramework() for test setup. This function does not exist anywhere in the codebase (verified via grep). This is a fabricated API reference that would mislead the AI agent. Remove the bullet or replace with the actual test initialization pattern from acceptance/acceptance_test.go.

  • [description-field-formatting] .claude/skills/debug-failure/SKILL.md:3 — Trigger phrases in the description: frontmatter fields vary in style across the six files: some use quoted command strings ("run benchmark"), others mix quoted commands with bare phrases ("test failed", "debug failure"). Minor inconsistency in skill trigger phrase formatting.


Labels: PR adds AI agent skill files under .claude/skills/

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment agentic documentation Improvements or additions to documentation labels Jul 23, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 27, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:31 PM UTC · Completed 12:47 PM UTC
Commit: 87c4a29 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@joejstuart

Copy link
Copy Markdown
Contributor

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 pr-checklist refactored.

---
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-code

If there are uncommitted changes after make generate, flag them and stage the results.

Step 2: Check module tidiness

Run go mod tidy in every module that has changes, then check for drift:

go mod tidy
git diff --exit-code go.mod go.sum

If 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.sum

Step 3: Run lint

make lint

Zero warnings required. If there are failures, run make lint-fix and report what was auto-fixed vs what needs manual attention.

Step 4: Run tests

make test

If acceptance-relevant code changed (features/, acceptance/, CLI commands), also run:

make acceptance

If behavior changed, check whether snapshots need updating:

git diff features/__snapshots__/

If snapshots are stale, update them with UPDATE_SNAPS=true make acceptance and report which snapshots changed.

Step 5: Verify build tags on new test files

Find any new test files in the diff and verify each starts with a build tag (//go:build unit, integration, or generative):

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 description

If a PR already exists for this branch, fetch its description and verify it covers:

  • What: What the change does
  • Why: Context and background
  • Tickets: Link to Jira issue

Compare against the template in .github/pull_request_template.md.

Step 7: Report

Produce a summary table:

Check Status Notes
Generated code pass/fail
Module tidiness pass/fail
Lint pass/fail
Tests pass/fail
Build tags pass/fail
Commit messages pass/fail
PR description pass/fail/skip

List any items that need attention before the PR is ready for review.

Comment thread .claude/skills/pr-checklist/SKILL.md Outdated
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 9:15 AM UTC · Ended 9:16 AM UTC
Commit: 87c4a29 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 9:17 AM UTC · Ended 9:22 AM UTC
Commit: 87c4a29 · View workflow run →

@BohdanMar
BohdanMar force-pushed the EC-1862-ai-skills branch from 0f51eff to 83e15ea Compare July 29, 2026 09:21
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:23 AM UTC · Completed 9:43 AM UTC
Commit: 87c4a29 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Jul 29, 2026
Add 6 reusable workflow skills under .claude/skills/ following the
pattern established in conforma/e2e-tests (EC-1929). Each skill is
a step-by-step executable workflow with bash commands and pass/fail
reporting.

Skills: run-tests, debug-failure, build-and-lint, pr-checklist,
write-tests, benchmark.

Also fixes AGENTS.md: correct --debug claim (only EC_DEBUG=1 preserves
temp dirs) and remove conventional commits enforcement.
@BohdanMar
BohdanMar force-pushed the EC-1862-ai-skills branch from 83e15ea to ba2469b Compare July 29, 2026 11:19
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:21 AM UTC · Completed 11:38 AM UTC
Commit: 87c4a29 · View workflow run →

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 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 34-38: Update the generation verification instructions in
.claude/skills/build-and-lint/SKILL.md (lines 34-38) and
.claude/skills/pr-checklist/SKILL.md (lines 15-20) to check only expected
generated outputs or compare repository status before and after generation,
rather than using an unscoped git diff. Remove all instructions to stage or
commit generated changes in both sites.
- Around line 76-79: Use independent subshells for the acceptance and tools Go
module checks in both .claude/skills/build-and-lint/SKILL.md lines 76-79 and
.claude/skills/pr-checklist/SKILL.md lines 26-36, so each cd starts from the
original working directory and both go mod tidy/git diff checks execute
correctly.
- Around line 62-65: Update the module-detection command in the build-and-lint
skill to inspect the PR branch diff range, matching the range used by the other
skills, rather than relying only on unstaged git diff output. Ensure changed
acceptance/ and tools/ modules are detected on clean multi-commit branches,
while preserving separate handling of any local modifications if required.

In @.claude/skills/run-tests/SKILL.md:
- Around line 62-72: Update the direct acceptance test commands in the run-tests
instructions so each runs independently in a subshell, using the acceptance
module’s current package target: `(cd acceptance && go test . ...)`. Apply this
consistently to the persist, restore, and tag variants, preserving their
existing arguments.
🪄 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: ea281057-548c-40de-a492-e6b9f1134cfa

📥 Commits

Reviewing files that changed from the base of the PR and between 0ba04c5 and ba2469b.

📒 Files selected for processing (7)
  • .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
  • AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • .claude/skills/benchmark/SKILL.md
  • .claude/skills/write-tests/SKILL.md
  • .claude/skills/debug-failure/SKILL.md

Comment on lines +34 to +38
make generate
git diff --exit-code
```

If there are changes, stage and commit them. CI will fail if generated code is out of sync.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Avoid mutating the repository from an unscoped generation diff.

Both workflows can include unrelated edits in git diff and omit untracked generated files, then instruct the agent to stage changes. Restrict verification to expected generated outputs or compare status before/after, and do not stage or commit automatically.

  • .claude/skills/build-and-lint/SKILL.md#L34-L38: remove the automatic staging/commit instruction and scope the generation check.
  • .claude/skills/pr-checklist/SKILL.md#L15-L20: remove the automatic staging instruction and scope the generation check.
📍 Affects 2 files
  • .claude/skills/build-and-lint/SKILL.md#L34-L38 (this comment)
  • .claude/skills/pr-checklist/SKILL.md#L15-L20
🤖 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 34 - 38, Update the
generation verification instructions in .claude/skills/build-and-lint/SKILL.md
(lines 34-38) and .claude/skills/pr-checklist/SKILL.md (lines 15-20) to check
only expected generated outputs or compare repository status before and after
generation, rather than using an unscoped git diff. Remove all instructions to
stage or commit generated changes in both sites.

Comment on lines +62 to +65
Identify which modules have changes:

```bash
git diff --name-only | grep -E '^(acceptance/|tools/)' || echo "root module only"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use the branch diff when identifying changed modules.

git diff --name-only only reports local unstaged changes, so a clean working tree on a multi-commit PR incorrectly reports “root module only” and skips acceptance/ or tools/ tidiness checks. Use the PR range, consistent with the other skills, and separately account for local modifications if needed.

🤖 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 62 - 65, Update the
module-detection command in the build-and-lint skill to inspect the PR branch
diff range, matching the range used by the other skills, rather than relying
only on unstaged git diff output. Ensure changed acceptance/ and tools/ modules
are detected on clean multi-commit branches, while preserving separate handling
of any local modifications if required.

Comment on lines +76 to +79
cd acceptance && go mod tidy && git diff --exit-code go.mod go.sum

# If tools/ files changed
cd tools && go mod tidy && git diff --exit-code go.mod go.sum

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use independent shells for each module command.

In both workflows, the second cd is evaluated relative to the first module directory, so the tools check is skipped or fails.

  • .claude/skills/build-and-lint/SKILL.md#L76-L79: wrap the acceptance and tools commands in separate subshells.
  • .claude/skills/pr-checklist/SKILL.md#L26-L36: wrap the acceptance and tools commands in separate subshells.
📍 Affects 2 files
  • .claude/skills/build-and-lint/SKILL.md#L76-L79 (this comment)
  • .claude/skills/pr-checklist/SKILL.md#L26-L36
🤖 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 76 - 79, Use independent
subshells for the acceptance and tools Go module checks in both
.claude/skills/build-and-lint/SKILL.md lines 76-79 and
.claude/skills/pr-checklist/SKILL.md lines 26-36, so each cd starts from the
original working directory and both go mod tidy/git diff checks execute
correctly.

Comment on lines +62 to +72
These require `go test` directly, not `make`:

```bash
# Keep test containers running after failure
cd acceptance && go test ./acceptance -args -persist

# Reattach to persisted containers
cd acceptance && go test ./acceptance -args -restore

# Run with specific tags
cd acceptance && go test ./acceptance -args -tags=@focus

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Make each direct acceptance command independently runnable.

This block fails when executed sequentially: after line 66 changes into acceptance, the later cd acceptance commands target acceptance/acceptance. Also, from the acceptance module, go test ./acceptance targets the wrong package. Use subshells and go test ., for example (cd acceptance && go test . -args -persist).

🤖 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 62 - 72, Update the direct
acceptance test commands in the run-tests instructions so each runs
independently in a subshell, using the acceptance module’s current package
target: `(cd acceptance && go test . ...)`. Apply this consistently to the
persist, restore, and tag variants, preserving their existing arguments.

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review July 29, 2026 11:38

Superseded by updated review

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.


```bash
cd acceptance && go test ./acceptance -args -persist
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] incorrect-command

The command cd acceptance && go test ./acceptance -args -persist will fail. After cd acceptance, the module root is github.com/conforma/cli/acceptance and ./acceptance resolves to a nonexistent acceptance/acceptance/ subdirectory. The Makefile's focus-acceptance target confirms the correct form: cd acceptance && go test . -args .... Same issue at line 33 with -restore.

Suggested fix: Change ./acceptance to . in both commands (lines 27 and 33).


## Step 4: Debug acceptance test options

These require `go test` directly, not `make`:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] incorrect-command

Same incorrect go test ./acceptance path after cd acceptance. Affects lines 62 (-persist), 66 (-restore), and 69 (-tags=@focus).

Suggested fix: Replace ./acceptance with . in all three commands.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jul 29, 2026
@BohdanMar
BohdanMar requested a review from joejstuart July 29, 2026 12:54

@joejstuart joejstuart left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agentic documentation Improvements or additions to documentation requires-manual-review Review requires human judgment size: XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants