Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@rccrdpccl: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
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:
WalkthroughAdded five new files: three symbolic reference files in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
bot
added
the
size/L
openshift-ci
bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.ai/skills/assisted-service-verification/SKILL.md:
- Around line 23-26: Update the fenced code block that contains the two commands
("source .venv/bin/activate && skipper make generate" and "git diff
--exit-code") to specify the language identifier "bash" on the opening triple
backticks so the block becomes a bash code block; locate the code block in
SKILL.md containing those two lines and change the opening fence from ``` to
```bash to enable proper syntax highlighting and tooling support.
- Around line 169-177: The fenced code block containing the shell steps (the
block starting with ``` and lines "1. go mod tidy" ... "7. go test <affected
packages>") should include a language identifier; update the opening fence from
``` to ```bash so the block is ```bash ... ``` to enable proper syntax
highlighting and tooling support in SKILL.md.
- Around line 14-19: The fenced code block in SKILL.md lacks a language
identifier; update the triple-backtick fence to include "bash" (i.e., change ```
to ```bash) for the block containing the four commands (lines with "source
.venv/bin/activate && skipper make build-minimal", "skipper make lint",
"SKIP_UT_DB=1 go test -v ./path/to/changed/package/...", and "make unit-test")
so tooling and syntax highlighting recognize it as Bash.
In @.ai/skills/mgmt-jira/SKILL.md:
- Around line 28-30: The document uses lowercase "markdown" in the SKILL.md
content (e.g., the parameter example contentFormat: "markdown" and other
mentions) which should be capitalized consistently as "Markdown"; update each
occurrence (including the parameter example and the mentions around the
headings/bullets) to use "Markdown" to ensure consistent capitalization across
the file (search for contentFormat: "markdown" and the other inline mentions and
replace with "Markdown").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 57b9907d-245d-4b6e-8ad4-b482783bea9f
📒 Files selected for processing (6)
.ai/skills/assisted-service-dev-mode/SKILL.md.ai/skills/assisted-service-verification/SKILL.md.ai/skills/mgmt-jira/SKILL.md.cursor/skills/assisted-service-dev-mode.cursor/skills/assisted-service-verification.cursor/skills/mgmt-jira
| ``` | ||
| 1. source .venv/bin/activate && skipper make build-minimal # compilation check (ALWAYS use skipper) | ||
| 2. source .venv/bin/activate && skipper make lint # linters | ||
| 3. SKIP_UT_DB=1 go test -v ./path/to/changed/package/... # unit tests for changed packages | ||
| 4. make unit-test # full unit tests (optional, needs Docker/Podman + DB) | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to code block.
The code block should specify bash as the language for proper syntax highlighting and better tool support.
📝 Proposed fix to add language identifier
-```
+```bash
1. source .venv/bin/activate && skipper make build-minimal # compilation check (ALWAYS use skipper)
2. source .venv/bin/activate && skipper make lint # linters
3. SKIP_UT_DB=1 go test -v ./path/to/changed/package/... # unit tests for changed packages
4. make unit-test # full unit tests (optional, needs Docker/Podman + DB)
</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @.ai/skills/assisted-service-verification/SKILL.md around lines 14 - 19, The
fenced code block in SKILL.md lacks a language identifier; update the
triple-backtick fence to include "bash" (i.e., change tobash) for the
block containing the four commands (lines with "source .venv/bin/activate &&
skipper make build-minimal", "skipper make lint", "SKIP_UT_DB=1 go test -v
./path/to/changed/package/...", and "make unit-test") so tooling and syntax
highlighting recognize it as Bash.
</details>
<!-- fingerprinting:phantom:triton:puma:5021381c-1edb-4397-865d-9a895a9dc0c1 -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ``` | ||
| 5. source .venv/bin/activate && skipper make generate # regenerate ALL | ||
| 6. git diff --exit-code # verify no generated code diff | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to code block.
The code block should specify bash as the language for proper syntax highlighting and better tool support.
📝 Proposed fix to add language identifier
-```
+```bash
5. source .venv/bin/activate && skipper make generate # regenerate ALL
6. git diff --exit-code # verify no generated code diff
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.ai/skills/assisted-service-verification/SKILL.md around lines 23 - 26,
Update the fenced code block that contains the two commands ("source
.venv/bin/activate && skipper make generate" and "git diff --exit-code") to
specify the language identifier "bash" on the opening triple backticks so the
block becomes a bash code block; locate the code block in SKILL.md containing
those two lines and change the opening fence from ``` to ```bash to enable
proper syntax highlighting and tooling support.
| ``` | ||
| 1. go mod tidy | ||
| 2. go mod vendor | ||
| 3. go build ./... | ||
| 4. skipper make generate # regenerate everything | ||
| 5. git diff --exit-code # verify no generated code diff | ||
| 6. skipper make lint | ||
| 7. go test <affected packages> | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to code block.
The code block should specify bash as the language for proper syntax highlighting and better tool support.
📝 Proposed fix to add language identifier
-```
+```bash
1. go mod tidy
2. go mod vendor
3. go build ./...
4. skipper make generate # regenerate everything
5. git diff --exit-code # verify no generated code diff
6. skipper make lint
7. go test <affected packages>
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 169-169: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.ai/skills/assisted-service-verification/SKILL.md around lines 169 - 177,
The fenced code block containing the shell steps (the block starting with ```
and lines "1. go mod tidy" ... "7. go test <affected packages>") should include
a language identifier; update the opening fence from ``` to ```bash so the block
is ```bash ... ``` to enable proper syntax highlighting and tooling support in
SKILL.md.
| 2. **Use markdown format** for descriptions | ||
| - Set `contentFormat: "markdown"` parameter | ||
| - Use standard markdown: `##` for headings, backticks for code, `-` for bullets, `|---|---|` for tables |
There was a problem hiding this comment.
Use “Markdown” capitalization consistently
Minor docs polish: “Markdown” should be capitalized in Line 28, Line 30, and Line 126 for consistency and correctness.
Also applies to: 126-126
🧰 Tools
🪛 LanguageTool
[uncategorized] ~28-~28: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...rge stories to be broken down 2. Use markdown format for descriptions - Set `con...
(MARKDOWN_NNP)
[uncategorized] ~30-~30: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ..."markdown"parameter - Use standard markdown:##` for headings, backticks for code,...
(MARKDOWN_NNP)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.ai/skills/mgmt-jira/SKILL.md around lines 28 - 30, The document uses
lowercase "markdown" in the SKILL.md content (e.g., the parameter example
contentFormat: "markdown" and other mentions) which should be capitalized
consistently as "Markdown"; update each occurrence (including the parameter
example and the mentions around the headings/bullets) to use "Markdown" to
ensure consistent capitalization across the file (search for contentFormat:
"markdown" and the other inline mentions and replace with "Markdown").
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10090 +/- ##
==========================================
+ Coverage 44.17% 44.28% +0.10%
==========================================
Files 416 416
Lines 72404 72549 +145
==========================================
+ Hits 31985 32125 +140
+ Misses 37522 37520 -2
- Partials 2897 2904 +7 🚀 New features to boost your workflow:
|
|
For jira and skipper, there are skills here: https://github.com/redhat-community-ai-tools/claude-plugins |
I think I disagree. We could argue how to use skipper could be in the generic md file, but I think as it's not an information that it's always necessary (only when running makefile targets), we can reduce context by adding it as skill. The plan is to reduce CLAUDE.md/AGENTS.md file once we have enough skills. |
|
I see, ok, sounds good. |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: omer-vishlitzky, rccrdpccl The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
rccrdpccl
force-pushed
the
add-cursor-skills
branch
from
59a7532 to
9d1c1e2
Compare
|
New changes are detected. LGTM label has been removed. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.ai/skills/assisted-service-verification/SKILL.md (1)
14-19:⚠️ Potential issue | 🟡 MinorAdd
bashlanguage identifiers to fenced command blocks.These fenced blocks still omit a language tag, which keeps tripping markdown lint and reduces editor/tooling support.
Proposed doc fix
-``` +```bash 1. source .venv/bin/activate && skipper make build-minimal # compilation check (ALWAYS use skipper) 2. source .venv/bin/activate && skipper make lint # linters 3. SKIP_UT_DB=1 go test -v ./path/to/changed/package/... # unit tests for changed packages 4. make unit-test # full unit tests (optional, needs Docker/Podman + DB)-
+bash
5. source .venv/bin/activate && skipper make generate # regenerate ALL
6. git diff --exit-code # verify no generated code diff-``` +```bash 1. go mod tidy 2. go mod vendor 3. go build ./... 4. skipper make generate # regenerate everything 5. git diff --exit-code # verify no generated code diff 6. skipper make lint 7. go test <affected packages></details> Also applies to: 23-26, 169-177 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @.ai/skills/assisted-service-verification/SKILL.md around lines 14 - 19, The
fenced command blocks in .ai/skills/assisted-service-verification/SKILL.md are
missing a language tag; update each opening triple-backtick fence for the
command examples (the build/test/generate/vendor/build sequences) to include the
"bash" language identifier (i.e. changetobash) for all affected blocks
(including the other occurrences called out in the review) so markdown lint and
editor tooling recognize them.</details> </blockquote></details> <details> <summary>.ai/skills/mgmt-jira/SKILL.md (1)</summary><blockquote> `28-30`: _⚠️ Potential issue_ | _🟡 Minor_ **Capitalize "Markdown" when referring to the language (Lines 28, 30).** Lines 28 and 30 use lowercase "markdown" when referring to the Markdown formatting language, which is a proper noun and should be capitalized. However, keep the parameter value `"markdown"` in `contentFormat: "markdown"` lowercase, as it's a JSON string value. Changes needed: - Line 28: "**Use Markdown format**" (capitalize) - Line 30: "Use standard Markdown:" (capitalize) - Keep: `contentFormat: "markdown"` (lowercase in code) <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In @.ai/skills/mgmt-jira/SKILL.md around lines 28 - 30, Update the wording to capitalize the Markdown language name in prose while leaving the JSON parameter value unchanged: change the heading and the sentence text to "Use Markdown format" and "Use standard Markdown:" respectively, but keep the parameter value `contentFormat: "markdown"` exactly as-is; locate and modify the strings around the `contentFormat` mention in SKILL.md (the heading and the explanatory sentence) and do not alter the code snippet token value. ``` </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 @.ai/skills/assisted-service-verification/SKILL.md:
- Around line 169-173: The steps in SKILL.md conflict: the checklist currently
includes "go build ./..." while the doc earlier forbids bare "go build"; update
the step to use the approved verification command instead of "go build ./..." so
it aligns with the rule (e.g., replace the "go build ./..." line with an
approved alternative such as "go list ./..." or the project’s sanctioned
build/check command), ensuring the sequence remains "go mod tidy", "go mod
vendor", then the approved verification command, and finally "skipper make
generate".
Duplicate comments:
In @.ai/skills/assisted-service-verification/SKILL.md:
- Around line 14-19: The fenced command blocks in
.ai/skills/assisted-service-verification/SKILL.md are missing a language tag;
update each opening triple-backtick fence for the command examples (the
build/test/generate/vendor/build sequences) to include the "bash" language
identifier (i.e. changetobash) for all affected blocks (including the
other occurrences called out in the review) so markdown lint and editor tooling
recognize them.In @.ai/skills/mgmt-jira/SKILL.md:
- Around line 28-30: Update the wording to capitalize the Markdown language name
in prose while leaving the JSON parameter value unchanged: change the heading
and the sentence text to "Use Markdown format" and "Use standard Markdown:"
respectively, but keep the parameter valuecontentFormat: "markdown"exactly
as-is; locate and modify the strings around thecontentFormatmention in
SKILL.md (the heading and the explanatory sentence) and do not alter the code
snippet token value.</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**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `c4d2f431-e0df-43b7-8429-cad23d8a0806` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 59a75326bae8b6c29d931cc838d064bd3ddd5fcf and 9d1c1e2391c0e0a371357f674ad3f95b879bf3fd. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `.ai/skills/assisted-service-dev-mode/SKILL.md` * `.ai/skills/assisted-service-verification/SKILL.md` * `.ai/skills/mgmt-jira/SKILL.md` * `.cursor/skills/assisted-service-dev-mode` * `.cursor/skills/assisted-service-verification` * `.cursor/skills/mgmt-jira` </details> <details> <summary>✅ Files skipped from review due to trivial changes (3)</summary> * .cursor/skills/assisted-service-verification * .cursor/skills/mgmt-jira * .cursor/skills/assisted-service-dev-mode </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| ``` | ||
| 1. go mod tidy | ||
| 2. go mod vendor | ||
| 3. go build ./... | ||
| 4. skipper make generate # regenerate everything |
There was a problem hiding this comment.
Dependency verification steps contradict the critical execution rules.
Line 172 instructs go build ./..., but Line 30 explicitly says bare go build should not be used. This creates conflicting guidance in the same skill and can break the intended workflow.
Proposed doc fix
-1. go mod tidy
-2. go mod vendor
-3. go build ./...
+1. go mod tidy
+2. go mod vendor
+3. source .venv/bin/activate && skipper make build-minimal
4. skipper make generate # regenerate everything
5. git diff --exit-code # verify no generated code diff
6. skipper make lint
7. go test <affected packages>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.ai/skills/assisted-service-verification/SKILL.md around lines 169 - 173,
The steps in SKILL.md conflict: the checklist currently includes "go build
./..." while the doc earlier forbids bare "go build"; update the step to use the
approved verification command instead of "go build ./..." so it aligns with the
rule (e.g., replace the "go build ./..." line with an approved alternative such
as "go list ./..." or the project’s sanctioned build/check command), ensuring
the sequence remains "go mod tidy", "go mod vendor", then the approved
verification command, and finally "skipper make generate".
f0daaee to
2da14d6
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (4)
.ai/skills/assisted-service-verification/SKILL.md (4)
23-26:⚠️ Potential issue | 🟡 MinorAdd language identifier to code block.
The code block lacks a language identifier for proper syntax highlighting and tool support.
📝 Proposed fix
-``` +```bash 5. source .venv/bin/activate && skipper make generate # regenerate ALL 6. git diff --exit-code # verify no generated code diff🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.ai/skills/assisted-service-verification/SKILL.md around lines 23 - 26, The fenced code block in SKILL.md containing the commands "source .venv/bin/activate && skipper make generate" and "git diff --exit-code" needs a language identifier for proper highlighting: change the opening triple backticks for that block to include "bash" (i.e., replace ``` with ```bash) so the block is marked as bash/sh.
169-177:⚠️ Potential issue | 🟡 MinorAdd language identifier to code block.
The code block lacks a language identifier for proper syntax highlighting and tool support.
📝 Proposed fix
-``` +```bash 1. go mod tidy 2. go mod vendor 3. go build ./... 4. skipper make generate # regenerate everything 5. git diff --exit-code # verify no generated code diff 6. skipper make lint 7. go test <affected packages>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.ai/skills/assisted-service-verification/SKILL.md around lines 169 - 177, Add a language identifier to the fenced code block that lists the seven commands (the block starting with "go mod tidy" and containing "skipper make generate" and "go test <affected packages>"). Change the opening fence from ``` to ```bash so the block becomes a bash-highlighted snippet; ensure only the opening fence is modified and no other content is changed.
14-19:⚠️ Potential issue | 🟡 MinorAdd language identifier to code block.
The code block lacks a language identifier for proper syntax highlighting and tool support.
📝 Proposed fix
-``` +```bash 1. source .venv/bin/activate && skipper make build-minimal # compilation check (ALWAYS use skipper) 2. source .venv/bin/activate && skipper make lint # linters 3. SKIP_UT_DB=1 go test -v ./path/to/changed/package/... # unit tests for changed packages 4. make unit-test # full unit tests (optional, needs Docker/Podman + DB)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.ai/skills/assisted-service-verification/SKILL.md around lines 14 - 19, The fenced code block in SKILL.md is missing a language identifier; update the triple-backtick fence that wraps the four commands so it reads triple-backtick followed by "bash" (i.e., use ```bash) to enable proper syntax highlighting and tooling support for the commands shown (the block containing the commands starting with "source .venv/bin/activate && skipper make build-minimal" and ending with "make unit-test").
169-173:⚠️ Potential issue | 🟠 MajorConflicting guidance: step 3 contradicts the critical execution rules.
Line 172 instructs
go build ./..., but line 30 explicitly states "NEVER run barego build,go test, orgo vetfor compilation/lint" because dependencies and headers are only available inside the skipper container. This contradiction will confuse users and lead to failed verification attempts.🔧 Proposed fix to align with critical rules
1. go mod tidy 2. go mod vendor -3. go build ./... +3. source .venv/bin/activate && skipper make build-minimal 4. skipper make generate # regenerate everything 5. git diff --exit-code # verify no generated code diff 6. skipper make lint 7. go test <affected packages>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.ai/skills/assisted-service-verification/SKILL.md around lines 169 - 173, The checklist currently contradicts the project's critical execution rule by including "go build ./..." while the SKILL.md explicitly forbids bare go build/test/vet commands; remove or replace the "go build ./..." step so it conforms to the rule—either delete step 3 or change it to instruct running the build inside the skipper container (e.g., invoke the project's containerized build command such as using "skipper" or the documented container workflow) and keep the surrounding steps ("go mod tidy", "go mod vendor", "skipper make generate") intact; ensure the wording references "go build ./..." and the rule "NEVER run bare `go build`, `go test`, or `go vet`" so future editors see the rationale.
🧹 Nitpick comments (1)
.ai/skills/mgmt-jira/SKILL.md (1)
22-24: Add language specifier to the code block.The fenced code block lacks a language specifier. While this is just a flow diagram (not executable code), adding
textorplaintextwould satisfy markdown linting rules.📝 Suggested improvement
-``` +```text User request → mgmt-jira (sets MGMT defaults) → jira:jira-task-management (executes)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @.ai/skills/mgmt-jira/SKILL.md around lines 22 - 24, The fenced code block
containing "User request → mgmt-jira (sets MGMT defaults) →
jira:jira-task-management (executes)" should include a language specifier to
satisfy markdown linting; update the opening fence fromtotext (ordiagram content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.ai/skills/assisted-service-verification/SKILL.md:
- Around line 23-26: The fenced code block in SKILL.md containing the commands
"source .venv/bin/activate && skipper make generate" and "git diff --exit-code"
needs a language identifier for proper highlighting: change the opening triple
backticks for that block to include "bash" (i.e., replace ``` with ```bash) so
the block is marked as bash/sh.
- Around line 169-177: Add a language identifier to the fenced code block that
lists the seven commands (the block starting with "go mod tidy" and containing
"skipper make generate" and "go test <affected packages>"). Change the opening
fence from ``` to ```bash so the block becomes a bash-highlighted snippet;
ensure only the opening fence is modified and no other content is changed.
- Around line 14-19: The fenced code block in SKILL.md is missing a language
identifier; update the triple-backtick fence that wraps the four commands so it
reads triple-backtick followed by "bash" (i.e., use ```bash) to enable proper
syntax highlighting and tooling support for the commands shown (the block
containing the commands starting with "source .venv/bin/activate && skipper make
build-minimal" and ending with "make unit-test").
- Around line 169-173: The checklist currently contradicts the project's
critical execution rule by including "go build ./..." while the SKILL.md
explicitly forbids bare go build/test/vet commands; remove or replace the "go
build ./..." step so it conforms to the rule—either delete step 3 or change it
to instruct running the build inside the skipper container (e.g., invoke the
project's containerized build command such as using "skipper" or the documented
container workflow) and keep the surrounding steps ("go mod tidy", "go mod
vendor", "skipper make generate") intact; ensure the wording references "go
build ./..." and the rule "NEVER run bare `go build`, `go test`, or `go vet`" so
future editors see the rationale.
---
Nitpick comments:
In @.ai/skills/mgmt-jira/SKILL.md:
- Around line 22-24: The fenced code block containing "User request → mgmt-jira
(sets MGMT defaults) → jira:jira-task-management (executes)" should include a
language specifier to satisfy markdown linting; update the opening fence from
``` to ```text (or ```plaintext) so the block becomes a plain text code block
while preserving the diagram content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f774eb9-8997-469a-b580-cae01dfad118
📒 Files selected for processing (6)
.ai/skills/assisted-service-dev-mode/SKILL.md.ai/skills/assisted-service-verification/SKILL.md.ai/skills/mgmt-jira/SKILL.md.cursor/skills/assisted-service-dev-mode.cursor/skills/assisted-service-verification.cursor/skills/mgmt-jira
✅ Files skipped from review due to trivial changes (3)
- .cursor/skills/assisted-service-verification
- .cursor/skills/assisted-service-dev-mode
- .cursor/skills/mgmt-jira
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.ai/skills/assisted-service-verification/SKILL.md (1)
169-173:⚠️ Potential issue | 🟠 MajorDependency verification steps contradict the critical execution rules.
Line 172 instructs
go build ./..., but Line 30 explicitly says barego buildshould not be used because "Go tooling, C headers (nmstate), and dependencies are only available inside the skipper container." This creates conflicting guidance that could lead users to run commands that will fail.📋 Proposed fix to align with critical rules
1. go mod tidy 2. go mod vendor -3. go build ./... +3. source .venv/bin/activate && skipper make build-minimal 4. skipper make generate # regenerate everything 5. git diff --exit-code # verify no generated code diff 6. skipper make lint 7. go test <affected packages>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.ai/skills/assisted-service-verification/SKILL.md around lines 169 - 173, Replace the bare "go build ./..." step with a command that runs the build inside the skipper container (e.g., change "go build ./..." to "skipper exec -- go build ./...") or remove the build step entirely and add a note that builds must be executed inside the skipper container; specifically update the checklist entry that currently reads "go build ./..." to avoid using bare "go build" (as referenced in the critical rule about Go tooling and C headers only being available inside the skipper container).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.ai/skills/assisted-service-verification/SKILL.md:
- Around line 169-173: Replace the bare "go build ./..." step with a command
that runs the build inside the skipper container (e.g., change "go build ./..."
to "skipper exec -- go build ./...") or remove the build step entirely and add a
note that builds must be executed inside the skipper container; specifically
update the checklist entry that currently reads "go build ./..." to avoid using
bare "go build" (as referenced in the critical rule about Go tooling and C
headers only being available inside the skipper container).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71780c66-4fe5-4385-9d4b-b19e1561cb2c
📒 Files selected for processing (6)
.ai/skills/assisted-service-dev-mode/SKILL.md.ai/skills/assisted-service-verification/SKILL.md.ai/skills/mgmt-jira/SKILL.md.cursor/skills/assisted-service-dev-mode.cursor/skills/assisted-service-verification.cursor/skills/mgmt-jira
✅ Files skipped from review due to trivial changes (4)
- .cursor/skills/assisted-service-verification
- .cursor/skills/mgmt-jira
- .cursor/skills/assisted-service-dev-mode
- .ai/skills/mgmt-jira/SKILL.md
|
Better use |
.ai/skills/mgmt-jira/SKILL.md
Outdated
| - `performance` - Performance-related issues | ||
| - `kube-api` - Kubernetes API mode specific | ||
|
|
||
| ### Recent Versions (Affects Version for Bugs) |
There was a problem hiding this comment.
Did you try to open a bug with this skill? (I don't think any of the jira cli tools can set Affects Version)
There was a problem hiding this comment.
well to be fair I am using JIRA MCP. The idea was to have some JIRA skill to help with "how do we fill the issue" and a personal skill to deal "how do we deal with JIRA API" , so ideally it would work with both.
What are your thoughts on this? What would u recommend for the in-project skill?
Regarding why I use MCP instead of jira-cli, is simply because it was taking a few tries to get the issue created vs immediate with MCP, even though MCP consumes more tokens - but probably it's just my setup that needs tuning.
rccrdpccl
force-pushed
the
add-cursor-skills
branch
from
2da14d6 to
49f48e9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.agents/skills/assisted-service-verification/SKILL.md (2)
169-173:⚠️ Potential issue | 🟠 MajorDependency checklist contradicts the critical execution rules.
Line 172 uses
go build ./..., but Line 30 explicitly says never run barego buildfor compilation/lint. This creates conflicting instructions in the same skill.Proposed doc fix
1. go mod tidy 2. go mod vendor -3. go build ./... +3. source .venv/bin/activate && skipper make build-minimal 4. skipper make generate # regenerate everything🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/assisted-service-verification/SKILL.md around lines 169 - 173, The dependency checklist contradicts the execution rules by including the forbidden command "go build ./..." in SKILL.md; replace that line with the approved build invocation (for example use the project's Make target such as "skipper make build" or "make build") so the document no longer instructs running a bare "go build" and is consistent with the rule referenced elsewhere in SKILL.md.
14-19:⚠️ Potential issue | 🟡 MinorAdd
bashlanguage tags to fenced command blocks.The fenced command blocks at Line 14, Line 23, and Line 169 are still missing a language identifier, so markdown linting and tooling support remain degraded.
Proposed doc fix
-``` +```bash 1. source .venv/bin/activate && skipper make build-minimal # compilation check (ALWAYS use skipper) 2. source .venv/bin/activate && skipper make lint # linters 3. SKIP_UT_DB=1 go test -v ./path/to/changed/package/... # unit tests for changed packages 4. make unit-test # full unit tests (optional, needs Docker/Podman + DB)-
+bash
5. source .venv/bin/activate && skipper make generate # regenerate ALL
6. git diff --exit-code # verify no generated code diff-``` +```bash 1. go mod tidy 2. go mod vendor 3. go build ./... 4. skipper make generate # regenerate everything 5. git diff --exit-code # verify no generated code diff 6. skipper make lint 7. go test <affected packages></details> Also applies to: 23-26, 169-177 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @.agents/skills/assisted-service-verification/SKILL.md around lines 14 - 19,
Update the three fenced code blocks in SKILL.md that currently start with the
command lines "source .venv/bin/activate && skipper make build-minimal", "source
.venv/bin/activate && skipper make generate", and "go mod tidy" to add a
language identifier by replacing the opening triple backticks () with "bash" so the blocks become bash-highlighted; ensure each matching closing
triple backtick remains unchanged.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>.agents/skills/mgmt-jira/SKILL.md (1)</summary><blockquote> `12-16`: **Centralize and periodically verify Jira IDs to avoid drift** This skill hardcodes project metadata and multiple numeric IDs (issue types/components). If Jira admins change them, create/update flows will silently fail at runtime. Prefer a single metadata source (or a small “how to refresh IDs” step in this file) and validate IDs periodically. Also applies to: 21-27, 54-63 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In @.agents/skills/mgmt-jira/SKILL.md around lines 12 - 16, The hardcoded Jira metadata (fields like "Project", "Cloud ID", "Default Component", and URLs) in SKILL.md causes drift; replace these scattered numeric IDs with a single canonical source (e.g., a config key or env variable referenced by your skill) and add a validation step (implement/point to a function like validateJiraMetadata() or a small refresh script) that checks each ID against the Jira API before create/update flows run; update SKILL.md to document where the single metadata source lives and include a short "how to refresh IDs" procedure and a periodic verification recommendation for the sections that contain IDs (including the other blocks referenced at lines 21-27 and 54-63). ``` </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 @.agents/skills/mgmt-jira/SKILL.md:
- Around line 115-116: The string used in the user prompt "add Assisted
Installer component to MGMT-XXXXX" is inconsistent with the canonical name
"Assisted-Installer" used elsewhere; update that prompt so it reads "add
Assisted-Installer component to MGMT-XXXXX" (the line that shows the
editJiraIssue example with {"components": [{"id": "49033"}]}), or alternatively
normalize all occurrences in SKILL.md to "Assisted-Installer" to avoid
copy/paste mistakes.
Duplicate comments:
In @.agents/skills/assisted-service-verification/SKILL.md:
- Around line 169-173: The dependency checklist contradicts the execution rules
by including the forbidden command "go build ./..." in SKILL.md; replace that
line with the approved build invocation (for example use the project's Make
target such as "skipper make build" or "make build") so the document no longer
instructs running a bare "go build" and is consistent with the rule referenced
elsewhere in SKILL.md.- Around line 14-19: Update the three fenced code blocks in SKILL.md that
currently start with the command lines "source .venv/bin/activate && skipper
make build-minimal", "source .venv/bin/activate && skipper make generate", and
"go mod tidy" to add a language identifier by replacing the opening triple
backticks () with "bash" so the blocks become bash-highlighted; ensure
each matching closing triple backtick remains unchanged.
Nitpick comments:
In @.agents/skills/mgmt-jira/SKILL.md:
- Around line 12-16: The hardcoded Jira metadata (fields like "Project", "Cloud
ID", "Default Component", and URLs) in SKILL.md causes drift; replace these
scattered numeric IDs with a single canonical source (e.g., a config key or env
variable referenced by your skill) and add a validation step (implement/point to
a function like validateJiraMetadata() or a small refresh script) that checks
each ID against the Jira API before create/update flows run; update SKILL.md to
document where the single metadata source lives and include a short "how to
refresh IDs" procedure and a periodic verification recommendation for the
sections that contain IDs (including the other blocks referenced at lines 21-27
and 54-63).</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**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `e7ab1631-a62a-4396-a7f0-31fddf9c1108` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 2da14d68f7d558c801e25322bea7630e4a240234 and 49f48e915e18a23dfca69e7bbec94bcfb75f63d5. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `.agents/skills/assisted-service-dev-mode/SKILL.md` * `.agents/skills/assisted-service-verification/SKILL.md` * `.agents/skills/mgmt-jira/SKILL.md` * `.cursor/skills/assisted-service-dev-mode` * `.cursor/skills/assisted-service-verification` * `.cursor/skills/mgmt-jira` </details> <details> <summary>✅ Files skipped from review due to trivial changes (3)</summary> * .cursor/skills/mgmt-jira * .cursor/skills/assisted-service-dev-mode * .cursor/skills/assisted-service-verification </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| When user says "add Assisted Installer component to MGMT-XXXXX": | ||
| 1. Use editJiraIssue with `{"components": [{"id": "49033"}]}` |
There was a problem hiding this comment.
Use the canonical component name consistently
Line 115 says “Assisted Installer” while the rest of the file uses “Assisted-Installer”. Keep one spelling to prevent copy/paste mistakes in user prompts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/mgmt-jira/SKILL.md around lines 115 - 116, The string used in
the user prompt "add Assisted Installer component to MGMT-XXXXX" is inconsistent
with the canonical name "Assisted-Installer" used elsewhere; update that prompt
so it reads "add Assisted-Installer component to MGMT-XXXXX" (the line that
shows the editJiraIssue example with {"components": [{"id": "49033"}]}), or
alternatively normalize all occurrences in SKILL.md to "Assisted-Installer" to
avoid copy/paste mistakes.
|
/override ci/prow/e2e-agent-compact-ipv4 |
|
@rccrdpccl: Overrode contexts on behalf of rccrdpccl: ci/prow/e2e-agent-compact-ipv4 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@rccrdpccl: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/retitle NO-ISSUE: Add AI skills |
openshift-ci
bot
changed the title
| ### Available Components | ||
|
|
||
| Common assisted-service components: | ||
| - **Assisted-Installer** (49033) - Default, general AI work |
There was a problem hiding this comment.
Sometimes we also use Infrastructure Operator IIRC if it's just operator/controller specific work
| ```typescript | ||
| mcp__plugin_atlassian_atlassian__searchJiraIssuesUsingJql({ | ||
| cloudId: "2b9e35e3-6bd3-4cec-b838-f4249ee02432", | ||
| jql: "project = MGMT AND component = 'Assisted-Installer' AND status = 'To Do'" |
There was a problem hiding this comment.
Adding the component and status here might be too specific, but just MGMT also might be to broad ... unsure what the best option is, but at the very least I think limiting search to only TODO status by default is probably not great especially since some issue types have different "new" status values.
5 participants
Summary
.ai/skills/as single source of truth for AI tooling skillsassisted-service-verificationskill (mandatory pre-commit checklist)mgmt-jiraskill (JIRA issue management for MGMT project)assisted-service-dev-modeto.ai/skills/.cursor/skills/pointing to.ai/skills/The
.ai/directory provides a general-purpose location for AI tooling configuration, supporting multiple clients (Claude, Cursor, Copilot, etc.)Skills Added
assisted-service-verification
Mandatory verification checklist to run before any git commit/push:
mgmt-jira
JIRA issue management for the MGMT project:
🤖 Generated with Claude Code