Add AI agent skills, guidelines, and agents#961
Add AI agent skills, guidelines, and agents#961aureliensibiril wants to merge 8 commits intogetprobo:mainfrom
Conversation
Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
There was a problem hiding this comment.
11 issues found across 35 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".claude/agents/reviewers/architecture-reviewer.md">
<violation number="1" location=".claude/agents/reviewers/architecture-reviewer.md:18">
P3: Reviewer scope contradicts itself: tests are excluded but the checklist still requires E2E test checks.</violation>
</file>
<file name=".claude/skills/potion-implement/SKILL.md">
<violation number="1" location=".claude/skills/potion-implement/SKILL.md:37">
P2: Implementer agent IDs in the skill do not match the actual registered agent names, so delegation will fail at runtime.</violation>
</file>
<file name=".claude/guidelines/go-backend/module-notes/pkg-cmd.md">
<violation number="1" location=".claude/guidelines/go-backend/module-notes/pkg-cmd.md:52">
P2: Guidance example prints a success message without checking the GraphQL call error; this can mislead implementations to report success even when the API call fails.</violation>
</file>
<file name=".claude/guidelines/typescript-frontend/patterns.md">
<violation number="1" location=".claude/guidelines/typescript-frontend/patterns.md:271">
P2: Guideline incorrectly claims GraphQL errors are in `response.errors[]` inside `onCompleted`, but repo usage expects the `errors` argument passed to `onCompleted(response, errors)`. This could mislead mutation error handling.</violation>
</file>
<file name=".claude/agents/reviewers/pattern-reviewer.md">
<violation number="1" location=".claude/agents/reviewers/pattern-reviewer.md:54">
P2: Checklist mandates `new(expr)`, but Go’s `new` only accepts a type (`new(T)`), so the rule is syntactically invalid and would produce incorrect review guidance.</violation>
</file>
<file name=".claude/agents/reviewers/style-reviewer.md">
<violation number="1" location=".claude/agents/reviewers/style-reviewer.md:47">
P2: Guideline specifies `new(expr)` for pointer literals, but Go’s `new` takes a type (e.g., `new(T)`) rather than an expression. This guidance is invalid and could lead to non-compilable recommendations.</violation>
</file>
<file name=".claude/guidelines/go-backend/module-notes/pkg-server-api.md">
<violation number="1" location=".claude/guidelines/go-backend/module-notes/pkg-server-api.md:8">
P2: The new module guideline defines conflicting API-surface rules (count/path and mandatory GraphQL+MCP+CLI), making the documented contract unreliable for implementation and review.</violation>
</file>
<file name=".claude/skills/potion-plan/SKILL.md">
<violation number="1" location=".claude/skills/potion-plan/SKILL.md:351">
P2: The path verification rule requires every mentioned path to exist and be removed if it doesn't, which contradicts the earlier requirement to list files with action `create`. New files won't exist yet, so valid create steps would fail verification or be removed.</violation>
</file>
<file name=".claude/agents/planner.md">
<violation number="1" location=".claude/agents/planner.md:302">
P2: Planner rules require every file path to pre-exist, which conflicts with the earlier requirement to plan new file creation and can block valid new-feature plans.</violation>
</file>
<file name=".claude/guidelines/typescript-frontend/pitfalls.md">
<violation number="1" location=".claude/guidelines/typescript-frontend/pitfalls.md:58">
P3: Conflicting guidance: the heading calls out missing `@appendEdge`, but the remediation tells readers to always use `@prependEdge`. Align these instructions so agents aren’t misled about which edge directive to use.</violation>
<violation number="2" location=".claude/guidelines/typescript-frontend/pitfalls.md:74">
P2: The guidance recommends inspecting raw errors in onCompleted, but the custom Relay fetch throws on the first recognized error before returning the payload, so onCompleted won’t run and the raw errors won’t be available. This remediation is ineffective.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| client := api.NewClient(hc.Host, hc.Token, "/api/console/v1/graphql", 0) | ||
|
|
||
| // 5. Execute GraphQL | ||
| result, err := client.Do(cmd.Context(), query, variables) |
There was a problem hiding this comment.
P2: Guidance example prints a success message without checking the GraphQL call error; this can mislead implementations to report success even when the API call fails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/guidelines/go-backend/module-notes/pkg-cmd.md, line 52:
<comment>Guidance example prints a success message without checking the GraphQL call error; this can mislead implementations to report success even when the API call fails.</comment>
<file context>
@@ -0,0 +1,124 @@
+ client := api.NewClient(hc.Host, hc.Token, "/api/console/v1/graphql", 0)
+
+ // 5. Execute GraphQL
+ result, err := client.Do(cmd.Context(), query, variables)
+
+ // 6. Print output
</file context>
| ### Mutation Error Handling (universal) | ||
|
|
||
| Mutations handle errors in two callbacks: | ||
| - `onCompleted` -- for GraphQL-layer errors returned in `response.errors[]` |
There was a problem hiding this comment.
P2: Guideline incorrectly claims GraphQL errors are in response.errors[] inside onCompleted, but repo usage expects the errors argument passed to onCompleted(response, errors). This could mislead mutation error handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/guidelines/typescript-frontend/patterns.md, line 271:
<comment>Guideline incorrectly claims GraphQL errors are in `response.errors[]` inside `onCompleted`, but repo usage expects the `errors` argument passed to `onCompleted(response, errors)`. This could mislead mutation error handling.</comment>
<file context>
@@ -0,0 +1,284 @@
+### Mutation Error Handling (universal)
+
+Mutations handle errors in two callbacks:
+- `onCompleted` -- for GraphQL-layer errors returned in `response.errors[]`
+- `onError` -- for network/auth errors
+
</file context>
| - [ ] Request structs with `Validate()` for mutating methods | ||
| - [ ] String-based enums in `const ()` blocks (not iota -- flagged in review) | ||
| - [ ] Grouped `type ()`, `const ()`, `var ()` blocks | ||
| - [ ] `new(expr)` for pointer literals (Go 1.26) |
There was a problem hiding this comment.
P2: Checklist mandates new(expr), but Go’s new only accepts a type (new(T)), so the rule is syntactically invalid and would produce incorrect review guidance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/agents/reviewers/pattern-reviewer.md, line 54:
<comment>Checklist mandates `new(expr)`, but Go’s `new` only accepts a type (`new(T)`), so the rule is syntactically invalid and would produce incorrect review guidance.</comment>
<file context>
@@ -0,0 +1,107 @@
+- [ ] Request structs with `Validate()` for mutating methods
+- [ ] String-based enums in `const ()` blocks (not iota -- flagged in review)
+- [ ] Grouped `type ()`, `const ()`, `var ()` blocks
+- [ ] `new(expr)` for pointer literals (Go 1.26)
+
+## Checklist -- TypeScript Frontend
</file context>
| - [ ] String-based enums (never iota) | ||
| - [ ] One argument per line or all on one line (never mixed -- approval blocker) | ||
| - [ ] Error messages: lowercase starting with "cannot" (never "failed to" -- approval blocker) | ||
| - [ ] `new(expr)` for pointer literals (Go 1.26) |
There was a problem hiding this comment.
P2: Guideline specifies new(expr) for pointer literals, but Go’s new takes a type (e.g., new(T)) rather than an expression. This guidance is invalid and could lead to non-compilable recommendations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/agents/reviewers/style-reviewer.md, line 47:
<comment>Guideline specifies `new(expr)` for pointer literals, but Go’s `new` takes a type (e.g., `new(T)`) rather than an expression. This guidance is invalid and could lead to non-compilable recommendations.</comment>
<file context>
@@ -0,0 +1,123 @@
+- [ ] String-based enums (never iota)
+- [ ] One argument per line or all on one line (never mixed -- approval blocker)
+- [ ] Error messages: lowercase starting with "cannot" (never "failed to" -- approval blocker)
+- [ ] `new(expr)` for pointer literals (Go 1.26)
+- [ ] Compile-time interface satisfaction: `var _ Interface = (*Impl)(nil)`
+
</file context>
| @@ -0,0 +1,503 @@ | |||
| --- | |||
There was a problem hiding this comment.
P2: The path verification rule requires every mentioned path to exist and be removed if it doesn't, which contradicts the earlier requirement to list files with action create. New files won't exist yet, so valid create steps would fail verification or be removed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/potion-plan/SKILL.md, line 351:
<comment>The path verification rule requires every mentioned path to exist and be removed if it doesn't, which contradicts the earlier requirement to list files with action `create`. New files won't exist yet, so valid create steps would fail verification or be removed.</comment>
<file context>
@@ -0,0 +1,503 @@
+| "Handle edge cases" | List each edge case and its expected behavior |
+
+**File path verification** -- for every file path mentioned in the plan,
+verify it exists:
+```
+Glob({ pattern: "{exact_path}" })
</file context>
|
|
||
| ## Rules | ||
|
|
||
| - Every file path in your plan must exist (verify with Glob/Grep) |
There was a problem hiding this comment.
P2: Planner rules require every file path to pre-exist, which conflicts with the earlier requirement to plan new file creation and can block valid new-feature plans.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/agents/planner.md, line 302:
<comment>Planner rules require every file path to pre-exist, which conflicts with the earlier requirement to plan new file creation and can block valid new-feature plans.</comment>
<file context>
@@ -0,0 +1,306 @@
+
+## Rules
+
+- Every file path in your plan must exist (verify with Glob/Grep)
+- Reference canonical examples, not abstract patterns
+- If a requirement is ambiguous, list what needs clarification
</file context>
|
|
||
| **Why**: `packages/relay/src/fetch.ts` short-circuits on the first `errors.find(...)` match. | ||
|
|
||
| **How to avoid**: Be aware that error boundaries will only see the first error. If you need to handle multiple error codes, inspect the raw response in the `onCompleted` callback. |
There was a problem hiding this comment.
P2: The guidance recommends inspecting raw errors in onCompleted, but the custom Relay fetch throws on the first recognized error before returning the payload, so onCompleted won’t run and the raw errors won’t be available. This remediation is ineffective.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/guidelines/typescript-frontend/pitfalls.md, line 74:
<comment>The guidance recommends inspecting raw errors in onCompleted, but the custom Relay fetch throws on the first recognized error before returning the payload, so onCompleted won’t run and the raw errors won’t be available. This remediation is ineffective.</comment>
<file context>
@@ -0,0 +1,160 @@
+
+**Why**: `packages/relay/src/fetch.ts` short-circuits on the first `errors.find(...)` match.
+
+**How to avoid**: Be aware that error boundaries will only see the first error. If you need to handle multiple error codes, inspect the raw response in the `onCompleted` callback.
+
+**Source**: Codebase pattern -- `packages/relay/src/fetch.ts` lines 85-108.
</file context>
| **How to avoid**: Be aware that error boundaries will only see the first error. If you need to handle multiple error codes, inspect the raw response in the `onCompleted` callback. | |
| **How to avoid**: Be aware that error boundaries will only see the first error. If you need to handle multiple error codes, inspect/log the raw `json.errors` in the custom fetch layer before throwing, or adjust the fetch layer to surface all errors. |
| # Probo Architecture Reviewer | ||
|
|
||
| You review code changes for **architectural correctness** only. | ||
| Do not check style, tests, or security -- other reviewers handle those. |
There was a problem hiding this comment.
P3: Reviewer scope contradicts itself: tests are excluded but the checklist still requires E2E test checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/agents/reviewers/architecture-reviewer.md, line 18:
<comment>Reviewer scope contradicts itself: tests are excluded but the checklist still requires E2E test checks.</comment>
<file context>
@@ -0,0 +1,89 @@
+# Probo Architecture Reviewer
+
+You review code changes for **architectural correctness** only.
+Do not check style, tests, or security -- other reviewers handle those.
+
+## Before reviewing
</file context>
|
|
||
| ## Relay Store Update Pitfalls | ||
|
|
||
| ### 6. Forgetting `@appendEdge`/`@deleteEdge` directives on mutations |
There was a problem hiding this comment.
P3: Conflicting guidance: the heading calls out missing @appendEdge, but the remediation tells readers to always use @prependEdge. Align these instructions so agents aren’t misled about which edge directive to use.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/guidelines/typescript-frontend/pitfalls.md, line 58:
<comment>Conflicting guidance: the heading calls out missing `@appendEdge`, but the remediation tells readers to always use `@prependEdge`. Align these instructions so agents aren’t misled about which edge directive to use.</comment>
<file context>
@@ -0,0 +1,160 @@
+
+## Relay Store Update Pitfalls
+
+### 6. Forgetting `@appendEdge`/`@deleteEdge` directives on mutations
+
+**What goes wrong**: The UI does not reflect the mutation result until a full page reload. Items appear to not be created or not be deleted.
</file context>
Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
Summary
.claude/guidelines/)/potion-ask,/potion-plan,/potion-implement,/potion-reviewGuidelines were synthesized from analysis of 35 modules, 15 existing docs in
contrib/claude/, and 25 review patterns extracted from 44 merged PRs.Test plan
/potion-askanswers questions about the codebase accurately/potion-planproduces correct multi-step plans for new features/potion-implementroutes to the correct stack-specific implementer/potion-reviewcatches known anti-patterns (e.g.NamedArgsvsStrictNamedArgs)Summary by cubic
Added agents and stack guidelines to power new
/potion-ask,/potion-plan,/potion-implement, and/potion-reviewskills. Fixed agent name references in skills; this brings consistent planning, implementation, and review across the Go backend and TypeScript frontend.New Features
.claude/guidelines/(Go and TypeScript: patterns, conventions, testing, pitfalls, module notes)./potion-ask,/potion-plan,/potion-implement,/potion-review.Bug Fixes
potion--prefixed agent names for correct delegation.Written for commit f468c80. Summary will update on new commits.