feat: overhaul Claude Code automation — agents, skills, hooks, docs#1293
Open
jeremyeder wants to merge 11 commits intoambient-code:mainfrom
Open
feat: overhaul Claude Code automation — agents, skills, hooks, docs#1293jeremyeder wants to merge 11 commits intoambient-code:mainfrom
jeremyeder wants to merge 11 commits intoambient-code:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds multiple Claude agent workflows and many new skills (align, scaffold, memory, cypress-demo, pr-fixer, dev-cluster, unleash-flag, amber-review) with eval fixtures; centralizes convention docs and operator guidance; introduces editor hooks and a git stop-hook; and removes legacy Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Skill as "Skill (Claude Skill)"
participant Agent as "Agent (convention-eval / review agents)"
participant FS as "Repo Files / Shell"
participant Report as "Markdown Report"
User->>Skill: invoke /align (optional scope)
Skill->>Agent: call Agent tool (skill: convention-eval, args: scope)
Agent->>FS: run grep/glob/bash checks across repository
FS-->>Agent: return findings (file:line, violations)
Agent->>Report: assemble structured Markdown (scores, findings)
Agent-->>Skill: return report
Skill-->>User: display report (Markdown)
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (3)
.claude/agents/frontend-review.md (1)
39-40: F2 command can be simplified (current exclusion is redundant).Because the grep only scans
src/appandsrc/components, excludingservices/api/in a second pipe does nothing.Cleanup
-grep -rn "fetch(" components/frontend/src/app/ components/frontend/src/components/ --include="*.tsx" --include="*.ts" | grep -v "services/api/" +grep -rn "fetch(" components/frontend/src/app/ components/frontend/src/components/ --include="*.tsx" --include="*.ts"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/frontend-review.md around lines 39 - 40, The grep pipeline currently pipes results into grep -v "services/api/" which is redundant because the initial paths are already limited to components/frontend/src/app and components/frontend/src/components; remove the trailing | grep -v "services/api/" and simplify the command to run a single grep over those two directories (the existing grep -rn "fetch(" with --include="*.tsx" and --include="*.ts") so the exclusion is no longer applied..claude/agents/runner-review.md (1)
24-35: Add executable detection commands for R1–R3 to keep reviews reproducible.R1–R3 are currently prose-only, unlike other agents. Adding concrete grep patterns (even heuristic) will make results more consistent run-to-run.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/runner-review.md around lines 24 - 35, The R1–R3 rules are prose-only and need concrete grep/heuristic checks to make reviews reproducible: add explicit detection commands for each rule—e.g., under R1 include patterns like "grep -RIn \"\\bopen\\(\" || grep -RIn \"requests\\.\" || grep -RIn \"time\\.sleep\\(\" and complementary positive patterns for async libs like \"aiofiles\"/\"httpx\"/\"asyncio\\.sleep\\(\"; under R2 add patterns to flag hardcoded credentials such as "grep -RIn \"\\b(password|passwd|secret|api_key|ACCESS_KEY|SECRET_KEY|AWS_SECRET)\\b\" and heuristics to ensure use of os.getenv/env/secret mounts instead of literal strings; under R3 add patterns to detect unsafe subprocess usage like "grep -RIn \"subprocess\\.run\\([^)]*\\)\" and flag missing check=True, \"subprocess\\.Popen\\(\" without \"wait()\"/\"communicate()\"/returncode checks, or use of os.system/commands without error handling; add these concrete grep examples into the R1–R3 sections (headings R1, R2, R3) so the runner can execute them automatically and mark findings reproducibly..claude/skills/jira-log/evals/evals.json (1)
1-20: LGTM! Consider expanding test coverage.The eval structure is valid and covers basic invocation patterns (direct slash command, natural language). However, the SKILL.md supports multiple issue types (Bug, Task) and priorities — consider adding test cases for:
- Issue type variations:
"input": "/jira-log Fix login bug", "expected_args": {...}(Bug type)- Priority handling:
"input": "/jira-log High priority: API timeout"- Edge cases: Empty input, malformed requests
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/jira-log/evals/evals.json around lines 1 - 20, The eval suite in evals.json only covers basic invocation patterns; add additional test entries exercising issue type variations, priority parsing, and edge cases by appending new objects to the array (e.g., a Bug-type case like input "/jira-log Fix login bug" with expected_args indicating issue type "Bug", a priority case like "/jira-log High priority: API timeout" with an expected priority field, and edge cases for empty or malformed inputs) so the "jira-log" skill is validated across types and failure modes; ensure each new test object follows the existing schema keys ("input", "expected_tool_call", "expected_args", "description") and uses clear descriptions to reflect the behavior being validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/backend-review.md:
- Around line 50-52: The grep pattern fails because BRE mode treats `\(`
specially and the `.*` is too greedy; update the command to use extended regex
(-E) and tighten the key capture to `[^"]+` so the pattern becomes an ERE
matching Object["..."].( — i.e., switch to `grep -rn -E` and replace
`Object\[".*"\]\.\(` with `Object\["[^"]+"\]\.\(` (keep the `--include="*.go"`
and the `| grep -v "_test.go"` parts unchanged).
In @.claude/agents/convention-eval.md:
- Around line 90-95: The Scoring section is ambiguous for scoped runs—update the
"Scoring" paragraph (the "Each check / Category score / Overall score" block) to
explicitly state how overall weights are handled for non-full scopes: either 1)
renormalize weights to sum to 1 across only the selected categories (provide the
formula: Overall = sum(weight_i * score_i) / sum(weight_selected)), or 2)
specify that missing categories count as zero; choose and document one option,
add a short example (e.g., frontend-only) and a note referencing "Overall score"
so readers know exactly how scoped-run scores are computed.
In @.claude/agents/operator-review.md:
- Around line 31-35: The current grep check uses only plural tokens
(Jobs|Secrets|PersistentVolumeClaims) and can miss create calls that use
singular types; update the search command used in the operator review (the grep
invocation searching components/operator/*.go) to include both singular and
plural type names (e.g.,
Job|Jobs|Secret|Secrets|PersistentVolumeClaim|PersistentVolumeClaims) and make
it case-insensitive or use -E for alternation so all create calls (e.g.,
Create(ctx, obj) for Job/Secret/PersistentVolumeClaim) are found; then for each
matched create call ensure the same function attaches OwnerReferences per the
pattern described in DEVELOPMENT.md (look for OwnerReferences assignment in the
enclosing function to validate).
In @.claude/agents/security-review.md:
- Around line 30-33: The S2 rule text is too narrow by limiting RBAC sequencing
to "write operations"; update the S2 heading/text so SelfSubjectAccessReview is
required before any resource access (not just writes). Locate the S2
rule/heading named "S2: RBAC before resource access (Critical)" and replace or
expand the phrase that currently says "precede write operations" to read
"precede resource access" (or equivalent) and ensure any explanatory sentence
and examples referencing SelfSubjectAccessReview reflect the broader scope.
In @.claude/skills/jira-log/SKILL.md:
- Around line 85-97: The confirmation prompt in the "4. Confirm Details" section
currently hardcodes "**Type**: Story"; replace that hardcoded value with the
actual parsed type variable used elsewhere (e.g., the same placeholder style as
"[extracted summary]" — use "[extracted type]" or the existing
issueType/parsedType variable) so the prompt shows Bug/Task/Story dynamically;
update the template line `**Type**: Story` to `**Type**: [extracted type]` (or
interpolate the existing parsedType/issueType symbol) so it reflects the real
parsed value.
- Around line 99-111: The Jira issue payload currently hardcodes "issue_type":
"Story" when calling mcp__jira__jira_create_issue, which ignores the parsed
issue type; update the payload to use the parsed issue type variable produced by
your parsing logic (e.g., issueType or parsedIssueType) instead of the literal
"Story", and ensure you validate or default it (fallback to "Story" only if the
parsed value is missing/invalid) before passing it to
mcp__jira__jira_create_issue.
- Around line 101-126: Update the tool calls to use the correct mcp-atlassian
namespace: replace mcp__jira__jira_create_issue with
mcp__mcp-atlassian__jira_create_issue, mcp__jira__jira_get_sprints_from_board
with mcp__mcp-atlassian__jira_get_sprints_from_board, and
mcp__jira__jira_add_issues_to_sprint with
mcp__mcp-atlassian__jira_add_issues_to_sprint wherever they are invoked;
additionally, after calling
mcp__mcp-atlassian__jira_get_sprints_from_board(checking the response from that
call), add logic to handle an empty/none active-sprints result by informing the
user that the issue was created but could not be auto-added to a sprint and
offer a manual-assignment option (do not assume an active sprint exists),
keeping the existing additional_fields JSON-string format for create/update
calls.
In @.claude/skills/memory/SKILL.md:
- Around line 34-39: The SKILL.md currently documents a hardcoded absolute path
string "/Users/jeder/.claude/projects/-Users-jeder-repos-platform/memory/" which
will break on other machines; update the Memory Location section to use a
portable pattern like "$HOME/.claude/projects/<project-slug>/memory/" and
describe how to compute <project-slug> (use the active project repo path with
"/" replaced by "-" or provide an environment variable for the project slug) so
documentation no longer references a specific developer's filesystem.
In @.claude/skills/unleash-flag/SKILL.md:
- Around line 148-163: The docs show a non-existent middleware
requireFeatureFlag; update the Backend Middleware Gating section to remove that
middleware example and instead show handler-level gating using the existing
helpers in handlers/featureflags.go: call FeatureEnabled(flagName string) for
global checks or FeatureEnabledForRequest(c *gin.Context, flagName string) to
gate inside handlers (or show a small pattern that returns 404 when the check
fails). Reference the existing symbols FeatureEnabled and
FeatureEnabledForRequest and replace the requireFeatureFlag usage in the example
with the handler-level pattern.
In `@components/operator/DEVELOPMENT.md`:
- Around line 20-28: The OwnerReference example in the doc sets
BlockOwnerDeletion via BlockOwnerDeletion: boolPtr(true), which conflicts with
the operator-safe pattern in components/operator/internal/handlers/sessions.go;
update the example to avoid setting BlockOwnerDeletion to true (either omit the
field or set it to nil/false) so child resource creates/updates do not require
elevated delete permissions—adjust the OwnerReferences block (and any helper
like boolPtr) to reflect omitting BlockOwnerDeletion or explicitly setting it to
false to match the operator's RBAC-safe behavior.
- Around line 85-91: The example incorrectly uses req.Context(); update the doc
example to use the context parameter passed into the Reconcile function (the ctx
argument of signature func (r *Reconciler) Reconcile(ctx context.Context, req
ctrl.Request) (ctrl.Result, error)), e.g. replace any uses of req.Context() with
the existing ctx variable so the snippet compiles and reflects the actual
Reconcile API.
In `@docs/security-standards.md`:
- Line 3: The relative link to CLAUDE.md is wrong in docs/security-standards.md;
update the reference from "CLAUDE.md#critical-conventions" to
"../CLAUDE.md#critical-conventions" so the link correctly resolves from the
docs/ directory to the repository root (edit the markdown link text in the line
containing the CLAUDE.md reference).
In `@scripts/claude-hooks/stop-review.sh`:
- Around line 7-10: The current if-condition only checks staged/unstaged changes
using "git diff --quiet HEAD" and "git diff --cached --quiet" and therefore
misses untracked files; update the check to also detect untracked files (e.g.,
run "git status --porcelain --untracked-files=all" or "git ls-files --others
--exclude-standard") and only exit 0 when all three checks report no
changes—modify the existing conditional that contains those two git diff
commands to include the untracked-files check so new files trigger the
/amber-review reminder.
---
Nitpick comments:
In @.claude/agents/frontend-review.md:
- Around line 39-40: The grep pipeline currently pipes results into grep -v
"services/api/" which is redundant because the initial paths are already limited
to components/frontend/src/app and components/frontend/src/components; remove
the trailing | grep -v "services/api/" and simplify the command to run a single
grep over those two directories (the existing grep -rn "fetch(" with
--include="*.tsx" and --include="*.ts") so the exclusion is no longer applied.
In @.claude/agents/runner-review.md:
- Around line 24-35: The R1–R3 rules are prose-only and need concrete
grep/heuristic checks to make reviews reproducible: add explicit detection
commands for each rule—e.g., under R1 include patterns like "grep -RIn
\"\\bopen\\(\" || grep -RIn \"requests\\.\" || grep -RIn \"time\\.sleep\\(\" and
complementary positive patterns for async libs like
\"aiofiles\"/\"httpx\"/\"asyncio\\.sleep\\(\"; under R2 add patterns to flag
hardcoded credentials such as "grep -RIn
\"\\b(password|passwd|secret|api_key|ACCESS_KEY|SECRET_KEY|AWS_SECRET)\\b\" and
heuristics to ensure use of os.getenv/env/secret mounts instead of literal
strings; under R3 add patterns to detect unsafe subprocess usage like "grep -RIn
\"subprocess\\.run\\([^)]*\\)\" and flag missing check=True,
\"subprocess\\.Popen\\(\" without \"wait()\"/\"communicate()\"/returncode
checks, or use of os.system/commands without error handling; add these concrete
grep examples into the R1–R3 sections (headings R1, R2, R3) so the runner can
execute them automatically and mark findings reproducibly.
In @.claude/skills/jira-log/evals/evals.json:
- Around line 1-20: The eval suite in evals.json only covers basic invocation
patterns; add additional test entries exercising issue type variations, priority
parsing, and edge cases by appending new objects to the array (e.g., a Bug-type
case like input "/jira-log Fix login bug" with expected_args indicating issue
type "Bug", a priority case like "/jira-log High priority: API timeout" with an
expected priority field, and edge cases for empty or malformed inputs) so the
"jira-log" skill is validated across types and failure modes; ensure each new
test object follows the existing schema keys ("input", "expected_tool_call",
"expected_args", "description") and uses clear descriptions to reflect the
behavior being validated.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 71e00d59-c67e-44c5-8895-870ec01fc79e
📒 Files selected for processing (47)
.claude/agents.claude/agents/backend-review.md.claude/agents/convention-eval.md.claude/agents/frontend-review.md.claude/agents/operator-review.md.claude/agents/runner-review.md.claude/agents/security-review.md.claude/commands/acp-compile.md.claude/commands/cypress-demo.md.claude/commands/jira.log.md.claude/commands/speckit.analyze.md.claude/commands/speckit.checklist.md.claude/commands/speckit.clarify.md.claude/commands/speckit.constitution.md.claude/commands/speckit.implement.md.claude/commands/speckit.plan.md.claude/commands/speckit.specify.md.claude/commands/speckit.tasks.md.claude/settings.json.claude/skills/align/SKILL.md.claude/skills/align/evals/evals.json.claude/skills/amber-review/SKILL.md.claude/skills/amber-review/evals/evals.json.claude/skills/cypress-demo/SKILL.md.claude/skills/cypress-demo/evals/evals.json.claude/skills/dev-cluster/SKILL.md.claude/skills/dev-cluster/evals/evals.json.claude/skills/jira-log/SKILL.md.claude/skills/jira-log/evals/evals.json.claude/skills/memory/SKILL.md.claude/skills/memory/evals/evals.json.claude/skills/pr-fixer/SKILL.md.claude/skills/pr-fixer/evals/evals.json.claude/skills/scaffold/SKILL.md.claude/skills/scaffold/evals/evals.json.claude/skills/unleash-flag/SKILL.md.claude/skills/unleash-flag/evals/evals.jsonBOOKMARKS.mdCLAUDE.mdcomponents/backend/DEVELOPMENT.mdcomponents/backend/ERROR_PATTERNS.mdcomponents/backend/K8S_CLIENT_PATTERNS.mdcomponents/frontend/DEVELOPMENT.mdcomponents/frontend/REACT_QUERY_PATTERNS.mdcomponents/operator/DEVELOPMENT.mddocs/security-standards.mdscripts/claude-hooks/stop-review.sh
💤 Files with no reviewable changes (12)
- .claude/commands/cypress-demo.md
- .claude/commands/speckit.checklist.md
- .claude/commands/acp-compile.md
- .claude/commands/jira.log.md
- .claude/commands/speckit.clarify.md
- .claude/commands/speckit.implement.md
- .claude/commands/speckit.analyze.md
- .claude/commands/speckit.specify.md
- .claude/commands/speckit.tasks.md
- .claude/agents
- .claude/commands/speckit.constitution.md
- .claude/commands/speckit.plan.md
.claude/agents/operator-review.md
Outdated
Comment on lines
+31
to
+35
| grep -rn "Jobs\|Secrets\|PersistentVolumeClaims" components/operator/ --include="*.go" | grep -i "create" | ||
| ``` | ||
|
|
||
| Cross-reference each create call with `OwnerReferences` in the same function. See `DEVELOPMENT.md` for the required pattern. | ||
|
|
Contributor
There was a problem hiding this comment.
O1 grep pattern can miss real create paths (false negatives).
Line 31 matches only plural tokens (Jobs|Secrets|PersistentVolumeClaims), while operator code often uses singular symbols (Job, Secret, PersistentVolumeClaim). This weakens a blocker check and can silently pass violations.
Suggested command fix
-grep -rn "Jobs\|Secrets\|PersistentVolumeClaims" components/operator/ --include="*.go" | grep -i "create"
+grep -rnE "Job|Jobs|Secret|Secrets|PersistentVolumeClaim|PersistentVolumeClaims" components/operator/ --include="*.go" | grep -Ei "create|Create|New"📝 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.
Suggested change
| grep -rn "Jobs\|Secrets\|PersistentVolumeClaims" components/operator/ --include="*.go" | grep -i "create" | |
| ``` | |
| Cross-reference each create call with `OwnerReferences` in the same function. See `DEVELOPMENT.md` for the required pattern. | |
| grep -rnE "Job|Jobs|Secret|Secrets|PersistentVolumeClaim|PersistentVolumeClaims" components/operator/ --include="*.go" | grep -Ei "create|Create|New" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/agents/operator-review.md around lines 31 - 35, The current grep
check uses only plural tokens (Jobs|Secrets|PersistentVolumeClaims) and can miss
create calls that use singular types; update the search command used in the
operator review (the grep invocation searching components/operator/*.go) to
include both singular and plural type names (e.g.,
Job|Jobs|Secret|Secrets|PersistentVolumeClaim|PersistentVolumeClaims) and make
it case-insensitive or use -E for alternation so all create calls (e.g.,
Create(ctx, obj) for Job/Secret/PersistentVolumeClaim) are found; then for each
matched create call ensure the same function attaches OwnerReferences per the
pattern described in DEVELOPMENT.md (look for OwnerReferences assignment in the
enclosing function to validate).
.claude/skills/jira-log/SKILL.md
Outdated
Comment on lines
+85
to
+97
| ### 4. Confirm Details | ||
|
|
||
| Before creating: | ||
| ``` | ||
| About to create RHOAIENG Jira: | ||
|
|
||
| **Summary**: [extracted summary] | ||
| **Type**: Story | ||
| **Component**: Agentic | ||
| **Team**: Ambient team | ||
|
|
||
| Shall I create this issue? (yes/no/edit) | ||
| ``` |
Contributor
There was a problem hiding this comment.
Hardcoded issue type in confirmation prompt.
Line 92 shows **Type**: Story hardcoded, but the skill supports Bug/Task types. The confirmation should display the actual parsed type:
-**Type**: Story
+**Type**: [actual issue type from parsing]📝 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.
Suggested change
| ### 4. Confirm Details | |
| Before creating: | |
| ``` | |
| About to create RHOAIENG Jira: | |
| **Summary**: [extracted summary] | |
| **Type**: Story | |
| **Component**: Agentic | |
| **Team**: Ambient team | |
| Shall I create this issue? (yes/no/edit) | |
| ``` | |
| ### 4. Confirm Details | |
| Before creating: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/jira-log/SKILL.md around lines 85 - 97, The confirmation
prompt in the "4. Confirm Details" section currently hardcodes "**Type**:
Story"; replace that hardcoded value with the actual parsed type variable used
elsewhere (e.g., the same placeholder style as "[extracted summary]" — use
"[extracted type]" or the existing issueType/parsedType variable) so the prompt
shows Bug/Task/Story dynamically; update the template line `**Type**: Story` to
`**Type**: [extracted type]` (or interpolate the existing parsedType/issueType
symbol) so it reflects the real parsed value.
Consolidate convention documentation from .claude/context/ and .claude/patterns/ into component-level files, create per-component review agents, overhaul existing skills to the Anthropic skill-creator standard, and add enforcement hooks. Infrastructure: - Fix broken .claude/agents symlink → real directory - Create .claude/settings.json with 7 PreToolUse/Stop hooks - Create scripts/claude-hooks/stop-review.sh - Delete 9 speckit + acp-compile commands Documentation consolidation: - Move context/pattern docs to component-level locations - Create components/operator/DEVELOPMENT.md - Add Critical Conventions section to CLAUDE.md - Update BOOKMARKS.md with table format and new paths - Add back-references in all component docs Skills (overhauls): - dev-cluster: pushy description, evals - pr-fixer: pre-flight checks, 30-min timeout, evals - unleash-flag: middleware gating, E2E guidance, GA cleanup lifecycle, evals Skills (promoted from commands): - amber-review, cypress-demo, jira-log → proper skills with frontmatter + evals Skills (new): - /align — convention alignment scoring via convention-eval agent - /scaffold — integration/endpoint/flag scaffolding templates - /memory — auto-memory search, audit, prune, add Agents (new): - backend-review, frontend-review, operator-review, runner-review, security-review - convention-eval (dispatched by /align) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8091349 to
9b488aa
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (5)
.claude/skills/memory/SKILL.md (1)
36-39:⚠️ Potential issue | 🟠 MajorReplace machine-specific memory path with a portable location.
Line 38 hardcodes one developer’s filesystem path, so
/memorybehavior will fail on other machines/environments.Suggested fix
All memory files live at: -``` -/Users/jeder/.claude/projects/-Users-jeder-repos-platform/memory/ -``` +```text +$HOME/.claude/projects/<project-slug>/memory/ +``` + +Where `<project-slug>` is the active repo path with `/` replaced by `-`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/memory/SKILL.md around lines 36 - 39, Replace the hardcoded machine path "/Users/jeder/.claude/projects/-Users-jeder-repos-platform/memory/" in SKILL.md with a portable template such as "$HOME/.claude/projects/<project-slug>/memory/"; document that <project-slug> is the active repo path with "/" replaced by "-" so the path works across environments and update the fenced code block and surrounding explanatory text accordingly.components/operator/DEVELOPMENT.md (2)
20-28:⚠️ Potential issue | 🟠 MajorOwnerReference example should not set
BlockOwnerDeletionto trueLine 27 conflicts with the operator’s current RBAC-safe pattern and can cause child resource create/update failures under constrained permissions.
Suggested doc fix
OwnerReferences: []metav1.OwnerReference{ { APIVersion: obj.GetAPIVersion(), Kind: obj.GetKind(), Name: obj.GetName(), UID: obj.GetUID(), Controller: boolPtr(true), - BlockOwnerDeletion: boolPtr(true), + // Omit BlockOwnerDeletion to avoid RBAC/finalizer permission issues }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/operator/DEVELOPMENT.md` around lines 20 - 28, The OwnerReference example sets BlockOwnerDeletion to true which conflicts with RBAC-safe patterns; update the OwnerReferences construction (metav1.OwnerReference in this block) to omit or set BlockOwnerDeletion to nil/false instead of boolPtr(true) so the operator does not require delete permissions on the owner. Ensure any helper boolPtr usage (e.g., boolPtr(true)) is adjusted or replaced so BlockOwnerDeletion is not enabled in the OwnerReferences example.
83-91:⚠️ Potential issue | 🟠 Major
req.Context()example is invalid for reconcile requestLine 90 shows
ctx := req.Context(), but reconcile already receivesctx context.Context; this snippet is not compilable guidance for controller-runtime reconciler code.Suggested doc fix
// Bad ctx := context.TODO() // Good — use the context passed to the reconciler -ctx := req.Context() +// Reuse the Reconcile function's ctx parameter directly +_ = ctx#!/bin/bash set -euo pipefail # Verify reconcile signatures and any req.Context() usage in operator code/docs. rg -nP --type=go 'func\s+\(\s*\w+\s+\*?\w+\s*\)\s*Reconcile\s*\(\s*ctx\s+context\.Context\s*,\s*req\s+ctrl\.Request' components/operator -C2 || true rg -nP '\breq\.Context\s*\(' components/operator -C2 || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/operator/DEVELOPMENT.md` around lines 83 - 91, The doc shows using ctx := req.Context() which is invalid for controller-runtime Reconcilers because Reconcile already receives ctx context.Context; update code and docs to use the existing ctx parameter (from the Reconcile(ctx context.Context, req ctrl.Request) signature) and remove any redeclarations or shadowing like ctx := req.Context(); search for occurrences of req.Context() inside Reconcile implementations and replace them with the passed-in ctx, and ensure the Reconcile function signature and any local variable names do not shadow the parameter..claude/skills/unleash-flag/SKILL.md (1)
148-163:⚠️ Potential issue | 🟠 Major
requireFeatureFlag()middleware guidance appears invalid for this repoLine 150 directs users to
requireFeatureFlag()inroutes.go, but this middleware was already flagged as not present; keeping this example will cause broken backend gating guidance. Replace this section with handler-level checks usingFeatureEnabled(...)/FeatureEnabledForRequest(...)(returning 404 if intentionally hidden).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/unleash-flag/SKILL.md around lines 148 - 163, The example referencing requireFeatureFlag() in routes.go is invalid because that middleware doesn't exist; update the "Backend Middleware Gating" section to remove the requireFeatureFlag() example and instead show handler-level checks using FeatureEnabled(...) or FeatureEnabledForRequest(...) inside your handlers (e.g., in ListResource/CreateResource) and return 404 when the feature is intentionally hidden; mention that this approach keeps routing intact while making endpoints non-discoverable and reference the handler names (ListResource, CreateResource) and the FeatureEnabled / FeatureEnabledForRequest helpers so readers know where to implement the checks..claude/agents/backend-review.md (1)
51-52:⚠️ Potential issue | 🟠 MajorB3 grep regex is malformed and can break the check
Line 51 uses BRE with
\(but no closing\), so the B3 scan may error and miss violations. Use ERE and a safer key pattern.Proposed fix
-grep -rn 'Object\[".*"\]\.\(' components/backend/ --include="*.go" | grep -v "_test.go" +grep -rnE 'Object\["[^"]+"\]\.\(' components/backend/ --include="*.go" | grep -v "_test.go"#!/bin/bash set -euo pipefail echo 'obj := Object["x"].(string)' > /tmp/b3_sample.go echo "Current command behavior (expect possible grep regex error):" grep -n 'Object\[".*"\]\.\(' /tmp/b3_sample.go || true echo echo "Fixed command behavior (should match line 1):" grep -nE 'Object\["[^"]+"\]\.\(' /tmp/b3_sample.go🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/backend-review.md around lines 51 - 52, The grep invocation uses a malformed BRE regex 'Object\[".*"\]\.\(' which can error; change it to use ERE and a safer key pattern by replacing the command that searches components/backend (the line invoking grep -rn 'Object\[".*"\]\.\(' components/backend/ --include="*.go" | grep -v "_test.go") with an ERE variant such as grep -rnE 'Object\["[^"]+"\]\.\(' components/backend/ --include="*.go" | grep -v "_test.go" so it correctly matches keys without relying on a problematic BRE and avoids matching across quotes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/frontend-review.md:
- Around line 39-40: The grep used to flag fetch() calls only excludes
"services/api/" so server-side route handlers under "src/app/api/" are
incorrectly reported; update the command that currently starts with grep -rn
"fetch(" to also exclude src/app/api/ (e.g., add a second pipe grep -v
"src/app/api/" or use --exclude-dir for both services/api and src/app/api) so
legitimate Next.js API route fetch calls are ignored; target the existing grep
invocation in .claude/agents/frontend-review.md.
In @.claude/skills/align/SKILL.md:
- Around line 18-25: The unlabeled fenced code blocks containing the CLI
examples (the block that lists lines like "/align # Full codebase
scan", "/align backend # Backend checks only", etc., and the similar
block around "/align security # Security checks only") are triggering
markdown-lint MD040; add an explicit fenced-code language (e.g., ```text or
```bash) to both fenced blocks so the linter recognizes them as plain
text/command examples and the docs/lint gates pass.
- Around line 49-51: The dispatch prompt in .claude/skills/align/SKILL.md still
contains the literal "[scope]" placeholder which leads to ambiguous agent
instructions; locate the prompt string (the prompt field for the convention-eval
agent) and replace the placeholder by interpolating the actual parsed scope
value (e.g., scope or parsedScope) into the string before dispatching to the
agent, ensuring the final dispatched prompt reads "Scope: backend" (or the
actual scope) rather than containing "[scope]"; update the code path that
builds/sends the prompt (the dispatch/send function that uses this prompt) to
perform the string interpolation or template substitution so the agent receives
the concrete scope.
In @.claude/skills/cypress-demo/SKILL.md:
- Around line 17-20: The fenced code block in SKILL.md (the block containing
"/cypress-demo" and "/cypress-demo chat input refactoring") is missing a
language tag; update that fenced block to use a language label (e.g., add "bash"
after the opening triple backticks) so markdownlint MD040 is satisfied and the
snippet is highlighted properly.
In @.claude/skills/memory/SKILL.md:
- Around line 18-24: The Markdown has unlabeled fenced code blocks causing
markdownlint MD040; update the two unlabeled fences that wrap the command list
(the block containing "/memory ... /memory add <topic>") and the path block (the
block containing "$HOME/.claude/projects/<project-slug>/memory/") to use a
language identifier such as text (replace ``` with ```text) so both fenced code
blocks are labeled and the MD040 warnings are resolved.
In @.claude/skills/scaffold/SKILL.md:
- Around line 19-23: The fenced code block that lists the scaffold CLI usage
(the lines starting with "/scaffold integration <name>", "/scaffold endpoint
<name>", "/scaffold feature-flag <name>") is missing a language identifier and
should be changed to a fenced block like "```text" (or "```bash") to satisfy
MD040; update the opening fence to include the chosen language identifier so the
block is properly marked as plain text/terminal usage.
- Around line 99-113: The template checks only reqK8s after calling
GetK8sClientsForRequest but can leave reqDyn nil; update the List{Resource}
handler to also guard for reqDyn == nil before using the dynamic client: after
calling GetK8sClientsForRequest(c) add a check that returns a 401 (or same
error) when reqDyn is nil, mirroring the existing reqK8s check so any subsequent
uses of reqDyn (list/get with reqDyn) are safe; locate this in the
List{Resource} function and adjust the early-return logic around reqK8s/reqDyn.
---
Duplicate comments:
In @.claude/agents/backend-review.md:
- Around line 51-52: The grep invocation uses a malformed BRE regex
'Object\[".*"\]\.\(' which can error; change it to use ERE and a safer key
pattern by replacing the command that searches components/backend (the line
invoking grep -rn 'Object\[".*"\]\.\(' components/backend/ --include="*.go" |
grep -v "_test.go") with an ERE variant such as grep -rnE
'Object\["[^"]+"\]\.\(' components/backend/ --include="*.go" | grep -v
"_test.go" so it correctly matches keys without relying on a problematic BRE and
avoids matching across quotes.
In @.claude/skills/memory/SKILL.md:
- Around line 36-39: Replace the hardcoded machine path
"/Users/jeder/.claude/projects/-Users-jeder-repos-platform/memory/" in SKILL.md
with a portable template such as
"$HOME/.claude/projects/<project-slug>/memory/"; document that <project-slug> is
the active repo path with "/" replaced by "-" so the path works across
environments and update the fenced code block and surrounding explanatory text
accordingly.
In @.claude/skills/unleash-flag/SKILL.md:
- Around line 148-163: The example referencing requireFeatureFlag() in routes.go
is invalid because that middleware doesn't exist; update the "Backend Middleware
Gating" section to remove the requireFeatureFlag() example and instead show
handler-level checks using FeatureEnabled(...) or FeatureEnabledForRequest(...)
inside your handlers (e.g., in ListResource/CreateResource) and return 404 when
the feature is intentionally hidden; mention that this approach keeps routing
intact while making endpoints non-discoverable and reference the handler names
(ListResource, CreateResource) and the FeatureEnabled / FeatureEnabledForRequest
helpers so readers know where to implement the checks.
In `@components/operator/DEVELOPMENT.md`:
- Around line 20-28: The OwnerReference example sets BlockOwnerDeletion to true
which conflicts with RBAC-safe patterns; update the OwnerReferences construction
(metav1.OwnerReference in this block) to omit or set BlockOwnerDeletion to
nil/false instead of boolPtr(true) so the operator does not require delete
permissions on the owner. Ensure any helper boolPtr usage (e.g., boolPtr(true))
is adjusted or replaced so BlockOwnerDeletion is not enabled in the
OwnerReferences example.
- Around line 83-91: The doc shows using ctx := req.Context() which is invalid
for controller-runtime Reconcilers because Reconcile already receives ctx
context.Context; update code and docs to use the existing ctx parameter (from
the Reconcile(ctx context.Context, req ctrl.Request) signature) and remove any
redeclarations or shadowing like ctx := req.Context(); search for occurrences of
req.Context() inside Reconcile implementations and replace them with the
passed-in ctx, and ensure the Reconcile function signature and any local
variable names do not shadow the parameter.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3cfd575a-6c6c-4fc5-8ec2-ab7ed4828ab6
📒 Files selected for processing (44)
.claude/agents.claude/agents/backend-review.md.claude/agents/convention-eval.md.claude/agents/frontend-review.md.claude/agents/operator-review.md.claude/agents/runner-review.md.claude/agents/security-review.md.claude/commands/acp-compile.md.claude/commands/cypress-demo.md.claude/commands/speckit.analyze.md.claude/commands/speckit.checklist.md.claude/commands/speckit.clarify.md.claude/commands/speckit.constitution.md.claude/commands/speckit.implement.md.claude/commands/speckit.plan.md.claude/commands/speckit.specify.md.claude/commands/speckit.tasks.md.claude/settings.json.claude/skills/align/SKILL.md.claude/skills/align/evals/evals.json.claude/skills/amber-review/SKILL.md.claude/skills/amber-review/evals/evals.json.claude/skills/cypress-demo/SKILL.md.claude/skills/cypress-demo/evals/evals.json.claude/skills/dev-cluster/SKILL.md.claude/skills/dev-cluster/evals/evals.json.claude/skills/memory/SKILL.md.claude/skills/memory/evals/evals.json.claude/skills/pr-fixer/SKILL.md.claude/skills/pr-fixer/evals/evals.json.claude/skills/scaffold/SKILL.md.claude/skills/scaffold/evals/evals.json.claude/skills/unleash-flag/SKILL.md.claude/skills/unleash-flag/evals/evals.jsonBOOKMARKS.mdCLAUDE.mdcomponents/backend/DEVELOPMENT.mdcomponents/backend/ERROR_PATTERNS.mdcomponents/backend/K8S_CLIENT_PATTERNS.mdcomponents/frontend/DEVELOPMENT.mdcomponents/frontend/REACT_QUERY_PATTERNS.mdcomponents/operator/DEVELOPMENT.mddocs/security-standards.mdscripts/claude-hooks/stop-review.sh
💤 Files with no reviewable changes (11)
- .claude/agents
- .claude/commands/speckit.analyze.md
- .claude/commands/speckit.constitution.md
- .claude/commands/acp-compile.md
- .claude/commands/cypress-demo.md
- .claude/commands/speckit.plan.md
- .claude/commands/speckit.checklist.md
- .claude/commands/speckit.tasks.md
- .claude/commands/speckit.clarify.md
- .claude/commands/speckit.implement.md
- .claude/commands/speckit.specify.md
✅ Files skipped from review due to trivial changes (20)
- components/backend/K8S_CLIENT_PATTERNS.md
- components/backend/ERROR_PATTERNS.md
- docs/security-standards.md
- components/frontend/REACT_QUERY_PATTERNS.md
- components/frontend/DEVELOPMENT.md
- components/backend/DEVELOPMENT.md
- .claude/skills/dev-cluster/evals/evals.json
- .claude/skills/amber-review/evals/evals.json
- .claude/skills/memory/evals/evals.json
- .claude/skills/scaffold/evals/evals.json
- .claude/skills/align/evals/evals.json
- .claude/skills/cypress-demo/evals/evals.json
- scripts/claude-hooks/stop-review.sh
- .claude/skills/pr-fixer/evals/evals.json
- .claude/skills/dev-cluster/SKILL.md
- .claude/agents/operator-review.md
- .claude/agents/security-review.md
- .claude/settings.json
- .claude/agents/convention-eval.md
- .claude/agents/runner-review.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .claude/skills/unleash-flag/evals/evals.json
- BOOKMARKS.md
- CLAUDE.md
- .claude/skills/pr-fixer/SKILL.md
- Fix regex patterns in backend-review (B3) and operator-review (O1) to match singular resource names - Clarify scoped-run scoring normalization in convention-eval - Broaden S2 RBAC check scope from write-only to all resource access - Remove hardcoded user path in memory skill, use $HOME placeholder - Fix BlockOwnerDeletion doc to match actual operator code (omitted for RBAC) - Fix context propagation doc to reference ctx param, not req.Context() - Detect untracked files in stop-review hook via git status --porcelain - Exclude src/app/api/ from F2 manual fetch check (false positives) - Resolve [scope] placeholders in align skill dispatch prompt - Add MD040 fenced-code language tags (align, cypress-demo, memory, scaffold) - Add reqDyn nil guard in scaffold handler template - Fix relative path to CLAUDE.md in security-standards.md - Replace non-existent requireFeatureFlag() with inline middleware pattern Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.claude/agents/frontend-review.md (1)
63-64: Harden the line-count command for path safetyLine 63 uses
find | xargs wc -l, which can break on filenames with spaces/newlines. Prefer null-delimited piping.Proposed robust command
-find components/frontend/src/ -name "*.tsx" | xargs wc -l | sort -rn | head -20 +find components/frontend/src/ -name "*.tsx" -print0 | xargs -0 wc -l | sort -rn | head -20🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/frontend-review.md around lines 63 - 64, Replace the unsafe pipeline "find components/frontend/src/ -name \"*.tsx\" | xargs wc -l | sort -rn | head -20" with a null-delimited or -exec form to handle filenames with spaces/newlines; e.g. use "find components/frontend/src/ -name \"*.tsx\" -print0 | xargs -0 wc -l | sort -rn | head -20" or "find components/frontend/src/ -name \"*.tsx\" -exec wc -l {} +", updating the command invocation in the file so it no longer breaks on spaces/newlines..claude/agents/backend-review.md (1)
83-85: Consider adding word boundaries to B7 pattern.The pattern
'fmt.Errorf.*%v.*err'will match any occurrence of the substring "err" within the format string, which could include words likeerror,errors,errHandler, etc.While this likely works in practice, adding word boundaries would reduce false positives:
♻️ Optional refinement with word boundaries
-grep -rn 'fmt.Errorf.*%v.*err' components/backend/ --include="*.go" | grep -v "_test.go" +grep -rn 'fmt.Errorf.*%v.*\berr\b' components/backend/ --include="*.go" | grep -v "_test.go"This ensures "err" is matched as a complete word rather than a substring.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/backend-review.md around lines 83 - 85, The grep pattern is too loose and can match "err" inside other words; update the search to use word boundaries so it only matches the standalone identifier err (for example change 'fmt.Errorf.*%v.*err' to 'fmt.Errorf.*%v.*\berr\b' and run grep with -P for Perl regex: grep -rnP 'fmt.Errorf.*%v.*\berr\b' components/backend/ --include="*.go" | grep -v "_test.go" so you target fmt.Errorf calls that pass the err variable precisely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/backend-review.md:
- Around line 58-63: Update the guidance around the grep check: change the line
that reads "Flag cases where the block body is empty or only contains a comment"
to instruct reviewers to inspect each result from the existing grep -rn "if err
!= nil {" -A1 ... command to identify truly empty blocks, and optionally replace
the naive command with a more precise multi-line check that detects an immediate
closing brace on the following line (or suggest running a short script) so the
doc no longer implies the simple grep can automatically filter empty error
blocks.
In @.claude/agents/frontend-review.md:
- Line 83: Update the documentation text that currently uses the word
"colocated" to the hyphenated form "co-located" for consistency; specifically
replace the instance of "colocated with their page in `_components/`" (the
phrase using "colocated") so the sentence reads "co-located with their page in
`_components/`".
---
Nitpick comments:
In @.claude/agents/backend-review.md:
- Around line 83-85: The grep pattern is too loose and can match "err" inside
other words; update the search to use word boundaries so it only matches the
standalone identifier err (for example change 'fmt.Errorf.*%v.*err' to
'fmt.Errorf.*%v.*\berr\b' and run grep with -P for Perl regex: grep -rnP
'fmt.Errorf.*%v.*\berr\b' components/backend/ --include="*.go" | grep -v
"_test.go" so you target fmt.Errorf calls that pass the err variable precisely.
In @.claude/agents/frontend-review.md:
- Around line 63-64: Replace the unsafe pipeline "find components/frontend/src/
-name \"*.tsx\" | xargs wc -l | sort -rn | head -20" with a null-delimited or
-exec form to handle filenames with spaces/newlines; e.g. use "find
components/frontend/src/ -name \"*.tsx\" -print0 | xargs -0 wc -l | sort -rn |
head -20" or "find components/frontend/src/ -name \"*.tsx\" -exec wc -l {} +",
updating the command invocation in the file so it no longer breaks on
spaces/newlines.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5e32db63-4837-4f81-b801-17a6aa210947
📒 Files selected for processing (13)
.claude/agents/backend-review.md.claude/agents/convention-eval.md.claude/agents/frontend-review.md.claude/agents/operator-review.md.claude/agents/security-review.md.claude/skills/align/SKILL.md.claude/skills/cypress-demo/SKILL.md.claude/skills/memory/SKILL.md.claude/skills/scaffold/SKILL.md.claude/skills/unleash-flag/SKILL.mdcomponents/operator/DEVELOPMENT.mddocs/security-standards.mdscripts/claude-hooks/stop-review.sh
✅ Files skipped from review due to trivial changes (8)
- docs/security-standards.md
- .claude/agents/security-review.md
- .claude/skills/align/SKILL.md
- .claude/skills/scaffold/SKILL.md
- .claude/agents/convention-eval.md
- .claude/skills/cypress-demo/SKILL.md
- .claude/agents/operator-review.md
- .claude/skills/memory/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/claude-hooks/stop-review.sh
…ew agents - B4: replace naive grep with multiline pattern to detect empty error blocks - B7: add word boundaries to err pattern to reduce false positives - F5: use null-delimited find/xargs for path safety - F7: fix spelling "colocated" → "co-located" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.claude/skills/memory/SKILL.md (1)
112-119:⚠️ Potential issue | 🟠 MajorAdd an explicit “never store secrets/tokens/PII” rule.
Line 112–Line 119 define exclusions, but they don’t explicitly block credentials/API tokens/PII. For a memory-persistence skill, that omission is risky and should be stated directly.
Suggested patch
## What NOT to Store - Code patterns derivable from reading current code - Git history (use `git log`) - Debugging solutions (the fix is in the code) - Anything in CLAUDE.md - Ephemeral task details +- Secrets or credentials (API keys, tokens, passwords, private keys, OAuth secrets) +- Sensitive personal data (emails, phone numbers, user identifiers) unless explicitly required and approvedBased on learnings: Never include tokens in logs, error messages, or API responses; use
len(token)for logging and provide generic messages to users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/memory/SKILL.md around lines 112 - 119, Update the "## What NOT to Store" section to explicitly prohibit storing secrets, credentials, API tokens, and PII by adding a clear rule like "Never store secrets/tokens/PII (credentials, API keys, OAuth tokens, passwords, SSNs, personal contact info)"; reference the existing exclusions (e.g., the "What NOT to Store" header and the list items) so it's added alongside them, and add a short logging guideline: never print tokens or secrets—log only token lengths (e.g., use len(token)) or generic messages and avoid including secrets in logs, errors, or API responses..claude/agents/backend-review.md (1)
58-63:⚠️ Potential issue | 🟠 MajorB4 guidance currently over-claims what the grep check can detect
Line 60 only surfaces
if err != nilblocks plus one line; it does not automatically isolate empty/comment-only bodies, so Line 63 is functionally misleading for reviewers and can skew results.Proposed fix
-Look for empty error handling blocks: +Find error handling blocks for manual inspection: ```bash grep -rn "if err != nil {" -A1 components/backend/ --include="*.go" | grep -v "_test.go"-Flag cases where the block body is empty or only contains a comment.
+Inspect each match and flag blocks whose body is empty or only a comment.</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @.claude/agents/backend-review.md around lines 58 - 63, Update the guidance
text that currently implies the grep command "grep -rn "if err != nil {" -A1
components/backend/ --include="*.go" | grep -v "_test.go"" will
automatically detect empty or comment-only error blocks; change it to explicitly
state that reviewers must inspect each match and flag blocks whose bodies are
empty or contain only comments (i.e., make the sentence read like "Inspect each
match and flag blocks whose body is empty or only a comment"), so the
instruction aligns with the actual capabilities of the grep command.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>.claude/skills/memory/evals/evals.json (1)</summary><blockquote> `1-20`: **Add one eval for default `/memory` behavior.** Consider adding a case for empty arguments (e.g., `/memory`) to lock in the documented “default to summary” behavior and prevent parser regressions. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @.claude/skills/memory/evals/evals.json around lines 1 - 20, Add an eval
entry to .claude/skills/memory/evals/evals.json that covers the empty-argument
case by adding a new object with "input": "/memory", "expected_tool_call":
"Skill", and "expected_args": { "skill": "memory", "args": "summary" } (and an
appropriate "description" like "Default to summary for /memory"), ensuring the
test suite will assert the documented default-to-summary behavior and catch
parser regressions.</details> </blockquote></details> <details> <summary>.claude/skills/pr-fixer/evals/evals.json (1)</summary><blockquote> `1-20`: **Consider one negative eval for missing PR number.** Adding a case like `/pr-fixer` (without ID) would validate the required-argument guard documented in the skill and improve resilience. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @.claude/skills/pr-fixer/evals/evals.json around lines 1 - 20, Add a negative
eval object to the existing JSON array that covers the missing-PR-id case: add
an entry with "input": "/pr-fixer" (no ID), "expected_tool_call": "None" (or
whatever the repo's convention is for no-call), and "expected_args": {} to
assert the skill "pr-fixer" rejects/does not invoke the tool when the required
PR number is omitted; follow the same object shape as the other entries so the
eval harness detects the guard against missing arguments.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/frontend-review.md:
- Around line 68-71: The F6 scanning rule currently only looks under the path
referenced near lines 75-79 ("components/frontend/src/app/") and therefore
misses components that call useQuery outside that folder; update the F6
rule/config to search all frontend component directories that may contain
query-using components (e.g., broaden the glob from
"components/frontend/src/app/" to "components/frontend/src/**" or the
repository-level frontend dirs) so any component calling useQuery is checked;
modify the rule text and any associated path/glob variables in the F6 section
(referencing F6 and useQuery) so shared/component-level usage is included.In @.claude/agents/security-review.md:
- Around line 52-54: The secret-scan grep invocation (the line starting with
grep -rn 'password.=."...' ) only includes *.go, *.py, and *.ts so it misses
common file types; update that grep command to broaden the --include globs (add
*.js, *.jsx, *.tsx, *.json, *.yaml, *.yml, *.env, Dockerfile, *.ini, etc.) or
remove --include and rely on the existing exclude filter (grep -v
"test|test|mock|example|fixture|.d.ts") so config and frontend files
are scanned; keep the existing secret regex and exclusions untouched and ensure
the command still runs in components/ as before.In @.claude/skills/unleash-flag/SKILL.md:
- Around line 181-183: Replace the hardcoded Authorization header in the E2E
example with an environment/CI-injected secret reference instead of a literal
token: update the example that sets headers: { Authorization:
'...unleash-admin-token' } to reference a placeholder/env var (e.g., using an
env variable name like UNLEASH_ADMIN_TOKEN) and add a short note in the SKILL.md
example explaining to inject that secret via CI or .env, and ensure any example
logging or error messages for the Unleash admin token use redaction (e.g., log
token length) rather than printing the token itself; look for occurrences of the
Authorization header in the file and replace them accordingly.
Duplicate comments:
In @.claude/agents/backend-review.md:
- Around line 58-63: Update the guidance text that currently implies the grep
command "grep -rn "if err != nil {" -A1 components/backend/ --include="*.go"
| grep -v "_test.go"" will automatically detect empty or comment-only error
blocks; change it to explicitly state that reviewers must inspect each match and
flag blocks whose bodies are empty or contain only comments (i.e., make the
sentence read like "Inspect each match and flag blocks whose body is empty or
only a comment"), so the instruction aligns with the actual capabilities of the
grep command.In @.claude/skills/memory/SKILL.md:
- Around line 112-119: Update the "## What NOT to Store" section to explicitly
prohibit storing secrets, credentials, API tokens, and PII by adding a clear
rule like "Never store secrets/tokens/PII (credentials, API keys, OAuth tokens,
passwords, SSNs, personal contact info)"; reference the existing exclusions
(e.g., the "What NOT to Store" header and the list items) so it's added
alongside them, and add a short logging guideline: never print tokens or
secrets—log only token lengths (e.g., use len(token)) or generic messages and
avoid including secrets in logs, errors, or API responses.
Nitpick comments:
In @.claude/skills/memory/evals/evals.json:
- Around line 1-20: Add an eval entry to .claude/skills/memory/evals/evals.json
that covers the empty-argument case by adding a new object with "input":
"/memory", "expected_tool_call": "Skill", and "expected_args": { "skill":
"memory", "args": "summary" } (and an appropriate "description" like "Default to
summary for /memory"), ensuring the test suite will assert the documented
default-to-summary behavior and catch parser regressions.In @.claude/skills/pr-fixer/evals/evals.json:
- Around line 1-20: Add a negative eval object to the existing JSON array that
covers the missing-PR-id case: add an entry with "input": "/pr-fixer" (no ID),
"expected_tool_call": "None" (or whatever the repo's convention is for no-call),
and "expected_args": {} to assert the skill "pr-fixer" rejects/does not invoke
the tool when the required PR number is omitted; follow the same object shape as
the other entries so the eval harness detects the guard against missing
arguments.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Run ID**: `af89dd7a-983a-43d4-835d-3ce9d1a32f70` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 8a2310aeb7c96a05ca5625fee0207f9c2dc15cb3 and 1ab2d8e5fec4364f43ab34020f2e6c1513a7c113. </details> <details> <summary>📒 Files selected for processing (44)</summary> * `.claude/agents` * `.claude/agents/backend-review.md` * `.claude/agents/convention-eval.md` * `.claude/agents/frontend-review.md` * `.claude/agents/operator-review.md` * `.claude/agents/runner-review.md` * `.claude/agents/security-review.md` * `.claude/commands/acp-compile.md` * `.claude/commands/cypress-demo.md` * `.claude/commands/speckit.analyze.md` * `.claude/commands/speckit.checklist.md` * `.claude/commands/speckit.clarify.md` * `.claude/commands/speckit.constitution.md` * `.claude/commands/speckit.implement.md` * `.claude/commands/speckit.plan.md` * `.claude/commands/speckit.specify.md` * `.claude/commands/speckit.tasks.md` * `.claude/settings.json` * `.claude/skills/align/SKILL.md` * `.claude/skills/align/evals/evals.json` * `.claude/skills/amber-review/SKILL.md` * `.claude/skills/amber-review/evals/evals.json` * `.claude/skills/cypress-demo/SKILL.md` * `.claude/skills/cypress-demo/evals/evals.json` * `.claude/skills/dev-cluster/SKILL.md` * `.claude/skills/dev-cluster/evals/evals.json` * `.claude/skills/memory/SKILL.md` * `.claude/skills/memory/evals/evals.json` * `.claude/skills/pr-fixer/SKILL.md` * `.claude/skills/pr-fixer/evals/evals.json` * `.claude/skills/scaffold/SKILL.md` * `.claude/skills/scaffold/evals/evals.json` * `.claude/skills/unleash-flag/SKILL.md` * `.claude/skills/unleash-flag/evals/evals.json` * `BOOKMARKS.md` * `CLAUDE.md` * `components/backend/DEVELOPMENT.md` * `components/backend/ERROR_PATTERNS.md` * `components/backend/K8S_CLIENT_PATTERNS.md` * `components/frontend/DEVELOPMENT.md` * `components/frontend/REACT_QUERY_PATTERNS.md` * `components/operator/DEVELOPMENT.md` * `docs/security-standards.md` * `scripts/claude-hooks/stop-review.sh` </details> <details> <summary>💤 Files with no reviewable changes (11)</summary> * .claude/agents * .claude/commands/acp-compile.md * .claude/commands/speckit.analyze.md * .claude/commands/speckit.checklist.md * .claude/commands/speckit.tasks.md * .claude/commands/speckit.implement.md * .claude/commands/speckit.specify.md * .claude/commands/speckit.clarify.md * .claude/commands/cypress-demo.md * .claude/commands/speckit.constitution.md * .claude/commands/speckit.plan.md </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.claude/agents/backend-review.md (1)
58-58: Hyphenate compound modifier for style consistency."error handling blocks" should be "error-handling blocks" when used as a compound adjective modifying "blocks." As per coding guidelines, compound modifiers should be hyphenated.
📝 Proposed style fix
-Look for empty error handling blocks: +Look for empty error-handling blocks:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/backend-review.md at line 58, Update the phrasing "error handling blocks" to the hyphenated compound modifier "error-handling blocks" where it appears in the comment line starting with "Look for empty error handling blocks:" so the compound adjective is styled consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/backend-review.md:
- Around line 89-95: The find/xargs pipeline shown as `find
components/backend/handlers/ -name "*.go" -not -name "*_test.go" | xargs wc -l |
sort -rn` is vulnerable to filenames with whitespace/special chars; update the
pipeline to emit and consume null-delimited paths (use find ... -print0 and
xargs -0) so wc receives safe filenames and the count/sort behavior is
preserved, leaving the sessions.go exception handling unchanged.
- Around line 36-46: The grep pattern only searches for "DynamicClient.Resource"
and misses other references to DynamicClient; update the search to cover all
DynamicClient usages and K8sClient usages by changing the pattern to
"DynamicClient\.|K8sClient\." (or use the ripgrep word-boundary pattern
'\b(DynamicClient|K8sClient)\b') when scanning components/backend/handlers/*.go
(excluding *_test.go), then review each match and ensure any List/Get usage is
replaced with a user-scoped client via GetK8sClientsForRequest(c) per the
decision tree in K8S_CLIENT_PATTERNS.md, while allowing the acceptable uses
(post-RBAC writes, token minting, cleanup).
---
Nitpick comments:
In @.claude/agents/backend-review.md:
- Line 58: Update the phrasing "error handling blocks" to the hyphenated
compound modifier "error-handling blocks" where it appears in the comment line
starting with "Look for empty error handling blocks:" so the compound adjective
is styled consistently.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dcba7449-2d0d-475c-be93-adb72936693f
📒 Files selected for processing (2)
.claude/agents/backend-review.md.claude/agents/frontend-review.md
✅ Files skipped from review due to trivial changes (1)
- .claude/agents/frontend-review.md
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.claude/skills/dev-cluster/SKILL.md (2)
129-147:⚠️ Potential issue | 🟠 MajorCommand examples violate the file’s own required
CONTAINER_ENGINErule.Line 143 says to always pass
CONTAINER_ENGINE=, but several later build examples omit it. On Podman-only environments, those examples can fail or use the wrong runtime. Please make the examples consistent with the required rule.Also applies to: 289-294, 323-329, 381-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/dev-cluster/SKILL.md around lines 129 - 147, Update all example make command invocations to always include the CONTAINER_ENGINE environment variable declared earlier; specifically modify the examples that call make (e.g., make build-frontend, make build-all and any later examples referenced at lines 289-294, 323-329, 381-385) so they pass CONTAINER_ENGINE=${CONTAINER_ENGINE} (or CONTAINER_ENGINE=docker/podman shown consistently) to avoid using the wrong runtime on Podman-only systems and to match the rule introduced where CONTAINER_ENGINE is detected.
60-63:⚠️ Potential issue | 🟠 MajorConflicting recovery guidance can lead to unsupported cluster state.
Line 62 says never manually recover failed
kind-up, but the MinIO section later prescribes manual bucket recovery commands. These instructions conflict and can cause operators to follow the wrong path.Also applies to: 355-359
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/dev-cluster/SKILL.md around lines 60 - 63, The "CRITICAL: Always Use `make kind-up`, Never Manually Recover" paragraph conflicts with the later MinIO recovery commands; update the SKILL.md so guidance is consistent by either (a) removing the manual MinIO bucket recovery commands under the "MinIO" section (lines ~355-359) and replacing them with an emergency-only subsection that explicitly directs operators to prefer rerunning `make kind-up` after `make kind-down`, or (b) keep the MinIO commands but add a clear exception to the CRITICAL heading naming "MinIO bucket recovery" as an emergency manual step and describing exact safe conditions to use it; reference the headings "CRITICAL: Always Use `make kind-up`, Never Manually Recover" and the "MinIO" section when applying the change so the two blocks present a single, non-conflicting recovery policy.
♻️ Duplicate comments (4)
.claude/skills/unleash-flag/SKILL.md (1)
182-182:⚠️ Potential issue | 🟠 MajorE2E examples still contain hardcoded admin token.
Lines 182 and 191 show
Authorization: '*:*.unleash-admin-token'as a literal string in documentation. This pattern encourages developers to commit credentials in test fixtures rather than using environment variable injection.Use environment variable injection
- headers: { Authorization: '*:*.unleash-admin-token' }, + headers: { Authorization: Cypress.env('UNLEASH_ADMIN_TOKEN') },Add a note explaining the secret should be injected via CI or local
.env, never committed.Based on learnings: Never include tokens in logs, error messages, or API responses; use
len(token)for logging and provide generic messages to users.Also applies to: 191-191
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/unleash-flag/SKILL.md at line 182, Replace the hardcoded admin token literal used in the example header (the line showing headers: { Authorization: '*:*.unleash-admin-token' } and the duplicate at line 191) with an environment-injection placeholder and add a short note instructing developers to inject the secret via CI or a local .env (never commit credentials); update the SKILL.md E2E examples to reference an env var like UNLEASH_ADMIN_TOKEN in the examples and add guidance to log only token length (e.g., len(token)) and avoid printing the token itself..claude/agents/security-review.md (1)
52-54:⚠️ Potential issue | 🔴 CriticalS7 grep still misses common credential leak locations.
The secret-scan command only checks
*.go,*.py, and*.ts, leaving*.tsx(frontend components),*.js,*.jsx, and config files (*.yaml,*.yml,*.json) unscanned. Frontend TypeScript React files and deployment manifests can contain hardcoded credentials just as easily as.tsfiles.Expand scan coverage
-grep -rn 'password.*=.*"\|api.key.*=.*"\|secret.*=.*"\|token.*=.*"' components/ --include="*.go" --include="*.py" --include="*.ts" | grep -v "_test\|test_\|mock\|example\|fixture\|\.d\.ts" +grep -rn 'password.*=.*"\|api[_-]\?key.*=.*"\|secret.*=.*"\|token.*=.*"' components/ \ + --include="*.go" --include="*.py" --include="*.ts" --include="*.tsx" --include="*.js" --include="*.jsx" \ + --include="*.yaml" --include="*.yml" --include="*.json" \ + | grep -v "_test\|test_\|mock\|example\|fixture\|\.d\.ts"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/security-review.md around lines 52 - 54, The secret-scan grep currently limits searches to *.go, *.py, and *.ts and misses frontend and config files; update the grep include list in the secret-scan command (the "grep -rn 'password.*=.*\"|api.key.*=.*\"|secret.*=.*\"|token.*=.*\"' ...") to add file globs for *.tsx, *.jsx, *.js, *.yaml, *.yml, *.json (and any other repo-specific config extensions) and ensure the --include list used with components/ covers these extensions so frontend components and deployment manifests are scanned as well..claude/agents/frontend-review.md (1)
75-76:⚠️ Potential issue | 🟠 MajorF6 grep misses shared components using React Query.
Line 76 only searches
src/app/but shared components insrc/components/can also useuseQueryand need loading/error state validation.Expand search scope
-grep -rl "useQuery\|useSessions\|useSession" components/frontend/src/app/ --include="*.tsx" +grep -rl "useQuery\|useSessions\|useSession" \ + components/frontend/src/app/ components/frontend/src/components/ \ + --include="*.tsx"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/frontend-review.md around lines 75 - 76, The grep command currently only searches src/app for hooks like useQuery, useSessions, and useSession, missing shared components; update the search to include shared component folders by running the grep against the broader source tree (e.g., components/frontend/src/) or use a glob such as components/frontend/src/**/*.{tsx,ts,jsx,js} and keep the pattern "useQuery\|useSessions\|useSession" so all files using these hooks (including shared components) are found for loading/error state validation..claude/agents/backend-review.md (1)
42-43:⚠️ Potential issue | 🔴 CriticalB2 grep pattern incomplete for DynamicClient detection.
Line 43 searches
DynamicClient\.Resourcebut misses non-method references (assignments, nil checks, returns). This weakens a Blocker-level check.Fix pattern to match K8sClient scope
-grep -rn "DynamicClient\.Resource\|K8sClient\." components/backend/handlers/ --include="*.go" | grep -v "_test.go" +grep -rn "DynamicClient\.|K8sClient\." components/backend/handlers/ --include="*.go" | grep -v "_test.go"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/backend-review.md around lines 42 - 43, The grep pattern only matches method calls like DynamicClient.Resource and K8sClient.* and misses other references (assignments, nil checks, returns); update the pattern used in the search to catch any occurrence of the types themselves—e.g. use a word-boundary regex like '\b(DynamicClient|K8sClient)\b' (or simply 'DynamicClient\|K8sClient') in the same grep command targeting components/backend/handlers/ to ensure all uses (method calls, assignments, comparisons, returns) are detected.
🧹 Nitpick comments (2)
.claude/skills/cypress-demo/SKILL.md (2)
47-50: Make secret-check guidance explicit and non-leaky.Prereq checks mention
TEST_TOKENandANTHROPIC_API_KEY, but should explicitly state to verify presence without printing values (to avoid leaking into logs/transcripts).Proposed doc edit
-- Verify `e2e/.env.test` or `e2e/.env` exists with `TEST_TOKEN` -- Check if `ANTHROPIC_API_KEY` is available (needed for Running state) +- Verify `e2e/.env.test` or `e2e/.env` exists with `TEST_TOKEN` (check presence only; never print token values) +- Check if `ANTHROPIC_API_KEY` is available (needed for Running state; check presence only, do not echo secrets)Based on learnings: Refer to docs/security-standards.md for security conventions across all components.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/cypress-demo/SKILL.md around lines 47 - 50, Update the prereq checks to instruct users to verify secrets exist without printing their values: ask them to confirm presence of TEST_TOKEN in e2e/.env.test or e2e/.env and presence of ANTHROPIC_API_KEY (for Running state) using non-leaking checks (e.g., test -n, grep -q, or environment variable existence checks) rather than echoing or outputting the values; reference docs/security-standards.md for the required security conventions and include a brief note to avoid copying secrets into logs/transcripts when checking Running state or any verification commands.
58-58: Avoid copy-pasting demo helpers into every spec.This instruction will cause helper drift across demo files. Prefer a shared Cypress support module and import helpers/constants from one place.
Proposed doc edit
-Copy the demo helpers (cursor, caption, click ripple, timing constants) from the reference implementation at `e2e/cypress/e2e/chatbox-demo.cy.ts` into each new demo file. The helpers are: `caption()`, `clearCaption()`, `initCursor()`, `moveTo()`, `moveToText()`, `clickEffect()`, `cursorClickText()`, plus timing constants (`LONG`, `PAUSE`, `SHORT`, `TYPE_DELAY`). +Use shared demo helpers from a single Cypress support module (for example, `e2e/cypress/support/demo-helpers.ts`) and import them into each new demo file. Avoid per-file copy/paste to prevent drift. Required helpers: `caption()`, `clearCaption()`, `initCursor()`, `moveTo()`, `moveToText()`, `clickEffect()`, `cursorClickText()`, plus timing constants (`LONG`, `PAUSE`, `SHORT`, `TYPE_DELAY`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/cypress-demo/SKILL.md at line 58, The demo helpers (caption, clearCaption, initCursor, moveTo, moveToText, clickEffect, cursorClickText) and timing constants (LONG, PAUSE, SHORT, TYPE_DELAY) should not be copy-pasted into every spec; instead extract them into a single shared Cypress support module (e.g., a shared helpers/support file) and update each demo spec to import these helpers/constants from that module; locate the helper implementations referenced by names caption(), clearCaption(), initCursor(), moveTo(), moveToText(), clickEffect(), cursorClickText() and the constants LONG, PAUSE, SHORT, TYPE_DELAY in your current demo (e2e/cypress/e2e/chatbox-demo.cy.ts) and move them into the shared support module, then replace the in-file copies with imports from that module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.claude/skills/dev-cluster/SKILL.md:
- Around line 129-147: Update all example make command invocations to always
include the CONTAINER_ENGINE environment variable declared earlier; specifically
modify the examples that call make (e.g., make build-frontend, make build-all
and any later examples referenced at lines 289-294, 323-329, 381-385) so they
pass CONTAINER_ENGINE=${CONTAINER_ENGINE} (or CONTAINER_ENGINE=docker/podman
shown consistently) to avoid using the wrong runtime on Podman-only systems and
to match the rule introduced where CONTAINER_ENGINE is detected.
- Around line 60-63: The "CRITICAL: Always Use `make kind-up`, Never Manually
Recover" paragraph conflicts with the later MinIO recovery commands; update the
SKILL.md so guidance is consistent by either (a) removing the manual MinIO
bucket recovery commands under the "MinIO" section (lines ~355-359) and
replacing them with an emergency-only subsection that explicitly directs
operators to prefer rerunning `make kind-up` after `make kind-down`, or (b) keep
the MinIO commands but add a clear exception to the CRITICAL heading naming
"MinIO bucket recovery" as an emergency manual step and describing exact safe
conditions to use it; reference the headings "CRITICAL: Always Use `make
kind-up`, Never Manually Recover" and the "MinIO" section when applying the
change so the two blocks present a single, non-conflicting recovery policy.
---
Duplicate comments:
In @.claude/agents/backend-review.md:
- Around line 42-43: The grep pattern only matches method calls like
DynamicClient.Resource and K8sClient.* and misses other references (assignments,
nil checks, returns); update the pattern used in the search to catch any
occurrence of the types themselves—e.g. use a word-boundary regex like
'\b(DynamicClient|K8sClient)\b' (or simply 'DynamicClient\|K8sClient') in the
same grep command targeting components/backend/handlers/ to ensure all uses
(method calls, assignments, comparisons, returns) are detected.
In @.claude/agents/frontend-review.md:
- Around line 75-76: The grep command currently only searches src/app for hooks
like useQuery, useSessions, and useSession, missing shared components; update
the search to include shared component folders by running the grep against the
broader source tree (e.g., components/frontend/src/) or use a glob such as
components/frontend/src/**/*.{tsx,ts,jsx,js} and keep the pattern
"useQuery\|useSessions\|useSession" so all files using these hooks (including
shared components) are found for loading/error state validation.
In @.claude/agents/security-review.md:
- Around line 52-54: The secret-scan grep currently limits searches to *.go,
*.py, and *.ts and misses frontend and config files; update the grep include
list in the secret-scan command (the "grep -rn
'password.*=.*\"|api.key.*=.*\"|secret.*=.*\"|token.*=.*\"' ...") to add file
globs for *.tsx, *.jsx, *.js, *.yaml, *.yml, *.json (and any other repo-specific
config extensions) and ensure the --include list used with components/ covers
these extensions so frontend components and deployment manifests are scanned as
well.
In @.claude/skills/unleash-flag/SKILL.md:
- Line 182: Replace the hardcoded admin token literal used in the example header
(the line showing headers: { Authorization: '*:*.unleash-admin-token' } and the
duplicate at line 191) with an environment-injection placeholder and add a short
note instructing developers to inject the secret via CI or a local .env (never
commit credentials); update the SKILL.md E2E examples to reference an env var
like UNLEASH_ADMIN_TOKEN in the examples and add guidance to log only token
length (e.g., len(token)) and avoid printing the token itself.
---
Nitpick comments:
In @.claude/skills/cypress-demo/SKILL.md:
- Around line 47-50: Update the prereq checks to instruct users to verify
secrets exist without printing their values: ask them to confirm presence of
TEST_TOKEN in e2e/.env.test or e2e/.env and presence of ANTHROPIC_API_KEY (for
Running state) using non-leaking checks (e.g., test -n, grep -q, or environment
variable existence checks) rather than echoing or outputting the values;
reference docs/security-standards.md for the required security conventions and
include a brief note to avoid copying secrets into logs/transcripts when
checking Running state or any verification commands.
- Line 58: The demo helpers (caption, clearCaption, initCursor, moveTo,
moveToText, clickEffect, cursorClickText) and timing constants (LONG, PAUSE,
SHORT, TYPE_DELAY) should not be copy-pasted into every spec; instead extract
them into a single shared Cypress support module (e.g., a shared helpers/support
file) and update each demo spec to import these helpers/constants from that
module; locate the helper implementations referenced by names caption(),
clearCaption(), initCursor(), moveTo(), moveToText(), clickEffect(),
cursorClickText() and the constants LONG, PAUSE, SHORT, TYPE_DELAY in your
current demo (e2e/cypress/e2e/chatbox-demo.cy.ts) and move them into the shared
support module, then replace the in-file copies with imports from that module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 694e7051-0901-40f5-8d2c-b7a8b4cc0b77
📒 Files selected for processing (44)
.claude/agents.claude/agents/backend-review.md.claude/agents/convention-eval.md.claude/agents/frontend-review.md.claude/agents/operator-review.md.claude/agents/runner-review.md.claude/agents/security-review.md.claude/commands/acp-compile.md.claude/commands/cypress-demo.md.claude/commands/speckit.analyze.md.claude/commands/speckit.checklist.md.claude/commands/speckit.clarify.md.claude/commands/speckit.constitution.md.claude/commands/speckit.implement.md.claude/commands/speckit.plan.md.claude/commands/speckit.specify.md.claude/commands/speckit.tasks.md.claude/settings.json.claude/skills/align/SKILL.md.claude/skills/align/evals/evals.json.claude/skills/amber-review/SKILL.md.claude/skills/amber-review/evals/evals.json.claude/skills/cypress-demo/SKILL.md.claude/skills/cypress-demo/evals/evals.json.claude/skills/dev-cluster/SKILL.md.claude/skills/dev-cluster/evals/evals.json.claude/skills/memory/SKILL.md.claude/skills/memory/evals/evals.json.claude/skills/pr-fixer/SKILL.md.claude/skills/pr-fixer/evals/evals.json.claude/skills/scaffold/SKILL.md.claude/skills/scaffold/evals/evals.json.claude/skills/unleash-flag/SKILL.md.claude/skills/unleash-flag/evals/evals.jsonBOOKMARKS.mdCLAUDE.mdcomponents/backend/DEVELOPMENT.mdcomponents/backend/ERROR_PATTERNS.mdcomponents/backend/K8S_CLIENT_PATTERNS.mdcomponents/frontend/DEVELOPMENT.mdcomponents/frontend/REACT_QUERY_PATTERNS.mdcomponents/operator/DEVELOPMENT.mddocs/security-standards.mdscripts/claude-hooks/stop-review.sh
💤 Files with no reviewable changes (11)
- .claude/agents
- .claude/commands/acp-compile.md
- .claude/commands/speckit.constitution.md
- .claude/commands/speckit.analyze.md
- .claude/commands/speckit.tasks.md
- .claude/commands/cypress-demo.md
- .claude/commands/speckit.plan.md
- .claude/commands/speckit.specify.md
- .claude/commands/speckit.clarify.md
- .claude/commands/speckit.checklist.md
- .claude/commands/speckit.implement.md
- F6: broaden useQuery scan to include components/ directory - S7: extend secret-scan grep to tsx, js, yaml, yml file types - Unleash E2E: replace hardcoded admin token with Cypress.env() - Memory skill: add explicit secrets/PII exclusion rule - Memory evals: add default /memory (no args) test case - PR fixer evals: add negative test for missing PR number Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/unleash-flag/SKILL.md:
- Around line 150-166: The middleware example uses an unqualified,
request-agnostic FeatureEnabled call and must be updated to call the
package-qualified, request-aware check; replace
FeatureEnabled("category.feature.enabled") with
handlers.FeatureEnabledForRequest(c, "category.feature.enabled") (or at minimum
handlers.FeatureEnabled(...)) inside the anonymous middleware for the flagged
router group so the Unleash strategies (user/IP/context) are respected and the
code compiles in package main.
- Around line 182-183: The call to Cypress.env('UNLEASH_ADMIN_TOKEN') can
resolve empty because there's no env mapping; update SKILL.md to instruct users
to set the token (e.g., export CYPRESS_UNLEASH_ADMIN_TOKEN) or add an explicit
mapping inside your e2e/cypress.config.ts setupNodeEvents so
Cypress.env('UNLEASH_ADMIN_TOKEN') is populated; reference the usage of
Cypress.env('UNLEASH_ADMIN_TOKEN') in the doc (lines around the headers: {
Authorization: Cypress.env('UNLEASH_ADMIN_TOKEN') }) and note the same guidance
for the other occurrence near line 191.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1c3a9409-90e3-4d53-931f-4c5f3b7b34f6
📒 Files selected for processing (6)
.claude/agents/frontend-review.md.claude/agents/security-review.md.claude/skills/memory/SKILL.md.claude/skills/memory/evals/evals.json.claude/skills/pr-fixer/evals/evals.json.claude/skills/unleash-flag/SKILL.md
✅ Files skipped from review due to trivial changes (4)
- .claude/skills/memory/evals/evals.json
- .claude/agents/security-review.md
- .claude/skills/memory/SKILL.md
- .claude/agents/frontend-review.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .claude/skills/pr-fixer/evals/evals.json
- Use FeatureEnabledForRequest(c, ...) for request-aware flag evaluation - Add CYPRESS_UNLEASH_ADMIN_TOKEN env var setup instructions for E2E tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/skills/dev-cluster/SKILL.md (1)
351-362:⚠️ Potential issue | 🟠 MajorConflicting recovery guidance will cause incorrect operator behavior.
Line 351-359 instructs manual bucket recovery, which conflicts with the hard rule on Line 60-63 (“Never manually recover”). Keep one authoritative path; otherwise the skill can execute the exact flow it forbids.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/dev-cluster/SKILL.md around lines 351 - 362, The recovery steps under "Sessions fail with init-hydrate exit code 1" conflict with the global "Never manually recover" rule; update SKILL.md to remove the manual MinIO bucket creation lines (the kubectl/mc commands) and instead direct operators to re-run make kind-up or rerun init-minio.sh (referencing init-minio.sh and make kind-up), and add a single sentence that manual bucket creation is a last-resort only with explicit approval if absolutely necessary to recover ambient-sessions.
♻️ Duplicate comments (3)
.claude/skills/unleash-flag/SKILL.md (1)
150-157:⚠️ Potential issue | 🟠 MajorBackend middleware example uses an out-of-scope symbol (
FeatureEnabled) and won’t compile if copied intoroutes.go.
At Line 156,FeatureEnabled(...)is unqualified. Incomponents/backend/routes.go, this should callhandlers.FeatureEnabledForRequest(c, ...)(orhandlers.FeatureEnabled(...)) to resolve correctly.Proposed doc fix
flagged := router.Group("/api/v1/feature") flagged.Use(func(c *gin.Context) { - if !FeatureEnabled("category.feature.enabled") { + if !handlers.FeatureEnabledForRequest(c, "category.feature.enabled") { c.AbortWithStatus(http.StatusNotFound) return } c.Next() })#!/bin/bash set -euo pipefail echo "1) Confirm where FeatureEnabled helpers are actually defined:" rg -nP --type=go '\bfunc\s+FeatureEnabled(?:ForRequest)?\s*\(' components/backend echo echo "2) Show package declaration in routes.go (copy/paste target context):" fd -i 'routes.go' components/backend --exec sed -n '1,40p' {} echo echo "3) Show the current SKILL.md middleware snippet lines:" sed -n '148,166p' .claude/skills/unleash-flag/SKILL.md🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/unleash-flag/SKILL.md around lines 150 - 157, The example middleware in SKILL.md uses an unqualified FeatureEnabled symbol that won't compile; update the snippet to call the actual helper from the handlers package used in the backend (e.g. replace FeatureEnabled("category.feature.enabled") with handlers.FeatureEnabledForRequest(c, "category.feature.enabled") or handlers.FeatureEnabled("category.feature.enabled") depending on which helper exists), and ensure the middleware receives the gin.Context (c) when calling the request-scoped variant so the route group in routes.go will compile..claude/agents/backend-review.md (1)
92-93:⚠️ Potential issue | 🟡 MinorB8 command is not path-safe and can miscount files.
Using plain
find | xargswill break on filenames with spaces/special characters, so the size check can produce incorrect results.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/backend-review.md around lines 92 - 93, The shell pipeline using "find components/backend/handlers/ -name \"*.go\" -not -name \"*_test.go\" | xargs wc -l | sort -rn" is not path-safe and will break on filenames with spaces/special chars; replace it with a null-delimited-safe form such as using "-print0" with "xargs -0" (e.g. "find ... -print0 | xargs -0 wc -l | sort -rn") or use "find ... -exec wc -l {} +" to safely pass filenames to wc; update the command in the document where the original pipeline appears..claude/agents/security-review.md (1)
52-54:⚠️ Potential issue | 🟠 MajorS7 scan still has blind spots for credential leaks.
The include list omits common credential-carrying files (notably
.jsonand.env), so this “Blocker” check can miss real hardcoded secrets. Expand scan scope to avoid false negatives.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/security-review.md around lines 52 - 54, The grep include list in the security scan command (the line starting with grep -rn 'password.*=.*"...' that uses multiple --include="*.go" --include="*.py"... ) omits common credential-carrying filenames like .json and .env, so expand the --include list to add --include="*.json" and --include=".env" (or other project-relevant globs such as --include="*.ini" or "*.env.*" if you use env files with extensions) so the blocker check will catch hardcoded secrets in those files; update the command in .claude/agents/security-review.md accordingly.
🧹 Nitpick comments (2)
.claude/skills/pr-fixer/evals/evals.json (1)
1-26: Consider expanding test coverage for all documented invocation patterns.The current evals cover the primary invocation patterns, but some examples from SKILL.md are not tested:
From the description (lines 6-7):
- ✅ "fix PR" (covered)
- ✅ "run pr-fixer" (covered)
- ❌ "address PR comments" (not covered)
- ❌ "auto-fix pull request" (not covered)
From "When to Invoke This Skill" (lines 72-76):
- ❌ "Trigger pr-fixer for PR
#1234" (not covered)💡 Suggested additional test cases
+ { + "input": "address PR comments on 1234", + "expected_tool_call": "Skill", + "expected_args": { "skill": "pr-fixer", "args": "1234" }, + "description": "Natural language: address PR comments" + }, + { + "input": "auto-fix pull request 1234", + "expected_tool_call": "Skill", + "expected_args": { "skill": "pr-fixer", "args": "1234" }, + "description": "Natural language: auto-fix pull request" + }, + { + "input": "trigger pr-fixer for PR `#1234`", + "expected_tool_call": "Skill", + "expected_args": { "skill": "pr-fixer", "args": "1234" }, + "description": "Natural language: trigger pr-fixer" + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/pr-fixer/evals/evals.json around lines 1 - 26, The evals JSON is missing test cases for documented invocation patterns; add new objects to the array in .claude/skills/pr-fixer/evals/evals.json for the phrases "address PR comments", "auto-fix pull request", and "Trigger pr-fixer for PR `#1234`" (and a variant with a hash like "Trigger pr-fixer for PR `#1234`") using the same object shape as existing entries: set "expected_tool_call" to "Skill" and "expected_args" to { "skill": "pr-fixer", "args": "1234" } (or "987" where appropriate), and include descriptive "description" fields to match the documented examples so the eval covers all invocation patterns referenced in SKILL.md..claude/agents/operator-review.md (1)
31-32: Grep pattern works for current codebase but could be more defensive.The current pattern successfully catches all Secret and PersistentVolumeClaim Create calls in the operator code. However, it relies on type names appearing on the same line as Create() calls. For defensive coding and potential future refactoring, explicitly including plural forms and word boundaries would be more robust.
Enhanced pattern for clarity
-grep -rn "Job\|Secret\|PersistentVolumeClaim" components/operator/ --include="*.go" | grep -i "create" +grep -rnE "\b(Job|Jobs|Secret|Secrets|PersistentVolumeClaim|PersistentVolumeClaims)\b" components/operator/ --include="*.go" | grep -i "create"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/operator-review.md around lines 31 - 32, Update the grep command to be more defensive by using an extended regex with word boundaries and plural forms so it matches type names and their plurals regardless of case or line layout; specifically, replace the simple pattern that searches for "Job|Secret|PersistentVolumeClaim" with an -E pattern using \b(Job|Jobs|Secret|Secrets|PersistentVolumeClaim|PersistentVolumeClaims)\b and make the Create match case-insensitive (e.g., match \bcreate\b or use -i), ensuring the grep invocation that currently targets components/operator and *.go files will catch Create calls even if type names or Create() are split or refactored elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/pr-fixer/SKILL.md:
- Around line 22-26: Update the `gh pr view` invocation in the "Pre-flight
checks" section: remove the unsupported `-q .state` usage and use `--json state`
alone, or if downstream parsing is required pipe the output to `jq -r '.state'`;
replace the current `gh pr view <N> --repo <owner/repo> --json state -q .state`
example with one of these corrected forms so the state extraction works with the
GH CLI.
In @.claude/skills/scaffold/SKILL.md:
- Around line 41-62: The scaffold references missing frontend and runner
template files which break generation: add the missing templates
`jira-connection-card.tsx`, `IntegrationsClient.tsx`, and
`integrations-panel.tsx` under
`components/frontend/src/components/integrations/` (modeled on the backend
patterns) and add an `auth.py` with `fetch_jira_credentials()` under
`components/runners/ambient-runner/src/` so `fetch_{provider}_credentials()` can
follow that pattern; alternatively edit SKILL.md to stop referencing those
nonexistent files and instead point to existing backend templates (e.g.,
`jira_auth.go`) and available frontend/runner patterns to ensure the scaffold
only references real artifacts.
---
Outside diff comments:
In @.claude/skills/dev-cluster/SKILL.md:
- Around line 351-362: The recovery steps under "Sessions fail with init-hydrate
exit code 1" conflict with the global "Never manually recover" rule; update
SKILL.md to remove the manual MinIO bucket creation lines (the kubectl/mc
commands) and instead direct operators to re-run make kind-up or rerun
init-minio.sh (referencing init-minio.sh and make kind-up), and add a single
sentence that manual bucket creation is a last-resort only with explicit
approval if absolutely necessary to recover ambient-sessions.
---
Duplicate comments:
In @.claude/agents/backend-review.md:
- Around line 92-93: The shell pipeline using "find components/backend/handlers/
-name \"*.go\" -not -name \"*_test.go\" | xargs wc -l | sort -rn" is not
path-safe and will break on filenames with spaces/special chars; replace it with
a null-delimited-safe form such as using "-print0" with "xargs -0" (e.g. "find
... -print0 | xargs -0 wc -l | sort -rn") or use "find ... -exec wc -l {} +" to
safely pass filenames to wc; update the command in the document where the
original pipeline appears.
In @.claude/agents/security-review.md:
- Around line 52-54: The grep include list in the security scan command (the
line starting with grep -rn 'password.*=.*"...' that uses multiple
--include="*.go" --include="*.py"... ) omits common credential-carrying
filenames like .json and .env, so expand the --include list to add
--include="*.json" and --include=".env" (or other project-relevant globs such as
--include="*.ini" or "*.env.*" if you use env files with extensions) so the
blocker check will catch hardcoded secrets in those files; update the command in
.claude/agents/security-review.md accordingly.
In @.claude/skills/unleash-flag/SKILL.md:
- Around line 150-157: The example middleware in SKILL.md uses an unqualified
FeatureEnabled symbol that won't compile; update the snippet to call the actual
helper from the handlers package used in the backend (e.g. replace
FeatureEnabled("category.feature.enabled") with
handlers.FeatureEnabledForRequest(c, "category.feature.enabled") or
handlers.FeatureEnabled("category.feature.enabled") depending on which helper
exists), and ensure the middleware receives the gin.Context (c) when calling the
request-scoped variant so the route group in routes.go will compile.
---
Nitpick comments:
In @.claude/agents/operator-review.md:
- Around line 31-32: Update the grep command to be more defensive by using an
extended regex with word boundaries and plural forms so it matches type names
and their plurals regardless of case or line layout; specifically, replace the
simple pattern that searches for "Job|Secret|PersistentVolumeClaim" with an -E
pattern using
\b(Job|Jobs|Secret|Secrets|PersistentVolumeClaim|PersistentVolumeClaims)\b and
make the Create match case-insensitive (e.g., match \bcreate\b or use -i),
ensuring the grep invocation that currently targets components/operator and *.go
files will catch Create calls even if type names or Create() are split or
refactored elsewhere.
In @.claude/skills/pr-fixer/evals/evals.json:
- Around line 1-26: The evals JSON is missing test cases for documented
invocation patterns; add new objects to the array in
.claude/skills/pr-fixer/evals/evals.json for the phrases "address PR comments",
"auto-fix pull request", and "Trigger pr-fixer for PR `#1234`" (and a variant with
a hash like "Trigger pr-fixer for PR `#1234`") using the same object shape as
existing entries: set "expected_tool_call" to "Skill" and "expected_args" to {
"skill": "pr-fixer", "args": "1234" } (or "987" where appropriate), and include
descriptive "description" fields to match the documented examples so the eval
covers all invocation patterns referenced in SKILL.md.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d16ee9cd-d925-4236-aabb-cde80c720564
📒 Files selected for processing (44)
.claude/agents.claude/agents/backend-review.md.claude/agents/convention-eval.md.claude/agents/frontend-review.md.claude/agents/operator-review.md.claude/agents/runner-review.md.claude/agents/security-review.md.claude/commands/acp-compile.md.claude/commands/cypress-demo.md.claude/commands/speckit.analyze.md.claude/commands/speckit.checklist.md.claude/commands/speckit.clarify.md.claude/commands/speckit.constitution.md.claude/commands/speckit.implement.md.claude/commands/speckit.plan.md.claude/commands/speckit.specify.md.claude/commands/speckit.tasks.md.claude/settings.json.claude/skills/align/SKILL.md.claude/skills/align/evals/evals.json.claude/skills/amber-review/SKILL.md.claude/skills/amber-review/evals/evals.json.claude/skills/cypress-demo/SKILL.md.claude/skills/cypress-demo/evals/evals.json.claude/skills/dev-cluster/SKILL.md.claude/skills/dev-cluster/evals/evals.json.claude/skills/memory/SKILL.md.claude/skills/memory/evals/evals.json.claude/skills/pr-fixer/SKILL.md.claude/skills/pr-fixer/evals/evals.json.claude/skills/scaffold/SKILL.md.claude/skills/scaffold/evals/evals.json.claude/skills/unleash-flag/SKILL.md.claude/skills/unleash-flag/evals/evals.jsonBOOKMARKS.mdCLAUDE.mdcomponents/backend/DEVELOPMENT.mdcomponents/backend/ERROR_PATTERNS.mdcomponents/backend/K8S_CLIENT_PATTERNS.mdcomponents/frontend/DEVELOPMENT.mdcomponents/frontend/REACT_QUERY_PATTERNS.mdcomponents/operator/DEVELOPMENT.mddocs/security-standards.mdscripts/claude-hooks/stop-review.sh
💤 Files with no reviewable changes (11)
- .claude/agents
- .claude/commands/acp-compile.md
- .claude/commands/speckit.tasks.md
- .claude/commands/speckit.specify.md
- .claude/commands/speckit.checklist.md
- .claude/commands/speckit.analyze.md
- .claude/commands/cypress-demo.md
- .claude/commands/speckit.constitution.md
- .claude/commands/speckit.implement.md
- .claude/commands/speckit.plan.md
- .claude/commands/speckit.clarify.md
- pr-fixer: fix gh CLI flag from -q to --jq for state extraction - dev-cluster: remove manual MinIO bucket recovery (conflicts with "never manually recover" rule) - dev-cluster: add CONTAINER_ENGINE to build examples for consistency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
.claude/agents/backend-review.md (1)
91-93:⚠️ Potential issue | 🟡 MinorUse null-delimited
find/xargsin B8 to avoid broken file counting.Line 92 can split paths on whitespace, which can skew or fail the line-count check.
Proposed fix
-find components/backend/handlers/ -name "*.go" -not -name "*_test.go" | xargs wc -l | sort -rn +find components/backend/handlers/ -name "*.go" -not -name "*_test.go" -print0 | xargs -0 wc -l | sort -rn#!/bin/bash set -euo pipefail echo "Inspect B8 command:" sed -n '89,94p' .claude/agents/backend-review.md🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/backend-review.md around lines 91 - 93, The find+xargs command in the snippet (the pipeline using find components/backend/handlers/ -name "*.go" -not -name "*_test.go" | xargs wc -l | sort -rn) can break on filenames containing whitespace; update it to use null-delimited output and input (find ... -print0 and xargs -0) so wc receives correct paths and line counts; locate the command in the file and replace the plain find/xargs pipeline with the null-safe variant using -print0 on find and -0 on xargs..claude/skills/unleash-flag/SKILL.md (1)
152-166:⚠️ Potential issue | 🟠 MajorPackage-qualify
FeatureEnabledForRequestin theroutes.goexample.Line 156 uses
FeatureEnabledForRequest(...)unqualified. Inroutes.go, this should behandlers.FeatureEnabledForRequest(...)(with thehandlersimport), otherwise the copied example won’t compile.Proposed doc fix
flagged := router.Group("/api/v1/feature") flagged.Use(func(c *gin.Context) { - if !FeatureEnabledForRequest(c, "category.feature.enabled") { + if !handlers.FeatureEnabledForRequest(c, "category.feature.enabled") { c.AbortWithStatus(http.StatusNotFound) return } c.Next() })#!/bin/bash set -euo pipefail echo "Check current SKILL example usage:" sed -n '148,166p' .claude/skills/unleash-flag/SKILL.md echo echo "Check actual helper definition package/symbol:" sed -n '1,40p' components/backend/handlers/featureflags.go🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/unleash-flag/SKILL.md around lines 152 - 166, The example in routes.go calls FeatureEnabledForRequest(...) unqualified which won't compile; update that call to handlers.FeatureEnabledForRequest(...) in the code block and ensure the handlers package is imported in the routes file. Locate the route-group example (the flagged router.Group block) and replace FeatureEnabledForRequest with handlers.FeatureEnabledForRequest, and confirm the handlers package that defines FeatureEnabledForRequest (function FeatureEnabledForRequest in featureflags.go) is present in the imports..claude/skills/scaffold/SKILL.md (1)
51-57:⚠️ Potential issue | 🟠 MajorScaffold points to template artifacts that may not exist.
This still appears to reference concrete files/patterns that were previously flagged as missing, which can make
/scaffold integrationgenerate unusable guidance.Run this to verify referenced artifacts are present before merging:
#!/bin/bash set -euo pipefail echo "Checking scaffold-referenced frontend templates..." for f in \ "components/frontend/src/components/integrations/jira-connection-card.tsx" \ "components/frontend/src/components/integrations/IntegrationsClient.tsx" \ "components/frontend/src/components/integrations/integrations-panel.tsx" \ "components/frontend/src/services/api/jira-auth.ts" \ "components/frontend/src/services/queries/use-jira.ts" do if [ -f "$f" ]; then echo "OK $f" else echo "MISS $f" fi done echo echo "Checking runner auth template and expected pattern..." if [ -f "components/runners/ambient-runner/src/auth.py" ]; then echo "OK components/runners/ambient-runner/src/auth.py" rg -n "fetch_jira_credentials|def fetch_.*_credentials" components/runners/ambient-runner/src/auth.py || true else echo "MISS components/runners/ambient-runner/src/auth.py" fiExpected result: no
MISSlines for templates that the skill declares as canonical references.Also applies to: 62-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/scaffold/SKILL.md around lines 51 - 57, The SKILL.md scaffold lists concrete template files (e.g., components/frontend/src/components/integrations/jira-connection-card.tsx, IntegrationsClient.tsx, integrations-panel.tsx, services/api/jira-auth.ts, services/queries/use-jira.ts and the runner auth pattern in components/runners/ambient-runner/src/auth.py) that may not exist; update the scaffold to either (1) verify these artifacts exist before advertising them (run the provided verification script as part of CI or the merge checklist) or (2) replace the concrete file references with generic template placeholders and instructions (e.g., `{provider}-connection-card.tsx`, `{provider}-auth.ts`, `use-{provider}.ts`) so `/scaffold integration` won’t output unusable guidance—ensure the SKILL.md entries and the `/scaffold integration` output consistently reference the same placeholder patterns or the verification step, and keep the Jira-specific examples isolated (jira-connection-card.tsx) behind an explicit “example” label.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/operator-review.md:
- Around line 36-41: The O2 terminal-phase rule is too strict by enforcing
"Error" while the controller uses "Failed": update the rule text (the O2 rule at
line referencing terminal phase) to either use "Failed" as the canonical
terminal phase or accept both "Error" and "Failed" as valid terminal phases;
reference the controller implementation in
components/operator/internal/controller/agenticsession_controller.go (where
terminal phase is set to "Failed") and adjust the doc wording so reconciler
guidance and controller behavior are aligned.
In @.claude/skills/amber-review/SKILL.md:
- Around line 29-35: The /amber-review skill's component list in SKILL.md omits
operator-specific conventions; update the list where the cross-component files
are enumerated (the block containing items like
`components/backend/DEVELOPMENT.md` and `components/frontend/DEVELOPMENT.md`) to
include `components/operator/DEVELOPMENT.md`, and make the same addition at the
other occurrence referenced around lines 127-129 so the `/amber-review` flow
loads operator context as well.
- Around line 96-98: The report heading was changed to "# Code Review", which
breaks the existing GitHub workflow that looks for comments starting with "#
Claude Code Review"; revert the heading in .claude/skills/amber-review/SKILL.md
back to "# Claude Code Review" (or alternatively update the workflow filter in
.github/workflows/amber-auto-review.yml to match the new heading), ensuring the
heading text used by the skill matches the startswith condition used in the
workflow so older bot comments continue to be matched and cleaned up.
In @.claude/skills/cypress-demo/SKILL.md:
- Around line 58-59: Replace the stale reference to
e2e/cypress/e2e/chatbox-demo.cy.ts with the actual canonical demo file
(e2e/cypress/e2e/sessions.cy.ts) and copy the demo helper implementations into
each new demo file: implement caption(), clearCaption(), initCursor(), moveTo(),
moveToText(), clickEffect(), cursorClickText() plus the timing constants LONG,
PAUSE, SHORT, TYPE_DELAY; update any documentation lines that point to
chatbox-demo.cy.ts to point to sessions.cy.ts and ensure the copied helpers are
imported or inlined in the new demo files so the steps that instruct "copy the
demo helpers" are actionable.
In @.claude/skills/pr-fixer/SKILL.md:
- Around line 22-30: The pre-flight sequence calls the PR lookup command (gh pr
view <N> --repo <owner/repo> --json state -q .state) before resolving the
repository name, so make the repo resolution step (gh repo view --json
nameWithOwner -q .nameWithOwner) run first or explicitly resolve and store
<owner/repo> before any PR-state checks; update the step ordering in the
pre-flight checklist and any descriptive text so that gh repo view is executed
prior to gh pr view (or add an explicit “Resolve repo to <owner/repo>” step that
subsequent commands reference).
In @.claude/skills/scaffold/SKILL.md:
- Around line 4-10: Update the scaffold spec to enforce the "Always includes a
feature flag" rule for endpoint scaffolds by changing the top-level statement in
SKILL.md and by adding a delegation step in the endpoint flow (the section
handling lines ~82-130) to call the existing /unleash-flag setup; specifically,
modify the summary sentence that claims every scaffold must include a feature
flag, and in the endpoint scaffold flow add an explicit requirement and
instruction to invoke /unleash-flag (or delegate to it) whenever a new endpoint
or provider is created so generated outputs consistently include Unleash gating.
---
Duplicate comments:
In @.claude/agents/backend-review.md:
- Around line 91-93: The find+xargs command in the snippet (the pipeline using
find components/backend/handlers/ -name "*.go" -not -name "*_test.go" | xargs wc
-l | sort -rn) can break on filenames containing whitespace; update it to use
null-delimited output and input (find ... -print0 and xargs -0) so wc receives
correct paths and line counts; locate the command in the file and replace the
plain find/xargs pipeline with the null-safe variant using -print0 on find and
-0 on xargs.
In @.claude/skills/scaffold/SKILL.md:
- Around line 51-57: The SKILL.md scaffold lists concrete template files (e.g.,
components/frontend/src/components/integrations/jira-connection-card.tsx,
IntegrationsClient.tsx, integrations-panel.tsx, services/api/jira-auth.ts,
services/queries/use-jira.ts and the runner auth pattern in
components/runners/ambient-runner/src/auth.py) that may not exist; update the
scaffold to either (1) verify these artifacts exist before advertising them (run
the provided verification script as part of CI or the merge checklist) or (2)
replace the concrete file references with generic template placeholders and
instructions (e.g., `{provider}-connection-card.tsx`, `{provider}-auth.ts`,
`use-{provider}.ts`) so `/scaffold integration` won’t output unusable
guidance—ensure the SKILL.md entries and the `/scaffold integration` output
consistently reference the same placeholder patterns or the verification step,
and keep the Jira-specific examples isolated (jira-connection-card.tsx) behind
an explicit “example” label.
In @.claude/skills/unleash-flag/SKILL.md:
- Around line 152-166: The example in routes.go calls
FeatureEnabledForRequest(...) unqualified which won't compile; update that call
to handlers.FeatureEnabledForRequest(...) in the code block and ensure the
handlers package is imported in the routes file. Locate the route-group example
(the flagged router.Group block) and replace FeatureEnabledForRequest with
handlers.FeatureEnabledForRequest, and confirm the handlers package that defines
FeatureEnabledForRequest (function FeatureEnabledForRequest in featureflags.go)
is present in the imports.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 67dd2e2b-ec8b-44fd-8df5-1d59d6069546
📒 Files selected for processing (44)
.claude/agents.claude/agents/backend-review.md.claude/agents/convention-eval.md.claude/agents/frontend-review.md.claude/agents/operator-review.md.claude/agents/runner-review.md.claude/agents/security-review.md.claude/commands/acp-compile.md.claude/commands/cypress-demo.md.claude/commands/speckit.analyze.md.claude/commands/speckit.checklist.md.claude/commands/speckit.clarify.md.claude/commands/speckit.constitution.md.claude/commands/speckit.implement.md.claude/commands/speckit.plan.md.claude/commands/speckit.specify.md.claude/commands/speckit.tasks.md.claude/settings.json.claude/skills/align/SKILL.md.claude/skills/align/evals/evals.json.claude/skills/amber-review/SKILL.md.claude/skills/amber-review/evals/evals.json.claude/skills/cypress-demo/SKILL.md.claude/skills/cypress-demo/evals/evals.json.claude/skills/dev-cluster/SKILL.md.claude/skills/dev-cluster/evals/evals.json.claude/skills/memory/SKILL.md.claude/skills/memory/evals/evals.json.claude/skills/pr-fixer/SKILL.md.claude/skills/pr-fixer/evals/evals.json.claude/skills/scaffold/SKILL.md.claude/skills/scaffold/evals/evals.json.claude/skills/unleash-flag/SKILL.md.claude/skills/unleash-flag/evals/evals.jsonBOOKMARKS.mdCLAUDE.mdcomponents/backend/DEVELOPMENT.mdcomponents/backend/ERROR_PATTERNS.mdcomponents/backend/K8S_CLIENT_PATTERNS.mdcomponents/frontend/DEVELOPMENT.mdcomponents/frontend/REACT_QUERY_PATTERNS.mdcomponents/operator/DEVELOPMENT.mddocs/security-standards.mdscripts/claude-hooks/stop-review.sh
💤 Files with no reviewable changes (11)
- .claude/agents
- .claude/commands/speckit.constitution.md
- .claude/commands/speckit.specify.md
- .claude/commands/cypress-demo.md
- .claude/commands/speckit.tasks.md
- .claude/commands/speckit.plan.md
- .claude/commands/speckit.analyze.md
- .claude/commands/speckit.checklist.md
- .claude/commands/acp-compile.md
- .claude/commands/speckit.clarify.md
- .claude/commands/speckit.implement.md
Comment on lines
+4
to
+10
| Generate the complete file set for a new integration, endpoint, or feature | ||
| flag following established project patterns. Use when adding a new | ||
| integration (like Jira, CodeRabbit, Google Drive), creating a new API | ||
| endpoint with full stack, or setting up a new feature flag. Triggers on: | ||
| "new integration", "add an integration", "scaffold", "create endpoint", | ||
| "add a new provider", "wire up a new service". Always includes a feature | ||
| flag. Follows the Jira integration pattern. |
Contributor
There was a problem hiding this comment.
“Always includes a feature flag” is not enforced for endpoint scaffolds.
Line 10 states every scaffold includes a feature flag, but the endpoint flow (Line 82-130) does not require or delegate /unleash-flag. That creates policy drift in generated outputs.
Suggested alignment
## Endpoint Scaffold
For adding a new API endpoint with full-stack support.
+Also include feature-flag setup by delegating to `/unleash-flag` unless explicitly waived.
@@
| Frontend proxy | `components/frontend/src/app/api/{resource}/route.ts` | Create |
+| Feature flag | `components/manifests/base/core/flags.json` | Add `{resource}.enabled` (or delegated naming) |Also applies to: 82-130
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/scaffold/SKILL.md around lines 4 - 10, Update the scaffold
spec to enforce the "Always includes a feature flag" rule for endpoint scaffolds
by changing the top-level statement in SKILL.md and by adding a delegation step
in the endpoint flow (the section handling lines ~82-130) to call the existing
/unleash-flag setup; specifically, modify the summary sentence that claims every
scaffold must include a feature flag, and in the endpoint scaffold flow add an
explicit requirement and instruction to invoke /unleash-flag (or delegate to it)
whenever a new endpoint or provider is created so generated outputs consistently
include Unleash gating.
- B8: use null-delimited find/xargs for safe file counting - O2: fix terminal phase from "Error" to "Failed" (matches operator code) - amber-review: add operator DEVELOPMENT.md to context, fix heading to "# Claude Code Review" (matches GHA workflow) - cypress-demo: update stale chatbox-demo.cy.ts ref to sessions.cy.ts - pr-fixer: reorder pre-flight to resolve repo before PR state check, fix -q to --jq - unleash-flag: qualify handlers.FeatureEnabledForRequest in routes.go example - scaffold: add verification note for reference files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.claude/agents/backend-review.md (1)
43-44:⚠️ Potential issue | 🟠 MajorB2 check is still asymmetric and can miss
DynamicClientmisuse.Line 43 only matches
DynamicClient.Resource, while the pairedK8sClient.side matches all member access. This can under-report violations forDynamicClient.usages outside.Resource.#!/bin/bash set -euo pipefail # Show all DynamicClient/K8sClient member usage in handlers (ground truth set) rg -nP --type=go -g '!**/*_test.go' '\b(DynamicClient|K8sClient)\.' components/backend/handlers/ echo "---- current B2 pattern matches ----" rg -nP --type=go -g '!**/*_test.go' 'DynamicClient\.Resource|K8sClient\.' components/backend/handlers/Suggested patch
-grep -rn "DynamicClient\.Resource\|K8sClient\." components/backend/handlers/ --include="*.go" | grep -v "_test.go" +grep -rnE "DynamicClient\.|K8sClient\." components/backend/handlers/ --include="*.go" | grep -v "_test.go"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/backend-review.md around lines 43 - 44, The B2 check regex is asymmetric (only matching DynamicClient.Resource but all K8sClient members), so update the pattern to match any DynamicClient member like K8sClient (e.g., change occurrences of "DynamicClient\.Resource|K8sClient\." to "DynamicClient\.|K8sClient\.") and update the example/commands that use rg/grep (the shell snippet and the reported pattern) so both sides are scanned equivalently; look for references to DynamicClient.Resource and the rg/grep command in the file and replace them with the broader "DynamicClient\." pattern to ensure all DynamicClient usages are detected.
🧹 Nitpick comments (1)
.claude/skills/unleash-flag/SKILL.md (1)
168-168: Tiny wording cleanup for readability.Consider changing “appears to not exist” to “appears not to exist”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/unleash-flag/SKILL.md at line 168, The sentence uses the phrase "appears to not exist" — update that phrasing to "appears not to exist" in the SKILL.md content so the line reads "This returns 404 (not 403) when the flag is off, so the endpoint appears not to exist." Locate and replace the exact string "appears to not exist" in the SKILL.md entry describing the 404 behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/backend-review.md:
- Around line 68-69: Update the grep invocation that searches for error patterns
(the line containing grep -rn
'gin.H{"error":.*fmt\.Sprintf\|gin.H{"error":.*err\.' ) to exclude test files
like the other checks do; add an --exclude="*_test.go" (or equivalent
--exclude=*_test.go) option so the search ignores _test.go files and avoids
false positives.
- Around line 60-61: Replace the grep command that uses PCRE with \n by using
ripgrep's multiline flag: change the search invocation that looks for the
pattern 'if err != nil { ... }' under components/backend/ (the current grep -rnP
invocation) to use rg -nUP so multiline matches are handled correctly; preserve
the file selection (include="*.go") and the intent of finding empty
error-handling blocks while ensuring the pattern is passed as a multiline regex
to rg.
In @.claude/agents/operator-review.md:
- Around line 38-41: The O2 terminal-phase rule and the status-on-error example
disagree about the CR phase value ("Failed" vs "Error"); update the operator
logic and/or docs to be consistent: either change the terminal-error path in the
rule (the code that sets CR status when encountering terminal errors) to set the
same phase string used in components/operator/DEVELOPMENT.md (or vice versa), or
modify the terminal-phase handling to accept and normalize both values (e.g.,
treat "Failed" and "Error" as equivalent when reading/writing status). Locate
references to errors.IsNotFound and the terminal error handling code that sets
the CR status phase and ensure the chosen canonical phase is used everywhere (or
add normalization logic), and update the documentation example to match.
---
Duplicate comments:
In @.claude/agents/backend-review.md:
- Around line 43-44: The B2 check regex is asymmetric (only matching
DynamicClient.Resource but all K8sClient members), so update the pattern to
match any DynamicClient member like K8sClient (e.g., change occurrences of
"DynamicClient\.Resource|K8sClient\." to "DynamicClient\.|K8sClient\.") and
update the example/commands that use rg/grep (the shell snippet and the reported
pattern) so both sides are scanned equivalently; look for references to
DynamicClient.Resource and the rg/grep command in the file and replace them with
the broader "DynamicClient\." pattern to ensure all DynamicClient usages are
detected.
---
Nitpick comments:
In @.claude/skills/unleash-flag/SKILL.md:
- Line 168: The sentence uses the phrase "appears to not exist" — update that
phrasing to "appears not to exist" in the SKILL.md content so the line reads
"This returns 404 (not 403) when the flag is off, so the endpoint appears not to
exist." Locate and replace the exact string "appears to not exist" in the
SKILL.md entry describing the 404 behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 38de0b62-17c7-431f-9799-8b1cc4ac27ec
📒 Files selected for processing (8)
.claude/agents/backend-review.md.claude/agents/operator-review.md.claude/skills/amber-review/SKILL.md.claude/skills/cypress-demo/SKILL.md.claude/skills/dev-cluster/SKILL.md.claude/skills/pr-fixer/SKILL.md.claude/skills/scaffold/SKILL.md.claude/skills/unleash-flag/SKILL.md
✅ Files skipped from review due to trivial changes (2)
- .claude/skills/scaffold/SKILL.md
- .claude/skills/cypress-demo/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .claude/skills/pr-fixer/SKILL.md
- .claude/skills/dev-cluster/SKILL.md
- B2: broaden DynamicClient grep to match all member access, not just .Resource - B4: switch to rg -nUP for proper multiline empty-block detection - B5: exclude test files from error exposure check - O2/operator docs: fix terminal phase from "Error" to "Failed" (matches code) - unleash-flag: fix split infinitive "appears to not exist" → "appears not to exist" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.claude/skills/unleash-flag/SKILL.md (3)
256-266: Consider specifying cleanup order to avoid partial state.The GA cleanup steps are comprehensive, but don't specify dependencies. Suggest documenting the recommended order:
- Verify feature is fully rolled out (all workspaces have flag enabled or no longer need it)
- Remove code checks (
useWorkspaceFlag/useFlagcalls) and deploy- Remove from
flags.jsonand deploy (triggers SyncFlags to stop creating flag)- Remove workspace ConfigMap overrides
- Archive and purge in Unleash
This sequence ensures no runtime errors from missing flags while cleanup progresses.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/unleash-flag/SKILL.md around lines 256 - 266, Add a clear ordered cleanup sequence to the GA section so teams avoid partial-state errors: document the recommended steps as 1) verify rollout (all workspaces either have flag enabled or no longer rely on it), 2) remove code checks (calls to useWorkspaceFlag / useFlag) and deploy, 3) remove the flag from flags.json and deploy (so SyncFlags stops recreating it), 4) remove any workspace ConfigMap overrides, and 5) archive/purge the flag in Unleash; also mention creating the Jira via /jira-log to track these steps.
174-174: Use a clearer placeholder for the token value.The pattern
'*:*.unleash-admin-token'resembles Unleash's token format (project:environment.token-id) and might confuse users or get copied verbatim. Use an explicit placeholder instead:-export CYPRESS_UNLEASH_ADMIN_TOKEN='*:*.unleash-admin-token' +export CYPRESS_UNLEASH_ADMIN_TOKEN='<your-unleash-admin-token>'Based on learnings: Never include tokens in logs, error messages, or API responses; use
len(token)for logging and provide generic messages to users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/unleash-flag/SKILL.md at line 174, Replace the ambiguous token example value for CYPRESS_UNLEASH_ADMIN_TOKEN; instead of '*:*.unleash-admin-token' use a clear placeholder that cannot be mistaken for a real token (e.g. '<PROJECT>:<ENV>.<UNLEASH_ADMIN_TOKEN>' or 'PROJECT:ENV.UNLEASH_ADMIN_TOKEN'), update the export line for CYPRESS_UNLEASH_ADMIN_TOKEN accordingly, and ensure no real token values are committed or logged (use token length for logging if needed).
170-203: Consider adding error handling guidance and token source.The E2E example could be strengthened:
- Error handling: The
cy.requestcalls don't verify success. Add.its('status').should('eq', 200)or wrap in error handling.- Token source: Document where to obtain the Unleash admin token (e.g., from Unleash UI under API Access, or reference deployment secrets).
- Alternative setup: Mention the cypress.config.ts mapping option for users who prefer centralized config over per-test exports.
These additions would reduce E2E test debugging friction.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/unleash-flag/SKILL.md around lines 170 - 203, Update the E2E example to add error handling and document the token source: assert the cy.request calls succeed (e.g., check response status) or handle failures so the test fails clearly instead of silently continuing, reference the exact calls to patch (cy.request and the Authorization header using Cypress.env('UNLEASH_ADMIN_TOKEN') / CYPRESS_UNLEASH_ADMIN_TOKEN) and add a note telling users where to obtain the Unleash admin token (Unleash UI → API Access or from deployment secrets), and include an alternative setup tip showing they can map the token centrally via cypress.config.ts instead of per-test environment exports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/operator/DEVELOPMENT.md`:
- Around line 68-78: The guide uses two different terminal phase names ("Error"
vs "Failed"); pick a single canonical phase and make the example and text
consistent. Update the status update call shown (the
updateAgenticSessionStatus(...) example) to use the chosen canonical phase
string (e.g. "Error") so it matches the earlier line that instructs terminal
errors → update CR status to "Error", and ensure any other occurrences in this
document use the same phase constant.
- Around line 52-57: The examples incorrectly use "return nil" which doesn't
match the Reconcile signature; update all example returns in the Reconcile
examples (e.g., the resource-deleted branch and the other error-handling
bullets) to return ctrl.Result{}, nil so they compile with Reconcile(ctx
context.Context, req ctrl.Request) (ctrl.Result, error). Also standardize the
status phase wording across the document: pick the phase used in the example
("Failed") and replace any occurrences of "Error" in the status-phase
bullets/examples so the status field is consistent with the Reconcile examples
and any automation/validation that expects "Failed".
---
Nitpick comments:
In @.claude/skills/unleash-flag/SKILL.md:
- Around line 256-266: Add a clear ordered cleanup sequence to the GA section so
teams avoid partial-state errors: document the recommended steps as 1) verify
rollout (all workspaces either have flag enabled or no longer rely on it), 2)
remove code checks (calls to useWorkspaceFlag / useFlag) and deploy, 3) remove
the flag from flags.json and deploy (so SyncFlags stops recreating it), 4)
remove any workspace ConfigMap overrides, and 5) archive/purge the flag in
Unleash; also mention creating the Jira via /jira-log to track these steps.
- Line 174: Replace the ambiguous token example value for
CYPRESS_UNLEASH_ADMIN_TOKEN; instead of '*:*.unleash-admin-token' use a clear
placeholder that cannot be mistaken for a real token (e.g.
'<PROJECT>:<ENV>.<UNLEASH_ADMIN_TOKEN>' or 'PROJECT:ENV.UNLEASH_ADMIN_TOKEN'),
update the export line for CYPRESS_UNLEASH_ADMIN_TOKEN accordingly, and ensure
no real token values are committed or logged (use token length for logging if
needed).
- Around line 170-203: Update the E2E example to add error handling and document
the token source: assert the cy.request calls succeed (e.g., check response
status) or handle failures so the test fails clearly instead of silently
continuing, reference the exact calls to patch (cy.request and the Authorization
header using Cypress.env('UNLEASH_ADMIN_TOKEN') / CYPRESS_UNLEASH_ADMIN_TOKEN)
and add a note telling users where to obtain the Unleash admin token (Unleash UI
→ API Access or from deployment secrets), and include an alternative setup tip
showing they can map the token centrally via cypress.config.ts instead of
per-test environment exports.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ae71b755-799d-4746-ac4a-9f575a59f775
📒 Files selected for processing (3)
.claude/agents/backend-review.md.claude/skills/unleash-flag/SKILL.mdcomponents/operator/DEVELOPMENT.md
- operator docs: fix return types to ctrl.Result{}, nil/err (matches Reconcile signature)
- operator docs: fix remaining "Error" → "Failed" in key patterns text
- unleash-flag: replace real-looking token placeholder with generic one
- unleash-flag: add ordered cleanup sequence for GA phase
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
- Add response status assertions to cy.request calls - Document token source (Unleash UI or deployment secrets) - Add cypress.config.ts alternative for centralized env mapping Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.claude/context/and.claude/patterns/into component-level files (components/*/DEVELOPMENT.md,*_PATTERNS.md), eliminating stale paths/alignfor convention scoring,/scaffoldfor integration/endpoint templates,/memoryfor auto-memory management).claude/settings.json(Shadcn UI, no manual fetch, service account misuse, no panic, skill-creator standard, feature flag nudge, stop review nudge)jira.log.mdas-isTest plan
.tsxfile with<button→ verify Shadcn reminder firespanic(→ verify panic reminder fires/align→ verify convention-eval agent dispatches and produces scored report/amber-review→ verify it loads component-level docs (not old.claude/context/paths)/scaffold integration test-provider→ verify checklist output/memory audit→ verify it scans memory directory🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Removals
Chores