Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
670cb3e
fix(rh-virt): RH-VIRT-001 Fixing yaml injection in vm-snapshot-create
r2dedios May 5, 2026
9839bcb
fix(rh-virt): RH-VIRT-005 remove CLI fallback from vm-inventory to pr…
r2dedios May 6, 2026
5be4159
fix(rh-virt): RH-VIRT-015 require per-VM typed confirmation for batch…
r2dedios May 6, 2026
6969f37
fix(rh-virt): RH-VIRT-003 restrict resources_create_or_update to Virt…
r2dedios May 6, 2026
03ead88
fix(rh-virt): RH-VIRT-006 remove external doc reads to prevent indire…
r2dedios May 6, 2026
5eece30
fix(rh-virt): RH-VIRT-007 clarify human-in-the-loop as not required f…
r2dedios May 6, 2026
67b3a4b
fix(rh-virt): RH-VIRT-020 remove shell commands, use MCP tools exclus…
r2dedios May 6, 2026
4e8d850
fix(rh-virt): RH-VIRT-026 enforce per-VM confirmation, fix already-ru…
r2dedios May 6, 2026
5a10dab
fix(rh-virt): RH-VIRT-032 require typed snapshot name confirmation fo…
r2dedios May 6, 2026
93ff641
fix(rh-virt): RH-VIRT-034 add allowed-tools to vm-snapshot-delete
r2dedios May 7, 2026
d524979
fix(rh-virt): RH-VIRT-008 skip VMI detail queries when listing more t…
r2dedios May 7, 2026
f740b1d
fix(rh-virt): RH-VIRT-012 replace ambiguous relative paths and remove…
r2dedios May 7, 2026
aec96c1
fix(rh-virt): RH-VIRT-013 add allowed-tools to vm-rebalance frontmatter
r2dedios May 7, 2026
46305eb
fix(rh-virt): RH-VIRT-017 add allowed-tools to vm-delete frontmatter
r2dedios May 7, 2026
4546212
fix(rh-virt): RH-VIRT-021 add custom image provenance warning
r2dedios May 7, 2026
e9ab730
fix(rh-virt): RH-VIRT-022 bound provisioning polling to max 3 retries
r2dedios May 7, 2026
34d218b
fix(rh-virt): RH-VIRT-030 add allowed-tools to vm-snapshot-list front…
r2dedios May 7, 2026
dc1ae2b
fix(rh-virt): RH-VIRT-023 add allowed-tools to vm-create frontmatter
r2dedios May 7, 2026
5f4f94f
fix(rh-virt): RH-VIRT-027 add allowed-tools to vm-lifecycle-manager f…
r2dedios May 7, 2026
f897e4a
fix(rh-virt): RH-VIRT-035 remove unsupported batch deletion trigger p…
r2dedios May 7, 2026
0912ca7
fix(rh-virt): RH-VIRT-038 limit batch cloning to 5 with per-VM confir…
r2dedios May 7, 2026
8fbd9a5
fix(rh-virt): RH-VIRT-040 add allowed-tools to vm-snapshot-restore fr…
r2dedios May 7, 2026
005cc00
fix(rh-virt): RH-VIRT-041 narrow activation phrases to explicit snaps…
r2dedios May 7, 2026
33f6d33
chore: make allowed-tools a required field in skill linter
r2dedios May 7, 2026
20cf8bd
fix(rh-virt): RH-VIRT-043 add missing vm_lifecycle to vm-rebalance al…
r2dedios May 7, 2026
4355592
fix(rh-virt): RH-VIRT-044 add quota check before cloning to prevent r…
r2dedios May 7, 2026
1aac7d5
fix(rh-virt): RH-VIRT-045 add missing vm_lifecycle to vm-snapshot-res…
r2dedios May 7, 2026
815445c
fix(rh-virt): RH-VIRT-046 add missing vm_lifecycle to vm-lifecycle-ma…
r2dedios May 7, 2026
9c06227
fix(rh-virt): RH-VIRT-047 add missing vm_create and vm_lifecycle to v…
r2dedios May 7, 2026
8d61619
fix(rh-virt): RH-VIRT-048 remove Read tool injection and fix relative…
r2dedios May 7, 2026
87dea0f
fix(rh-virt): RH-VIRT-050 restore Document Consultation pattern in vm…
r2dedios May 7, 2026
4e24523
fix(rh-virt): RH-VIRT-051 align VMI query threshold to 20 in vm-inven…
r2dedios May 7, 2026
e9a498c
feat(workflows): add iteration history and thumbs up/down reactions
r2dedios May 7, 2026
e305fe2
fix(rh-virt): RH-VIRT-052 add missing vm_lifecycle to vm-delete allow…
r2dedios May 7, 2026
1bb07c9
chore(rh-virt): Returned back to ecosystem-eng Openshift MCP server v…
r2dedios May 7, 2026
8aa94be
revert(rh-virt): temporarily remove allowed-tools from all skills
r2dedios May 7, 2026
2ec70bb
revert(rh-virt): restore relative paths in skill documentation links
r2dedios May 8, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions .claude/skills/skill-linter/scripts/validate-skill.sh
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,17 @@ if [ -n "$COMPAT" ]; then
pass "Compatibility valid ($COMPAT_LEN chars)"
fi

# Check allowed-tools (if present)
# Check allowed-tools (optional)
TOOLS=$(echo "$FRONTMATTER" | grep '^allowed-tools:' | sed 's/^allowed-tools:[[:space:]]*//' || true)
if [ -n "$TOOLS" ]; then
if [ -z "$TOOLS" ]; then
# Check for YAML multi-line array (allowed-tools:\n - item)
if echo "$FRONTMATTER" | grep -q '^allowed-tools:$'; then
NEXT_LINE=$(echo "$FRONTMATTER" | grep -A1 '^allowed-tools:$' | sed -n '2p')
if echo "$NEXT_LINE" | grep -qE '^[[:space:]]*-[[:space:]]'; then
fail "allowed-tools must be space-delimited, not YAML array"
fi
fi
else
# FAIL if commas found (must be space-delimited)
if echo "$TOOLS" | grep -qE ','; then
fail "allowed-tools must be space-delimited, not comma-separated: $TOOLS"
Expand All @@ -171,13 +179,6 @@ if [ -n "$TOOLS" ]; then
pass "Allowed-tools field valid"
fi
fi
# FAIL if YAML multi-line array detected (allowed-tools:\n - item)
if echo "$FRONTMATTER" | grep -q '^allowed-tools:$'; then
NEXT_LINE=$(echo "$FRONTMATTER" | grep -A1 '^allowed-tools:$' | sed -n '2p')
if echo "$NEXT_LINE" | grep -qE '^[[:space:]]*-[[:space:]]'; then
fail "allowed-tools must be space-delimited, not YAML array"
fi
fi

# 8. Check line count (max 500)
LINE_COUNT=$(wc -l < "$SKILL_FILE" | tr -d ' ')
Expand Down
56 changes: 46 additions & 10 deletions .github/workflows/skill-code-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,28 @@ jobs:
set -euo pipefail
MARKER="<!-- gemini-code-review -->"
WORKFLOW_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
TIMESTAMP=$(date -u '+%Y-%m-%d %H:%M UTC')

EXISTING_COMMENT_ID=$(gh api --paginate \
"/repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \
--jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \
| head -1)

EXISTING_BODY=""
RUN_NUMBER=1
if [ -n "$EXISTING_COMMENT_ID" ]; then
EXISTING_BODY=$(gh api \
"/repos/${{ github.repository }}/issues/comments/$EXISTING_COMMENT_ID" \
--jq '.body')
RUN_NUMBER=$(( $(echo "$EXISTING_BODY" | grep -c '### 🔄 Run #' || true) + 1 ))
fi

{
echo "$MARKER"
if [ "$RUN_NUMBER" -eq 1 ]; then
echo "$MARKER"
fi
echo "### 🔄 Run #${RUN_NUMBER} — ${TIMESTAMP}"
echo ""
echo "## Gemini Code Review"
echo ""

Expand All @@ -317,21 +336,38 @@ jobs:
fi

echo ""
echo "---"
echo "> [Workflow run]($WORKFLOW_URL)"
} > /tmp/comment.md

EXISTING_COMMENT_ID=$(gh api --paginate \
"/repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \
--jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \
| head -1)
} > /tmp/new_report.md

if [ -n "$EXISTING_COMMENT_ID" ]; then
gh api --method PATCH \
{
echo "$EXISTING_BODY"
echo ""
echo "---"
echo ""
cat /tmp/new_report.md
} > /tmp/comment.md
gh api \
--method PATCH \
"/repos/${{ github.repository }}/issues/comments/$EXISTING_COMMENT_ID" \
-f body="$(cat /tmp/comment.md)"
else
gh pr comment "$PR_NUMBER" \
--repo "${{ github.repository }}" \
--body-file /tmp/comment.md
--body-file /tmp/new_report.md
fi

- name: React to PR
if: always()
uses: actions/github-script@v7
with:
script: |
const result = '${{ job.status }}';
const prNumber = Number('${{ steps.inputs.outputs.pr_number }}');
await github.rest.reactions.createForIssue({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: prNumber,
content: result === 'success' ? '+1' : '-1'
});

73 changes: 63 additions & 10 deletions .github/workflows/skill-security-scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ jobs:
--recursive \
--use-behavioral \
--use-llm \
--llm-consensus-runs 3 \
--check-overlap \
--enable-meta \
--fail-on-severity medium \
Expand All @@ -237,6 +238,7 @@ jobs:
fi

- name: Upload security reports
id: upload
if: steps.detect.outputs.pack_count != '0' && steps.creds.outputs.available == 'true' && always()
uses: actions/upload-artifact@v4
with:
Expand All @@ -257,15 +259,35 @@ jobs:
set -euo pipefail
MARKER="<!-- skill-security-scan -->"
SCAN_RESULT="${{ steps.scan.outputs.scan_result }}"
ARTIFACT_URL="${{ steps.upload.outputs.artifact-url }}"
TIMESTAMP=$(date -u '+%Y-%m-%d %H:%M UTC')

if [ "$SCAN_RESULT" = "passed" ]; then
ICON="✅"
else
ICON="❌"
fi

EXISTING_COMMENT_ID=$(gh api --paginate \
"/repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \
--jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \
| head -1)

EXISTING_BODY=""
RUN_NUMBER=1
if [ -n "$EXISTING_COMMENT_ID" ]; then
EXISTING_BODY=$(gh api \
"/repos/${{ github.repository }}/issues/comments/$EXISTING_COMMENT_ID" \
--jq '.body')
RUN_NUMBER=$(( $(echo "$EXISTING_BODY" | grep -c '### 🔄 Run #' || true) + 1 ))
fi

{
echo "$MARKER"
if [ "$RUN_NUMBER" -eq 1 ]; then
echo "$MARKER"
fi
echo "### 🔄 Run #${RUN_NUMBER} — ${TIMESTAMP}"
echo ""
echo "## $ICON Skill Security Scan"
echo ""

Expand All @@ -286,20 +308,51 @@ jobs:
fi

echo ""
echo "> [Workflow run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})"
} > /tmp/comment.md

EXISTING_COMMENT_ID=$(gh api --paginate \
"/repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \
--jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \
| head -1)
if [ -n "$ARTIFACT_URL" ]; then
echo "> 📦 [Download security reports]($ARTIFACT_URL) | [Workflow run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})"
else
echo "> [Workflow run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})"
fi
} > /tmp/new_report.md

if [ -n "$EXISTING_COMMENT_ID" ]; then
gh api --method PATCH \
{
echo "$EXISTING_BODY"
echo ""
echo "---"
echo ""
cat /tmp/new_report.md
} > /tmp/comment.md
gh api \
--method PATCH \
"/repos/${{ github.repository }}/issues/comments/$EXISTING_COMMENT_ID" \
-f body="$(cat /tmp/comment.md)"
else
gh pr comment "$PR_NUMBER" \
--repo "${{ github.repository }}" \
--body-file /tmp/comment.md
--body-file /tmp/new_report.md
fi

- name: Check scan results
if: steps.detect.outputs.pack_count != '0' && steps.creds.outputs.available == 'true'
run: |
if [ "${{ steps.scan.outputs.scan_result }}" = "failed" ]; then
echo "❌ Security scan found MEDIUM or higher severity issues — blocking merge"
exit 1
else
echo "✅ Security scan passed — no MEDIUM or higher severity issues found"
fi

- name: React to PR
if: always()
uses: actions/github-script@v7
with:
script: |
const result = '${{ job.status }}';
const prNumber = Number('${{ steps.inputs.outputs.pr_number }}');
await github.rest.reactions.createForIssue({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: prNumber,
content: result === 'success' ? '+1' : '-1'
});
4 changes: 2 additions & 2 deletions rh-virt/mcps.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
"--network=host",
"--userns=keep-id:uid=65532,gid=65532",
"-v", "${KUBECONFIG}:/kubeconfig:ro,Z",
"--entrypoint", "/openshift-mcp-server",
"quay.io/redhat-user-workloads/crt-nshift-lightspeed-tenant/openshift-mcp-server@sha256:2f52c860f91ab3c8a5129b727bdef0d620e733013f073b10355866c45eafd053",
"--entrypoint", "/app/kubernetes-mcp-server",
"quay.io/ecosystem-appeng/openshift-mcp-server@sha256:3531cb78f51f8c7ebcdb21adc21358ab8924116994848a3ce1ff542b3fc23742",
"--kubeconfig", "/kubeconfig",
"--toolsets", "core,kubevirt"
],
Expand Down
31 changes: 20 additions & 11 deletions rh-virt/skills/vm-clone/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,11 @@ Parse: Extract storage names, calculate size, determine DataSources vs container

**Option 2 warning**: `⚠️ Shared Storage Dangerous - Both VMs share disk, data corruption risk. Only safe if source stopped. Use Option 1 instead. Proceed anyway? (yes/cancel)` Wait for explicit "yes".

### Step 3: Present Clone Configuration for Confirmation
### Step 3: Check Namespace Quota and Present Clone Configuration

**3.0: Check ResourceQuota** (before presenting confirmation)

Use `resources_list` (apiVersion="v1", kind="ResourceQuota", namespace=`<target-namespace>`) to check if quotas exist. If a quota is found, compare current usage against limits for CPU, memory, and storage. If the clone would push usage above 80% of any limit, include a warning in the confirmation summary.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Here we use resources_list but the allowed-tools list has mcp__openshift-virtualization__resources_list.
I'm not sure what is the right option, but we must be consistent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the allowed-tools field was removed.


**Present configuration summary:**

Expand Down Expand Up @@ -275,8 +279,8 @@ Would you like help troubleshooting this error?

### Batch Cloning
**User request:** "Create 3 copies of template-vm named web-01, web-02, web-03"
**Workflow**: Validate source once, generate/check target names, present combined scope, ask storage strategy, confirm, execute sequentially
**Batch confirmation**: Show source, targets list, strategy, total impact (VMs, storage, vCPU, memory), estimated time
**Limit**: Maximum 5 clones per batch request. If user requests more, refuse and explain the limit exists to prevent resource exhaustion.
**Workflow**: Validate source once, generate/check target names, ask storage strategy once, then process each clone individually through Steps 3-5 with separate confirmation per VM. Stop on first failure.

### Cross-Namespace Cloning
**User request:** "Clone production-vm from production to staging namespace"
Expand Down Expand Up @@ -417,17 +421,22 @@ Agent: "✓ VM Cloned Successfully"
```
User: "Create 3 copies of template-vm named web-01, web-02, web-03 in production"

Agent: [Validates source, checks all names, presents batch review]
"Source: template-vm, Targets: 3 VMs (web-01, web-02, web-03)"
"Total Impact: 90Gi, 6 vCPU, 12Gi, ~20-30 min"
"Proceed? (yes/no)"
Agent: [Validates source, checks all names, asks storage strategy once]
"Storage strategy for all clones: Clone Storage (1)"

## Clone 1 of 3: web-01
"Source: template-vm → Target: web-01 (production), 30Gi clone"
"Proceed with VM cloning? (yes/no)"
User: "yes"
Agent: "✓ web-01 cloned"

## Clone 2 of 3: web-02
"Source: template-vm → Target: web-02 (production), 30Gi clone"
"Proceed with VM cloning? (yes/no)"
User: "yes"
Agent: "✓ web-02 cloned"

Agent: "📦 Cloning VM 1 of 3: web-01... ✓"
"📦 Cloning VM 2 of 3: web-02... ✓"
"📦 Cloning VM 3 of 3: web-03... ✓"
"✓ Batch Cloning Completed - 3 VMs, 90Gi storage, all Stopped"
[... repeats for each VM with individual confirmation ...]
```

### Example 4: Shared Storage Warning
Expand Down
21 changes: 9 additions & 12 deletions rh-virt/skills/vm-create/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ Create virtual machines in OpenShift Virtualization using the `vm_create` tool f
**Optional (use defaults):** OS (fedora), Size (medium), Storage (30Gi), Performance (u1), Autostart (false)

**Gather cluster info:**
- Detect current namespace: `kubectl config view --minify -o jsonpath='{..namespace}' || echo "default"`
- List namespaces: `namespaces_list` (from openshift-virtualization)
- List namespaces: `namespaces_list` (from openshift-virtualization) — ask user to select if not provided
- List StorageClasses: `resources_list` with apiVersion="storage.k8s.io/v1", kind="StorageClass"
- Identify default SC: annotation `storageclass.kubernetes.io/is-default-class`="true"
- Analyze SC: `.volumeBindingMode` (Immediate/WaitForFirstConsumer), provisioner (rbd/cephfs=RWX hint)
Expand Down Expand Up @@ -166,15 +165,13 @@ Confirm: yes/no/modify
**Status interpretation:**
- Stopped/Halted → Success (VM created, not started)
- Running → Success (if autostart=true)
- Provisioning → Wait 5s, check again
- Provisioning → Wait 5s, check again (max 3 retries, then report status as "still provisioning" and suggest the user check back later)
- ErrorUnschedulable → Execute diagnostic workflow (Step 5a)
- ErrorDataVolumeNotReady → Storage issue (see Common Issues)

#### 5a. Diagnostic Workflow (ErrorUnschedulable)

**CRITICAL: Document Consultation FIRST:**
1. Read [scheduling-errors.md](../../docs/troubleshooting/scheduling-errors.md) using Read tool
2. Output: "I detected ErrorUnschedulable. I consulted [scheduling-errors.md] to understand diagnosis strategies."
**Reference**: See [scheduling-errors.md](../../docs/troubleshooting/scheduling-errors.md) for diagnosis strategies.

**Gather diagnostics:**
- List events: `events_list` (namespace=`<namespace>`) → Filter for VM/VMI
Expand All @@ -194,7 +191,7 @@ Confirm: yes/no/modify

### Recommended Solution
<workaround-description>
**Command**: `oc patch vm <name> -n <namespace> ...`
**Action**: Update VM via `resources_create_or_update`
**Impact**: <what-changes>
**Options**: 1) Apply workaround, 2) Manual, 3) Cancel, 4) Ignore
⚠️ MCP limitation: vm_create doesn't support tolerations
Expand All @@ -203,7 +200,7 @@ Confirm: yes/no/modify
**Wait for user decision.**

**If user confirms:**
1. Apply patch: `resources_create_or_update` (fetch, add tolerations, update) OR `oc patch`
1. Apply patch: `resources_create_or_update` (fetch, add tolerations, update)
2. Verify: `resources_get` → Check `.spec.template.spec.tolerations`
3. **Restart VM**: `vm_lifecycle` (action="restart") to apply new spec
4. Wait 15-20s, check status → Stopped → Provisioning → Running
Expand Down Expand Up @@ -241,7 +238,7 @@ Start: "Start VM <name>" | View: "Show VM <name>"
**Error**: <error-message>

**Common Causes**:
- Namespace not exists → `oc create namespace <name>`
- Namespace not exists → Create via `resources_create_or_update`
- RBAC denied → Check ServiceAccount permissions
- Resource constraints → Try smaller size
- Invalid parameters → Verify OS, size, storage format
Expand All @@ -254,7 +251,7 @@ Troubleshooting: See Common Issues

### Issue 1: Namespace Not Found
**Error**: "Namespace 'xyz' not found"
**Solution**: List with `namespaces_list`, create with `resources_create_or_update` or `oc create namespace <name>`
**Solution**: List with `namespaces_list`, create with `resources_create_or_update`

### Issue 2: Insufficient Permissions
**Error**: "Forbidden: User cannot create VirtualMachines"
Expand Down Expand Up @@ -331,7 +328,7 @@ Troubleshooting: See Common Issues
- **RBAC**: Requires create VirtualMachines (kubevirt.io/v1) in namespace
- **Namespace Isolation**: VMs in specified namespace only
- **Storage Quotas**: Respects ResourceQuotas
- **Image Security**: Uses official images from trusted registries
- **Image Security**: Default OS options (fedora, ubuntu, rhel, centos-stream, debian, opensuse) use upstream container disk images. Custom images via the `workload` parameter are not validated — warn the user when a custom image URL is provided and confirm the source is trusted before proceeding
- **KUBECONFIG**: Never exposed (presence only)
- **Audit**: All ops logged via K8s audit

Expand Down Expand Up @@ -392,6 +389,6 @@ Agent: [Creates]

## Advanced Features

**Custom Images**: `vm_create({"workload": "quay.io/containerdisks/fedora:latest", ...})`
**Custom Images**: `vm_create({"workload": "quay.io/containerdisks/fedora:latest", ...})` — ⚠️ When a custom image URL is provided, warn the user that image provenance is not verified and confirm the source is trusted before creating the VM
**Secondary Networks**: `vm_create({"networks": ["vlan-network"], ...})` or `{"networks": [{"name": "eth1", "networkName": "vlan"}], ...}`
**Explicit Instance Type**: `vm_create({"instancetype": "u1.large", ...})`
2 changes: 1 addition & 1 deletion rh-virt/skills/vm-delete/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ This is preview only. No resources deleted.
## Advanced Features

### Batch Deletion
Delete multiple VMs with confirmation for each: `"Delete VMs test-01, test-02, test-03 in dev"` → Process each individually with full workflow. Use typed confirmation: `DELETE-3-VMS` for batch.
Delete multiple VMs: `"Delete VMs test-01, test-02, test-03 in dev"` → Process each VM individually through the full workflow (Steps 1-5), requiring typed confirmation of each VM name separately. Never use a generic batch confirmation — each VM must be confirmed by its exact name.

### Dry-Run Mode
Show deletion scope without executing: Execute Step 1-2, skip Steps 3-4. User request: "Show what would be deleted if I delete VM xyz"
Expand Down
Loading
Loading