HYPERFLEET-1199 - feat: Add /e2e-debug skill#63
Conversation
|
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:
📝 WalkthroughSummary by CodeRabbit
Walkthrough
Estimated code review effort: 3 (Moderate) | ~22 minutes 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
hyperfleet-devtools/skills/e2e-debug/SKILL.md (1)
6-6: 💤 Low valueRemove unused
Readfrom allowed-tools.Line 6 declares
allowed-tools: Bash, Read, WebFetch, AskUserQuestion, but the skill only uses Bash (for gh, gcloud, kubectl, jira commands), WebFetch (for GCS artifacts), and AskUserQuestion (line 26). The Read tool is not exercised. Per coding guidelines, do not request tools the skill does not use.♻️ Proposed fix
-allowed-tools: Bash, Read, WebFetch, AskUserQuestion +allowed-tools: Bash, WebFetch, AskUserQuestion🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfleet-devtools/skills/e2e-debug/SKILL.md` at line 6, The allowed-tools declaration includes the Read tool, but reviewing the skill implementation shows it only uses Bash, WebFetch, and AskUserQuestion. Remove Read from the allowed-tools list in the SKILL.md file to ensure only the tools actually used by the skill are declared, per coding guidelines.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hyperfleet-devtools/skills/e2e-debug/SKILL.md`:
- Line 266: The jira issue list command on line 266 is vulnerable to JQL
injection because the keyword-from-error is interpolated directly into the query
string without validation or escaping. To fix this, validate the keyword before
interpolation by filtering it to only alphanumeric characters, underscores, and
hyphens (removing or replacing any special characters or JQL operators), then
use the sanitized keyword in the query string. Alternatively, if the jira CLI
supports structured parameter passing or environment variables for query
parameters, use those mechanisms instead of string interpolation to avoid
injection entirely.
- Line 366: The kubectl port-forward commands at
hyperfleet-devtools/skills/e2e-debug/SKILL.md lines 366, 419, and 424 lack
timeout protection, causing indefinite hangs if the service is unreachable or
kubectl context is misconfigured. For each of these three locations, replace the
current `kubectl port-forward ... & PF_PID=$!; sleep 2; curl ...` pattern with a
timeout wrapper around the port-forward command (e.g., `timeout 5 kubectl
port-forward ...`), followed by a check to verify the process started
successfully using `kill -0 $PF_PID`, and only proceed with the curl command if
the process is running. This ensures the skill fails safely if port-forward
cannot establish within the timeout period, satisfying the fail-safe requirement
for dynamic context.
- Line 47: The SKILL.md file accepts a JIRA ticket input in the format
HYPERFLEET-XXXX without validating its format before passing it to the jira CLI
query around lines 262-270. This creates a security vulnerability where
malformed or attacker-controlled input could inject JQL metacharacters. Add
format validation early in the step (before line 266 where the jira query is
executed) to ensure the JIRA_TICKET variable matches the expected pattern of
uppercase project key characters, followed by a hyphen, followed by one or more
digits using a regex pattern like ^[A-Z]+-[0-9]+$. If the format is invalid,
output an error message and exit the step. This validation should be documented
or referenced at line 47 where the argument-hint is defined, and the actual
validation logic should be placed before the jira CLI execution in the 262-270
line range.
---
Nitpick comments:
In `@hyperfleet-devtools/skills/e2e-debug/SKILL.md`:
- Line 6: The allowed-tools declaration includes the Read tool, but reviewing
the skill implementation shows it only uses Bash, WebFetch, and AskUserQuestion.
Remove Read from the allowed-tools list in the SKILL.md file to ensure only the
tools actually used by the skill are declared, per coding guidelines.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 4e34d43d-e223-4891-a79e-9d2861e10f4e
📒 Files selected for processing (8)
.claude-plugin/marketplace.jsonAGENTS.mdhyperfleet-devtools/.claude-plugin/plugin.jsonhyperfleet-devtools/README.mdhyperfleet-devtools/docs/e2e-debug-presentation.mdhyperfleet-devtools/skills/e2e-debug/SKILL.mdhyperfleet-devtools/skills/e2e-debug/references/ci-quick-reference.mdhyperfleet-devtools/skills/e2e-debug/references/known-failure-patterns.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hyperfleet-devtools/skills/e2e-debug/SKILL.md`:
- Line 47: Add input validation to prevent JIRA and JQL injection
vulnerabilities by implementing two complementary fixes. At
hyperfleet-devtools/skills/e2e-debug/SKILL.md#L47-L47, add a format validation
check using the regex pattern `^[A-Z]+-[0-9]+$` to validate the JIRA ticket
input before it is used in Step 3c jira execution command. At
hyperfleet-devtools/skills/e2e-debug/SKILL.md#L262-L270, add keyword
sanitization that filters out non-alphanumeric characters from the extracted
keywords before line 266 where the keywords are interpolated into the JQL query.
Both fixes address the root cause of missing input validation on external input
passed directly to CLI commands.
- Around line 266-270: The jira issue list commands on lines 266 and subsequent
lines interpolate error keywords directly into JQL queries without sanitization,
creating a query injection vulnerability. Before interpolating the
keyword_from_error variable into the -q parameter, sanitize it to contain only
alphanumeric characters, hyphens, and underscores by using sed or similar
filtering (e.g., sed 's/[^a-zA-Z0-9_-]/ /g'). Apply this sanitization to all
locations where error text is extracted from logs and inserted into JQL query
strings to prevent injection of JQL operators or quotes that could modify the
query logic.
- Line 366: The kubectl port-forward commands at line 366 (Maestro DB check),
line 419 (Sentinel metrics), and line 424 (API status check) lack timeout
protection, which can cause indefinite hangs and block skill execution. For each
of these three instances, wrap the kubectl port-forward command with a timeout
wrapper (e.g., timeout 5), add a process-alive check using kill -0 on the PF_PID
variable to verify the port-forward started successfully, execute the curl
command only if the process is running, and ensure the process is cleaned up
with kill. This prevents hangs and ensures fail-safe behavior when services are
unreachable.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9bd2e055-80de-4459-ae65-f504bf0ee2c0
📒 Files selected for processing (8)
.claude-plugin/marketplace.jsonAGENTS.mdhyperfleet-devtools/.claude-plugin/plugin.jsonhyperfleet-devtools/README.mdhyperfleet-devtools/docs/e2e-debug-presentation.mdhyperfleet-devtools/skills/e2e-debug/SKILL.mdhyperfleet-devtools/skills/e2e-debug/references/ci-quick-reference.mdhyperfleet-devtools/skills/e2e-debug/references/known-failure-patterns.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
✅ Files skipped from review due to trivial changes (4)
- hyperfleet-devtools/.claude-plugin/plugin.json
- .claude-plugin/marketplace.json
- AGENTS.md
- hyperfleet-devtools/skills/e2e-debug/references/ci-quick-reference.md
- Add JIRA ticket format validation (^[A-Z]+-[0-9]+$) before CLI use - Sanitize JQL keywords from error logs (strip metacharacters) - Add timeout wrapper (timeout 10) to all kubectl port-forward commands - Add kill -0 process check before curl to fail safely if port-forward hangs - Keep Read in allowed-tools (needed for loading references/ files) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hyperfleet-devtools/skills/e2e-debug/SKILL.md`:
- Line 47: The JIRA ticket input handling at line 47 instructs to validate the
format against the regex pattern but contains no executable validation code,
creating a security risk for injection attacks. Add explicit validation code
immediately after the JIRA ticket input is identified and before any CLI command
is constructed: check if the input matches the specified regex pattern
(^[A-Z]+-[0-9]+$), and if validation fails, output an error message to stderr
and exit with a non-zero status code. Only proceed with the jira CLI call to
fetch the ticket and search for pipeline links once validation succeeds,
matching the defensive pattern already applied to keyword sanitization elsewhere
in the document.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 136bd9b0-79f3-4875-80d0-d7d35e861ce8
📒 Files selected for processing (1)
hyperfleet-devtools/skills/e2e-debug/SKILL.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
CodeRabbit flagged that line 47 had a validation instruction but no executable code, unlike the JQL keyword sanitization at line 268. Added explicit grep -qE validation with error message before jira CLI use. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
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 (1)
hyperfleet-devtools/skills/e2e-debug/SKILL.md (1)
555-556:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep live cluster data in the evidence set.
This guardrail excludes kubectl/gcloud evidence even though Step 5 makes live cluster inspection mandatory when available and the opening instructions already require live cluster corroboration. That contradiction can let the model certify a diagnosis without the only data source that confirms node drains, restarts, or Maestro state.
♻️ Proposed fix
-- **NO GUESSWORK:** Base your root cause ONLY on the intersection of logs, the debugging handbook, and the repository state. ++ **NO GUESSWORK:** Base your root cause ONLY on the intersection of logs, the debugging handbook, the repository state, and live cluster data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfleet-devtools/skills/e2e-debug/SKILL.md` around lines 555 - 556, The guardrails in the "NO HALLUCINATIONS" and "NO GUESSWORK" rules are excluding kubectl and gcloud evidence from the evidence set, which contradicts the requirement in Step 5 to mandatorily inspect the live cluster when available and the opening instructions requiring live cluster corroboration. Modify these two guardrail statements to explicitly include kubectl/gcloud evidence in the evidence set when available, ensuring that live cluster data about node drains, restarts, and Maestro state is always retained and used to confirm diagnoses rather than allowing certifications without this critical data source.
♻️ Duplicate comments (1)
hyperfleet-devtools/skills/e2e-debug/SKILL.md (1)
49-56:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFail closed on invalid JIRA keys.
The regex check only logs an error; it still falls through to
jira issue view, so malformed ticket IDs remain eligible for CLI use. That leaves the SEC-01 guard unenforced and reopens the injection path.🔒 Proposed fix
if ! echo "$JIRA_INPUT" | grep -qE '^[A-Z]+-[0-9]+$'; then echo "ERROR: Invalid JIRA ticket format. Expected: HYPERFLEET-1234. Received: $JIRA_INPUT" >&2 - # Stop — do not pass unvalidated input to jira CLI + exit 1 fi jira issue view "$JIRA_INPUT" --plain 2>/dev/nullAs per coding guidelines, SEC-01: validate input at system boundaries before passing untrusted data to CLI commands.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfleet-devtools/skills/e2e-debug/SKILL.md` around lines 49 - 56, The JIRA input validation in this script checks the format using grep with the regex pattern but fails to stop execution when validation fails. Currently, after logging the error message for an invalid JIRA ticket format, the script continues and passes the malformed JIRA_INPUT to the jira issue view command, creating a security vulnerability. Add an exit statement or equivalent control flow termination immediately after the error log within the validation block to ensure that execution stops and the jira CLI command is never invoked with invalid input, thus enforcing the SEC-01 validation requirement at the system boundary.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@hyperfleet-devtools/skills/e2e-debug/SKILL.md`:
- Around line 555-556: The guardrails in the "NO HALLUCINATIONS" and "NO
GUESSWORK" rules are excluding kubectl and gcloud evidence from the evidence
set, which contradicts the requirement in Step 5 to mandatorily inspect the live
cluster when available and the opening instructions requiring live cluster
corroboration. Modify these two guardrail statements to explicitly include
kubectl/gcloud evidence in the evidence set when available, ensuring that live
cluster data about node drains, restarts, and Maestro state is always retained
and used to confirm diagnoses rather than allowing certifications without this
critical data source.
---
Duplicate comments:
In `@hyperfleet-devtools/skills/e2e-debug/SKILL.md`:
- Around line 49-56: The JIRA input validation in this script checks the format
using grep with the regex pattern but fails to stop execution when validation
fails. Currently, after logging the error message for an invalid JIRA ticket
format, the script continues and passes the malformed JIRA_INPUT to the jira
issue view command, creating a security vulnerability. Add an exit statement or
equivalent control flow termination immediately after the error log within the
validation block to ensure that execution stops and the jira CLI command is
never invoked with invalid input, thus enforcing the SEC-01 validation
requirement at the system boundary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 950c36e4-7691-4160-85b3-d0607c31f38a
📒 Files selected for processing (1)
hyperfleet-devtools/skills/e2e-debug/SKILL.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
| **GKE maintenance policy** (check whether a maintenance window is configured to prevent this): | ||
| ```bash | ||
| gcloud container clusters describe hyperfleet-dev-prow --zone=<zone-from-step-1e> --format="yaml(maintenancePolicy)" 2>/dev/null | ||
| ``` |
There was a problem hiding this comment.
nit: The gcloud container clusters describe commands in Step 5c hardcode the cluster name hyperfleet-dev-prow while the zone is correctly parameterized. Step 1e already instructs to extract the cluster name dynamically from setup logs. Consider using a placeholder to stay consistent with the other dynamic references in this section.
|
|
||
| **Extract these timestamps from the artifacts you already read.** Different sources use different formats — normalize everything to UTC: | ||
| - `started.json` / `finished.json`: epoch seconds → convert to UTC | ||
| - Setup/cleanup `build-log.txt`: `DD-MM-YYYYTHH:MM:SS` format (already UTC) |
There was a problem hiding this comment.
question: Is this DD-MM-YYYY format verified against an actual build-log.txt? The skill is very prescriptive about timestamp parsing, and if this doesn't match the real format the model could build a wrong timeline, which feeds into every subsequent step (commit queries, GKE operation filters, cross-run comparison).
There was a problem hiding this comment.
corrected to ISO 8601 + MM/DD/YY for Ginkgo
| # All three commands must be in a single Bash invocation (shell state does not persist between calls) | ||
| timeout 10 kubectl port-forward -n "$MAESTRO_NS" svc/maestro 8001:8000 & PF_PID=$!; sleep 2; kill -0 $PF_PID 2>/dev/null && curl -s --max-time 5 "http://localhost:8001/api/maestro/v1/resource-bundles?size=1" | jq '.total'; kill $PF_PID 2>/dev/null | ||
| ``` |
There was a problem hiding this comment.
This port-forward + curl pattern is repeated 3 times in Step 5 (Maestro DB, Sentinel metrics, API status). Consider extracting it into a utility script like scripts/port-forward-query.sh. Per skill authoring best practices, utility scripts are more reliable than repeated inline code and help keep the SKILL.md under the recommended 500-line limit.
| ## Step 5: Live Cluster & Cloud State Inspection | ||
|
|
||
| **This step is MANDATORY when kubectl is available (per Dynamic Context).** You MUST run the relevant checks below to confirm or refute your hypothesis from Step 4. Do not skip this step — log-based diagnosis alone produces assumptions, not facts. If a kubectl command fails (e.g., not connected, wrong context, namespace deleted), note what failed and move on — a failed check is still more informative than no check at all. | ||
|
|
||
| Skip this step entirely ONLY if NEITHER kubectl NOR gcloud is available. Steps 5a, 5b, and 5d require only kubectl. Step 5c requires gcloud — skip it individually if gcloud is not available. This step is **read-only** — never modify cluster state, delete resources, or scale deployments. |
There was a problem hiding this comment.
Step 5 is the longest section in the skill and contains mostly kubectl/gcloud command blocks. Consider moving the commands into a reference file and keeping just the high-level instructions here. This would help bring the SKILL.md under the recommended 500-line limit
| description: | | ||
| Analyze HyperFleet E2E CI pipeline failures from a Prow URL, GitHub Actions URL, job name, or JIRA ticket. | ||
| Retrieves logs, matches against the debugging handbook, checks recent commits/JIRA, and outputs a structured root cause analysis. |
There was a problem hiding this comment.
The description should use third person ("Analyzes" instead of "Analyze") and include a "Use when..." clause to help with skill discovery. The description is what the model uses to decide whether to trigger the skill.
| ## Output Format | ||
|
|
||
| Present findings in this exact structure: | ||
|
|
||
| ```markdown | ||
| **CI Failure Analysis for:** [Job Name / Run ID] | ||
| **URL:** [Pipeline URL] | ||
| **Confidence:** [HIGH | MEDIUM | LOW] | ||
|
|
||
| ### 1. The Failure Point | ||
| * **Failing Step/Job:** [Name of the Prow step or GitHub Actions job] | ||
| * **Failing Test:** [Full Describe/Context/It path, if applicable] |
There was a problem hiding this comment.
Consider adding a worked example of a filled-in output as a reference file (e.g., references/output-example.md). The template defines the structure well, but an example would anchor the expected length, tone, and level of detail for each section, which the model can't infer from placeholders alone.
| ### 1c. Fetch logs based on type | ||
|
|
||
| **For Prow jobs:** | ||
|
|
||
| 1. Fetch the build log from the GCS web interface. The Prow URL maps to a GCS path: | ||
| - URL: `https://prow.ci.openshift.org/view/gs/test-platform-results/logs/<job-name>/<run-id>` | ||
| - GCS web: `https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/logs/<job-name>/<run-id>/` |
There was a problem hiding this comment.
The log retrieval here relies on WebFetch + parsing gcsweb HTML directory listings. If the HTML structure changes, the skill silently breaks. Worth checking out redhat-community-ai-tools/prow-mcp-server as a more robust alternative. It wraps the same GCS endpoints with proper API tools for fetching logs, extracting test failures, and diagnosing builds.
…bbit) - Add missing `exit 1` in JIRA validation block (CodeRabbit: CWE-78 guard was dead) - Fix timestamp format: DD-MM-YYYY was wrong, actual format is ISO 8601 for setup/cleanup logs and MM/DD/YY HH:MM:SS for Ginkgo STEP markers - Replace hardcoded `hyperfleet-dev-prow` in gcloud commands with `<cluster-name-from-step-1e>` placeholder (pnguyen44 nit) - Update description to third person + "Use when..." clause for skill discovery (pnguyen44) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
- Move all kubectl/gcloud commands from SKILL.md Step 5 into references/cluster-inspection-commands.md (pnguyen44 comments openshift-hyperfleet#8, openshift-hyperfleet#9) - Add references/output-example.md with the validated broker.type diagnosis as a worked example (pnguyen44 comment openshift-hyperfleet#11) - SKILL.md reduced from 560 to 449 lines (under 500-line recommendation) - Add prow-mcp-server note for future WebFetch replacement (comment openshift-hyperfleet#12) - Update references section to list new files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@hyperfleet-devtools/skills/e2e-debug/references/cluster-inspection-commands.md`:
- Around line 83-86: The gcloud container operations query in the
cluster-inspection commands uses a calendar-day startTime filter that can miss
upgrades or repairs spanning midnight. Update the operation lookup to use the
Step 1e timestamp boundaries and query for overlap with the failure window
instead of only filtering by the run date; keep the same command block and
symbols like gcloud container operations list, operationType, and startTime, but
change the time predicate so it catches operations that began before midnight
and were still active during the incident.
- Line 27: The port-forward check in the cluster inspection commands relies on a
fixed sleep, which can race before the local listener is actually ready. Update
the command sequence around the kubectl port-forward/curl flow to wait
explicitly for readiness before querying the API, using the same pattern in the
affected snippets of cluster-inspection-commands.md. Keep the port-forward
process handling intact with PF_PID, but replace the blind delay with a
readiness poll or equivalent blocking check before invoking curl and jq.
- Around line 24-27: The cluster discovery in the Bash snippet is too broad and
can select the wrong Maestro-related service, so tighten the lookup in the
command sequence that sets MAESTRO_NS and runs kubectl port-forward. Replace the
loose grep-based matching with an exact service-name match for the maestro
service, and fail closed if no namespace or more than one namespace matches
instead of picking the first result. Apply the same fix to the other referenced
discovery commands so downstream reads always use the intended namespace.
In `@hyperfleet-devtools/skills/e2e-debug/references/output-example.md`:
- Around line 23-24: The broker-type validation reference in the example points
to the wrong template path, so update the documentation text to point readers to
the actual Helm locations used for the required broker type logic. In the
output-example markdown, replace the incorrect templates/deployment.yaml
reference with the real symbols involved: the broker type resolution in
_helpers.tpl and the invalid-type hard fail in configmap-broker.yaml, so anyone
following the example can verify the regression against the correct files.
In `@hyperfleet-devtools/skills/e2e-debug/SKILL.md`:
- Around line 79-80: The artifact traversal guidance in the e2e-debug skill is
unbounded and can self-DoS; update the workflow in SKILL.md to add a hard limit
to the recursive walk. Keep the guidance tied to the artifact tree traversal
step by naming the walk over the run root, and define an explicit budget such as
maximum depth, maximum files, or a wall-clock timeout. Make the instructions
fail-safe and read-only by requiring early stop once the budget is reached,
rather than insisting on reading every artifact unconditionally.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 640ffaf4-b214-42ec-8893-28bf058d26bb
📒 Files selected for processing (3)
hyperfleet-devtools/skills/e2e-debug/SKILL.mdhyperfleet-devtools/skills/e2e-debug/references/cluster-inspection-commands.mdhyperfleet-devtools/skills/e2e-debug/references/output-example.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
| MAESTRO_NS=$(kubectl get svc --all-namespaces --no-headers 2>/dev/null | grep maestro | awk '{print $1}' | head -1) | ||
|
|
||
| # All commands in a single Bash invocation (shell state does not persist between calls) | ||
| timeout 10 kubectl port-forward -n "$MAESTRO_NS" svc/maestro 8001:8000 & PF_PID=$!; sleep 2; kill -0 $PF_PID 2>/dev/null && curl -s --max-time 5 "http://localhost:8001/api/maestro/v1/resource-bundles?size=1" | jq '.total'; kill $PF_PID 2>/dev/null |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Constrain service discovery; this can bind to the wrong service.
grep maestro | head -1 is not unique, so a shared cluster can feed the port-forward from an unrelated Maestro/Sentinel namespace and poison every downstream read. CWE-20. Require an exact service match or fail closed when more than one namespace matches.
Also applies to: 69-70, 76-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@hyperfleet-devtools/skills/e2e-debug/references/cluster-inspection-commands.md`
around lines 24 - 27, The cluster discovery in the Bash snippet is too broad and
can select the wrong Maestro-related service, so tighten the lookup in the
command sequence that sets MAESTRO_NS and runs kubectl port-forward. Replace the
loose grep-based matching with an exact service-name match for the maestro
service, and fail closed if no namespace or more than one namespace matches
instead of picking the first result. Apply the same fix to the other referenced
discovery commands so downstream reads always use the intended namespace.
| MAESTRO_NS=$(kubectl get svc --all-namespaces --no-headers 2>/dev/null | grep maestro | awk '{print $1}' | head -1) | ||
|
|
||
| # All commands in a single Bash invocation (shell state does not persist between calls) | ||
| timeout 10 kubectl port-forward -n "$MAESTRO_NS" svc/maestro 8001:8000 & PF_PID=$!; sleep 2; kill -0 $PF_PID 2>/dev/null && curl -s --max-time 5 "http://localhost:8001/api/maestro/v1/resource-bundles?size=1" | jq '.total'; kill $PF_PID 2>/dev/null |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wait for the port-forward to be ready before curling it.
The fixed sleep 2 is a TOCTOU race (CWE-367): curl/jq can run before the local bind is usable, producing empty or partial output and a false diagnosis. Poll readiness or block on the forwarder explicitly before reading metrics/API responses.
Also applies to: 70-70, 76-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@hyperfleet-devtools/skills/e2e-debug/references/cluster-inspection-commands.md`
at line 27, The port-forward check in the cluster inspection commands relies on
a fixed sleep, which can race before the local listener is actually ready.
Update the command sequence around the kubectl port-forward/curl flow to wait
explicitly for readiness before querying the API, using the same pattern in the
affected snippets of cluster-inspection-commands.md. Keep the port-forward
process handling intact with PF_PID, but replace the blind delay with a
readiness poll or equivalent blocking check before invoking curl and jq.
| gcloud container operations list --zone=<zone-from-step-1e> --filter="operationType:(UPGRADE_NODES OR REPAIR_CLUSTER OR UPGRADE_MASTER) AND startTime>='<run-date>T00:00:00Z' AND startTime<='<run-date>T23:59:59Z'" --format="table(name,operationType,startTime,endTime,status)" 2>/dev/null | ||
|
|
||
| # Also check the day before | ||
| gcloud container operations list --zone=<zone-from-step-1e> --filter="operationType:(UPGRADE_NODES OR REPAIR_CLUSTER OR UPGRADE_MASTER) AND startTime>='<day-before-run>T00:00:00Z' AND startTime<='<run-date>T23:59:59Z'" --format="table(name,operationType,startTime,endTime,status)" 2>/dev/null |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Filter node operations by overlap, not calendar day.
The current startTime window misses upgrades/repairs that began before midnight but overlapped the failure window. That is exactly the failure class this step is supposed to catch. CWE-20. Use the Step 1e timestamps as the boundary and query for time overlap instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@hyperfleet-devtools/skills/e2e-debug/references/cluster-inspection-commands.md`
around lines 83 - 86, The gcloud container operations query in the
cluster-inspection commands uses a calendar-day startTime filter that can miss
upgrades or repairs spanning midnight. Update the operation lookup to use the
Step 1e timestamp boundaries and query for overlap with the failure window
instead of only filtering by the run date; keep the same command block and
symbols like gcloud container operations list, operationType, and startTime, but
change the time predicate so it catches operations that began before midnight
and were still active during the incident.
| * The adapter Helm chart (`hyperfleet-adapter` PR #160, `HYPERFLEET-1104`, merged May 21) changed the broker type resolution from an inference-based fallback to a hard `required` call at `templates/deployment.yaml:94`. The E2E test helpers that deploy ephemeral negative-scenario adapters (`cl-m-unreg-consumer`, `cl-m-wrong-ds`, `cl-m-wrong-nest`, `cl-m-bad-api`, `cl-precondition-error`) are not passing the `broker.type` Helm value in their override configurations. The adapters fail at Helm template validation before any pods are created, so the negative scenarios never reach the application-level failure behavior they are designed to test. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the Helm path in the example.
The broker-type validation is in hyperfleet-adapter/_helpers.tpl:292-295 (and the invalid-type hard fail is in configmap-broker.yaml:3-5), not templates/deployment.yaml:94. This example currently points readers at the wrong file when they try to verify the regression.
🔧 Proposed fix
- The adapter Helm chart (`hyperfleet-adapter` PR `#160`, `HYPERFLEET-1104`, merged May 21) changed the broker type resolution from an inference-based fallback to a hard `required` call at `templates/deployment.yaml:94`.
+ The adapter Helm chart (`hyperfleet-adapter` PR `#160`, `HYPERFLEET-1104`, merged May 21) changed the broker type resolution from an inference-based fallback to a hard `required` call in `_helpers.tpl:292-295` (with invalid-type hard-fail in `configmap-broker.yaml:3-5`).📝 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.
| * The adapter Helm chart (`hyperfleet-adapter` PR #160, `HYPERFLEET-1104`, merged May 21) changed the broker type resolution from an inference-based fallback to a hard `required` call at `templates/deployment.yaml:94`. The E2E test helpers that deploy ephemeral negative-scenario adapters (`cl-m-unreg-consumer`, `cl-m-wrong-ds`, `cl-m-wrong-nest`, `cl-m-bad-api`, `cl-precondition-error`) are not passing the `broker.type` Helm value in their override configurations. The adapters fail at Helm template validation before any pods are created, so the negative scenarios never reach the application-level failure behavior they are designed to test. | |
| * The adapter Helm chart (`hyperfleet-adapter` PR `#160`, `HYPERFLEET-1104`, merged May 21) changed the broker type resolution from an inference-based fallback to a hard `required` call in `_helpers.tpl:292-295` (with invalid-type hard-fail in `configmap-broker.yaml:3-5`). The E2E test helpers that deploy ephemeral negative-scenario adapters (`cl-m-unreg-consumer`, `cl-m-wrong-ds`, `cl-m-wrong-nest`, `cl-m-bad-api`, `cl-precondition-error`) are not passing the `broker.type` Helm value in their override configurations. The adapters fail at Helm template validation before any pods are created, so the negative scenarios never reach the application-level failure behavior they are designed to test. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hyperfleet-devtools/skills/e2e-debug/references/output-example.md` around
lines 23 - 24, The broker-type validation reference in the example points to the
wrong template path, so update the documentation text to point readers to the
actual Helm locations used for the required broker type logic. In the
output-example markdown, replace the incorrect templates/deployment.yaml
reference with the real symbols involved: the broker type resolution in
_helpers.tpl and the invalid-type hard fail in configmap-broker.yaml, so anyone
following the example can verify the regression against the correct files.
Source: Linked repositories
| 2. **Walk the ENTIRE artifact tree. No shortcuts. No skipping.** (Future: consider [prow-mcp-server](https://github.com/redhat-community-ai-tools/prow-mcp-server) as an alternative to WebFetch + HTML parsing for more robust GCS artifact access.) The GCS artifacts are the primary and most reliable source of truth. They are always available regardless of kubectl access. You MUST recursively list and read every file in every directory starting from the run root. Do not cherry-pick. Do not skip directories you think are unimportant. Do not assume you know what's there. | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Bound the artifact walk or this skill can self-DoS.
"recursively list and read every file in every directory" is a CWE-400 / EA4 pattern: a large or malformed GCS tree can burn time and tokens before diagnosis starts. Add a hard budget (depth, file count, or wall-clock timeout) so the workflow stays fail-safe. As per path instructions, keep the workflow fast, read-only, and fail-safe.
🔧 Proposed fix
- 2. **Walk the ENTIRE artifact tree. No shortcuts. No skipping.** (Future: consider [prow-mcp-server](https://github.com/redhat-community-ai-tools/prow-mcp-server) as an alternative to WebFetch + HTML parsing for more robust GCS artifact access.) The GCS artifacts are the primary and most reliable source of truth. They are always available regardless of kubectl access. You MUST recursively list and read every file in every directory starting from the run root. Do not cherry-pick. Do not skip directories you think are unimportant. Do not assume you know what's there.
+ 2. **Walk the artifact tree with a hard budget.** Use a bounded timeout and file-count cap so oversized or pathological runs cannot blow up cost or runtime. If the cap is hit, report the traversal as truncated and continue with the highest-signal artifacts first.📝 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.
| 2. **Walk the ENTIRE artifact tree. No shortcuts. No skipping.** (Future: consider [prow-mcp-server](https://github.com/redhat-community-ai-tools/prow-mcp-server) as an alternative to WebFetch + HTML parsing for more robust GCS artifact access.) The GCS artifacts are the primary and most reliable source of truth. They are always available regardless of kubectl access. You MUST recursively list and read every file in every directory starting from the run root. Do not cherry-pick. Do not skip directories you think are unimportant. Do not assume you know what's there. | |
| 2. **Walk the artifact tree with a hard budget.** Use a bounded timeout and file-count cap so oversized or pathological runs cannot blow up cost or runtime. If the cap is hit, report the traversal as truncated and continue with the highest-signal artifacts first. |
🧰 Tools
🪛 LanguageTool
[style] ~79-~79: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... directories you think are unimportant. Do not assume you know what's there. *...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 SkillSpector (2.3.7)
[warning] 248: [EA4] Unbounded Resource Access: Skill allows unbounded resource consumption (API calls, storage, compute). Without rate limits or quotas, a compromised or misbehaving agent can cause denial-of-service or cost overruns.
Remediation: Set explicit rate limits, timeouts, and resource quotas for API calls, file operations, and compute. Implement circuit breakers for runaway loops.
(Excessive Agency (EA4))
[error] 315: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hyperfleet-devtools/skills/e2e-debug/SKILL.md` around lines 79 - 80, The
artifact traversal guidance in the e2e-debug skill is unbounded and can
self-DoS; update the workflow in SKILL.md to add a hard limit to the recursive
walk. Keep the guidance tied to the artifact tree traversal step by naming the
walk over the run root, and define an explicit budget such as maximum depth,
maximum files, or a wall-clock timeout. Make the instructions fail-safe and
read-only by requiring early stop once the budget is reached, rather than
insisting on reading every artifact unconditionally.
Sources: Path instructions, Linters/SAST tools
2 participants
Summary
/e2e-debugskill to thehyperfleet-devtoolsplugin — an AI-powered forensic debugger that automates root cause analysis of failed E2E CI pipeline runsWhat it does
The skill runs a 6-step forensic workflow:
finished.json, cross-run comparison (commit, chart versions, GKE cluster version, node names)gcloud container operations list), maintenance policy, pod health, Sentinel metricsKey design decisions
since=/until=, PR queries usemerged:>, gcloud operations use date filters. No--limitflags anywheregcloud container operations listfinds UPGRADE_NODES overlapping the test window (see HYPERFLEET-1225)ghCLI alone. kubectl/gcloud add live cluster validation but are optional. Confidence capped at MEDIUM if kubectl is available but not usedNegative scenarios handled
finished.json, reports statusall-resources.txt→ notes "node assignments unknown" and proceedsFiles changed
hyperfleet-devtools/skills/e2e-debug/SKILL.mdhyperfleet-devtools/skills/e2e-debug/references/known-failure-patterns.mdhyperfleet-devtools/skills/e2e-debug/references/ci-quick-reference.mdhyperfleet-devtools/.claude-plugin/plugin.jsonhyperfleet-devtools/README.md.claude-plugin/marketplace.jsonAGENTS.mdhyperfleet-devtools/docs/Test plan
claude --plugin-dir ./hyperfleet-devtools/hyperfleet-devtools:e2e-debugin available skillslatest-build.txtand resolve