MCO-2220: MCO-2221: Add MCO QE tooling: Polarion TC creation from GitHub PRs and Jira#6149
Open
ptalgulk01 wants to merge 1 commit into
Open
MCO-2220: MCO-2221: Add MCO QE tooling: Polarion TC creation from GitHub PRs and Jira#6149ptalgulk01 wants to merge 1 commit into
ptalgulk01 wants to merge 1 commit into
Conversation
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
WalkthroughThis PR introduces a complete MCO QE tooling system for managing Polarion test cases. It adds comprehensive documentation, Python scripts for fetching and creating test cases from GitHub PRs and Jira, a Polarion HTTP client supporting REST and SOAP, validation logic, configuration templates, and a setup script to bootstrap the environment. ChangesMCO QE Tooling
Sequence Diagram(s)sequenceDiagram
participant User
participant FetchScripts
participant Validator
participant CreateScripts
participant PolarionClient
participant PolarionAPI
User->>FetchScripts: Run fetch command
FetchScripts->>PolarionAPI: Request source data
PolarionAPI-->>FetchScripts: Return payload
FetchScripts-->>User: Output draft JSON
User->>Validator: Validate draft
Validator-->>User: Return report
User->>CreateScripts: Submit draft for creation
CreateScripts->>PolarionClient: Create and update testcase
PolarionClient->>PolarionAPI: POST testcase
PolarionClient->>PolarionAPI: PATCH custom fields
PolarionClient->>PolarionAPI: Add test steps
PolarionAPI-->>PolarionClient: Return status
CreateScripts-->>User: Return result JSON
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
.claude/commands/mco-create-tc-from-pr.md-10-12 (1)
10-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSpecify languages for unlabeled fenced code blocks.
Add
text/bashwhere appropriate to satisfy MD040 consistently.Also applies to: 78-84
🤖 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 @.claude/commands/mco-create-tc-from-pr.md around lines 10 - 12, The markdown fenced code blocks containing the command examples (e.g., the snippet showing `/mco-create-tc-from-pr <PR-URL>`) need explicit language identifiers to satisfy MD040; update those fences to use an appropriate language tag such as "bash" or "text" (for example change ``` to ```bash or ```text) for the snippet in this file and the other similar fenced blocks referenced around the second occurrence (the block covering lines 78-84) so all unlabeled code fences are consistently labeled..claude/scripts/create_tc_hybrid.py-163-165 (1)
163-165:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidation failure output is mostly dropped.
validate_tc.pyprints its report to stdout, but this branch prints onlyvalidate_result.stderr, so users lose actionable failure details.🤖 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 @.claude/scripts/create_tc_hybrid.py around lines 163 - 165, The validation branch currently prints only validate_result.stderr so the useful report emitted by validate_tc.py (on stdout) is lost; update the failing branch that calls validate_result to print validate_result.stdout (or both validate_result.stdout and validate_result.stderr) to stderr before exiting so users see the full validation report—locate the print statements around validate_result in create_tc_hybrid.py and replace or augment printing validate_result.stderr with printing validate_result.stdout (and preserve stderr output as well)..claude/commands/mco-fetch.md-13-17 (1)
13-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language to the Usage code fence.
This resolves MD040 and keeps docs lint-clean.
🤖 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 @.claude/commands/mco-fetch.md around lines 13 - 17, The fenced code block showing the example commands (the lines starting with "/mco-fetch tc OCP-88122", "/mco-fetch pr 5691 [comment-id]", "/mco-fetch jira OCPBUGS-74223") is missing a language tag; update the opening triple-backtick fence to include a language (e.g., ```bash or ```sh) so the block is lint-clean and resolves MD040..claude/commands/mco-create-tc-from-jira.md-10-12 (1)
10-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language tags to unlabeled code fences.
Use
textfor slash-command snippets andtext/bashfor heredoc replacement blocks to clear MD040.Also applies to: 80-86
🤖 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 @.claude/commands/mco-create-tc-from-jira.md around lines 10 - 12, The unlabeled code fence containing the slash-command snippet "/mco-create-tc-from-jira <JIRA-URL>" and the heredoc replacement blocks should have language tags added to satisfy MD040: add "text" to the single-line slash-command fence and add "text" or "bash" (as appropriate) to the heredoc replacement code fences referenced around the heredoc blocks (also update the similar fences at lines indicated for the other heredoc snippets, e.g., the blocks around lines 80-86) so each triple-backtick fence includes an explicit language tag..claude/scripts/validate_tc.py-242-244 (1)
242-244:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSample draft creation can fail when
/tmp/test-specsdoesn’t exist.Line 243 writes directly to a nested tmp path without ensuring the parent directory exists.
Suggested fix
+import os @@ filepath = '/tmp/test-specs/sample_draft.json' + os.makedirs(os.path.dirname(filepath), exist_ok=True) with open(filepath, 'w') as f: json.dump(sample, f, indent=2)🤖 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 @.claude/scripts/validate_tc.py around lines 242 - 244, The write to filepath = '/tmp/test-specs/sample_draft.json' can fail if the parent directory doesn't exist; before opening the file, ensure the parent directory exists by creating os.path.dirname(filepath) with os.makedirs(..., exist_ok=True) (and add an import for os if missing), then proceed to open and json.dump(sample, ...)..claude/README.md-165-170 (1)
165-170:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix broken documentation references in .claude/README.md (lines 165–170)
The QUICKSTART references these files, but they don’t exist in the repo:
.claude/FETCH_USAGE.md.claude/CREATE_TC_USAGE.md.claude/UPDATE_TC_USAGE.md.claude/COMPLETE_WORKFLOW.md.claude/FINAL_SUMMARY.mdAdd the missing docs or update the references to point to the correct existing content.
🤖 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 @.claude/README.md around lines 165 - 170, README references to .claude/FETCH_USAGE.md, .claude/CREATE_TC_USAGE.md, .claude/UPDATE_TC_USAGE.md, .claude/COMPLETE_WORKFLOW.md, and .claude/FINAL_SUMMARY.md are broken; either add those missing markdown files with the appropriate quickstart content or update the links in .claude/README.md to point to the actual existing docs. Locate the QUICKSTART block in .claude/README.md and for each referenced filename either (A) create a corresponding file with the expected usage/test-step content and a brief header, or (B) replace the link target with the correct existing file names (or inline the content into the README) so the references resolve; ensure the link text and file names match exactly..claude/README.md-248-248 (1)
248-248:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix broken PDF example references in
.claude/README.md.
.claude/README.mdreferenceswrong.pdfandOCP-88122.pdf“in repo root” (e.g., around line 248), but neither file exists anywhere in the repository, so the examples will be unusable. Update the doc to point to the actual location for these artifacts or include the PDFs in the repo/PR.🤖 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 @.claude/README.md at line 248, The README references non-existent artifacts named "wrong.pdf" and "OCP-88122.pdf" in .claude/README.md; either update those references to point to the actual locations (e.g., correct relative paths, external URLs, or an artifacts directory), or add the two PDF files to the repo/PR and update the README to reference their repo paths; ensure the references to "wrong.pdf" and "OCP-88122.pdf" in .claude/README.md are accurate and accessible..claude/scripts/setup.sh-102-105 (1)
102-105:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrint the actual setup script path.
Lines 105 and 197 tell users to run
./scripts/setup.sh --verify, but this PR adds.claude/scripts/setup.sh. Following the printed command from repo root will fail.Also applies to: 195-198
🤖 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 @.claude/scripts/setup.sh around lines 102 - 105, Update the printed next steps so the path points to the new script location: replace the echo that prints "./scripts/setup.sh --verify" with ".claude/scripts/setup.sh --verify" (and update the matching echo on the other occurrence) so the "⚠️ NEXT STEPS:" message reflects the actual script path; locate the echo statements that output the run command string and change the path text accordingly.
🧹 Nitpick comments (5)
.claude/skills/mco-create-tc-workflow.md (2)
171-216: 💤 Low valueConsider documenting loop limit rationale.
The step deletion loop has a hardcoded limit of 10 attempts (line 205). For test cases with more than 10 steps, this could leave orphaned steps. Consider either:
- Increasing the limit (e.g., 50 or 100)
- Making it dynamic based on the number of steps to upload
- Adding a check after the loop to verify all steps were deleted
💡 Suggested improvement
-# Loop-delete: Polarion re-indexes IDs after each delete, so fetch fresh each round -for attempt in range(10): +# Loop-delete: Polarion re-indexes IDs after each delete, so fetch fresh each round +# Use generous limit to handle TCs with many steps +for attempt in range(50): result = client._make_request("GET", f"projects/OSE/workitems/{TC_ID}/teststeps") remaining = result.get("data", []) if not remaining: break for s in remaining: sid = s.get('id', '').split('/')[-1] client._make_request("DELETE", f"projects/OSE/workitems/{TC_ID}/teststeps/{sid}") time.sleep(0.5) + +# Verify all steps deleted +if remaining: + print(f"⚠️ Warning: {len(remaining)} steps remain after 50 attempts", file=sys.stderr)🤖 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 @.claude/skills/mco-create-tc-workflow.md around lines 171 - 216, The hardcoded retry limit in the deletion loop (for attempt in range(10)) can leave orphaned steps; update the loop around client._make_request DELETE calls (and the surrounding loop that reads result = client._make_request("GET", f"projects/OSE/workitems/{TC_ID}/teststeps")) to use a dynamic limit or higher cap (e.g., compute from initial remaining count or use 100) and add an explicit post-loop verification that remaining is empty—if not, log/raise an error so callers know deletion failed; keep references to load_env, TC_ID, and client._make_request when making changes.
27-41: ⚡ Quick winGitHub API authentication missing in example.
The Python snippet for fetching PR reviews uses unauthenticated requests, which are subject to GitHub's 60 requests/hour rate limit. For production use, consider adding authentication via
GITHUB_TOKENenvironment variable.🔐 Suggested enhancement
+import os import urllib.request, json def fetch(url): + headers = { + "Accept": "application/vnd.github.v3+json", + "User-Agent": "mco-tc-script" + } + # Use token if available to avoid rate limits + token = os.environ.get('GITHUB_TOKEN') + if token: + headers["Authorization"] = f"token {token}" + req = urllib.request.Request(url, headers={ - "Accept": "application/vnd.github.v3+json", - "User-Agent": "mco-tc-script" + **headers }) return json.loads(urllib.request.urlopen(req).read())🤖 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 @.claude/skills/mco-create-tc-workflow.md around lines 27 - 41, The example uses unauthenticated GitHub API calls in the fetch function which hit low rate limits; update fetch to read a GITHUB_TOKEN from environment and add an Authorization: token <GITHUB_TOKEN> header (and keep Accept/User-Agent) so requests are authenticated, and handle the missing token case by either raising a clear error or falling back with a warning; modify the fetch function and its header construction (refer to function name fetch and the existing header dict) so PR and reviews calls use the token..claude/skills/mco-automate-test-workflow.md (1)
33-36: 💤 Low valueConsider explicit validation script invocation.
The validation step states "Ensure test steps have commands (not just narrative)" but doesn't specify whether this is a manual check or should run
validate_tc.py. For consistency with the create-tc workflow (which explicitly runsvalidate_tc.py), consider clarifying whether automated validation is expected here.📝 Suggested clarification
3. **Validate**: + ```bash + python3 .claude/scripts/validate_tc.py /tmp/test-specs/<id>.txt + ``` - - Ensure test steps have commands (not just narrative) + - Verify test steps have commands (not just narrative) - Verify required fields present (Component, Sub Team, Version)Or alternatively, if manual validation is intended:
3. **Validate** (manual check): - Ensure test steps have commands (not just narrative) - Verify required fields present (Component, Sub Team, Version)🤖 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 @.claude/skills/mco-automate-test-workflow.md around lines 33 - 36, Clarify the "Validate" step by explicitly stating whether automated validation should run the existing script or remain manual: if automated, add a line invoking python3 .claude/scripts/validate_tc.py against the generated spec (e.g., `python3 .claude/scripts/validate_tc.py /tmp/test-specs/<id>.txt`) and change "Ensure test steps have commands (not just narrative)" to "Verify test steps have commands (not just narrative)"; if manual, add a note that reviewers must perform this check and reference the create-tc workflow as an example. Include the script name validate_tc.py and the create-tc workflow reference to make the expected action unambiguous..env.example (1)
29-29: ⚡ Quick winAdd quotes around multi-word values.
The value
Machine Config Operatorcontains spaces but is not quoted. While some.envparsers handle this, it's safer to quote multi-word values for consistency and portability.📝 Suggested fix
# MCO Test Defaults -MCO_COMPONENT=Machine Config Operator +MCO_COMPONENT="Machine Config Operator" MCO_SUB_TEAM=MCO MCO_TEST_TYPE=Functional MCO_PRODUCTS=OCP🤖 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 @.env.example at line 29, The MCO_COMPONENT environment value is a multi-word string and should be quoted for portability; update the .env example by changing the MCO_COMPONENT assignment to include quotes around "Machine Config Operator" so the variable MCO_COMPONENT is defined as MCO_COMPONENT="Machine Config Operator"..claude/TESTING.md (1)
126-130: ⚡ Quick winSimplify or remove the unsafe Python one-liner.
The test command attempts to manipulate
sys.pathand execute a file directly, which is complex and could be fragile.Consider a simpler approach:
🔄 Suggested alternative
### Missing Dependencies ```bash -# Simulate missing requests library -python3 -c "import sys; sys.path = [p for p in sys.path if 'site-packages' not in p]; exec(open('.claude/scripts/fetch_polarion.py').read())" +# Test graceful handling when requests library is missing +# (Remove requests temporarily or test in clean venv) +python3 -c "import requests" || echo "Expected: requests not installed" +python3 .claude/scripts/fetch_polarion.py OCP-88122-Expected: Clear error about missing
requestslibrary
+Expected: Script detects missing dependency and provides helpful error messageThe current one-liner is difficult to maintain and doesn't reliably simulate the missing dependency scenario.
🤖 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 @.claude/TESTING.md around lines 126 - 130, Replace the fragile sys.path hack in the TESTING.md example with a simpler, safer test for the missing requests dependency: remove the python -c exec one-liner and instead show a reproducible approach such as verifying import requests fails (or instructing to use a clean virtualenv) and then running .claude/scripts/fetch_polarion.py with a sample ticket ID (e.g., OCP-88122) to confirm the script emits a clear dependency error; update the "Expected" text to say the script detects the missing dependency and prints a helpful error message.
🤖 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 @.claude/commands/mco-create-tc-from-jira.md:
- Around line 130-141: The Step 7 example is inconsistent: it says "Always patch
fields + steps separately" but the shown command to run update_polarion_tc.py
only patches fields; add the steps argument to the example and clarify usage of
the --steps-file option. Update the Step 7 documentation to call the script
update_polarion_tc.py with both the field flags (e.g., --component, --sub-team,
--products, --test-type, --version, --trello-jira) and the --steps-file
"<PATH_TO_STEPS>" parameter so the command actually patches test steps as well.
In @.claude/scripts/create_polarion_tc.py:
- Around line 151-154: The code currently prints warnings when patch_result or
step upload fails but still returns a success status, so update the function
that builds and returns the result (the dict containing "status") to reflect
errors: check patch_result for "error" and the step upload result (e.g.,
step_upload_result or similar variable used in the 187-200 block) and if either
contains an error set the returned "status" to "failed" (or include a boolean
success flag) and include the error details in the return payload; ensure you
change both the custom-field patch branch (where patch_result and custom_fields
are used) and the step-upload branch to aggregate errors instead of only
printing warnings so downstream automation can reliably detect incomplete TC
creation.
- Around line 45-71: validate_draft currently only verifies that test_steps
exists and therefore allows drafts that the canonical validator (validate_tc.py)
would reject; update validate_draft to replicate the per-step validation from
validate_tc.py: iterate draft['test_steps'] and verify each step contains
required fields (e.g., 'action' and 'expected' or whatever the canonical
validator requires), enforce narrative-only constraints (disallow or flag steps
missing both action and expected unless marked as narrative), validate step
ordering/position if the canonical logic does, and append corresponding errors
to errors[] so the function returns False for invalid steps; reference
validate_draft, test_steps, and mirror the checks implemented in validate_tc.py
to ensure parity.
In @.claude/scripts/create_tc_hybrid.py:
- Around line 174-177: The dry-run subprocess invocation in create_tc_hybrid.py
currently ignores the child's return code; change the subprocess.run call that
executes ['python3', 'create_polarion_tc.py', draft_file, '--dry-run'] (in
create_tc_hybrid.py) to capture its CompletedProcess result and propagate
non-zero return codes (e.g., check result.returncode and
sys.exit(result.returncode) or raise SystemExit) so the script exits with the
child's error code when the dry-run fails.
- Around line 170-185: The script currently proceeds to call
subprocess.run(['python3', 'create_polarion_tc.py', draft_file]) when
args.dry_run is false, skipping the “confirm” step; modify the flow in the main
block where args.dry_run is checked to require an explicit confirmation before
invoking subprocess.run (either by adding support for an args.yes/--yes boolean
flag on the argparse parser or by prompting the user with input() and requiring
a "y/yes" response), and only call subprocess.run(...) and
sys.exit(result.returncode) if confirmation is received; reference the
args.dry_run conditional and the subprocess.run invocation for where to insert
the confirmation logic.
In @.claude/scripts/fetch_github_pr.py:
- Around line 310-323: The code currently always writes the parsed draft to
disk; add a fail-fast guard before the "Write draft" block that checks the
parsed draft's steps and aborts when none were parsed. After calling
parse_qe_comment (function parse_qe_comment) inspect the draft for the test
steps (e.g., draft.get("steps") or draft["steps"]) and if that list is empty,
log an error and exit non‑zero (or raise) instead of proceeding to open
output_file and json.dump; only proceed to create output_dir/open output_file
and write when there is at least one parsed step.
- Around line 98-105: The current Environment Setup capture regex (env_match)
stops at the next "Word:" field because it treats any inline field like
"Platform:" as a section boundary; change the lookahead so it only stops at a
real section header at the start of a line (optional '##' + capitalized header
name + ':'), e.g. replace the lookahead (?=\n(?:##\s*)?\w+:|\Z) with a
start-of-line anchored pattern using re.MULTILINE such as
(?=\n(?:##\s*)?[A-Z][A-Za-z0-9 _-]*:\s*\n|\Z) and apply re.MULTILINE in the
flags when compiling the env_match regex (update both regex attempts), so fields
like "Version:" and "Platform:" inside the Environment Setup block are captured
instead of terminating the match.
- Around line 62-82: fetch_pr_comments currently only requests a single page
from /issues/{pr_number}/comments and therefore misses paginated issue comments
and PR review comments; update fetch_pr_comments to iterate with per_page (e.g.,
100) and page params in a loop to collect all pages from the issues comments
endpoint, and additionally make one or two extra requests to collect review data
from /pulls/{pr_number}/reviews (and/or /pulls/{pr_number}/comments if you need
inline review comments), then merge the returned lists into a single List[Dict]
before returning; preserve the existing auth flow (including the 401 fallback
that removes Authorization) but ensure pagination loops repeat the
unauthenticated retry if needed and that response.raise_for_status is still used
for errors.
In @.claude/scripts/fetch_polarion.py:
- Around line 255-259: The PolarionClient is constructed without SOAP
credentials so the SOAP fallback used by _soap_get_test_steps() is unreachable;
update the PolarionClient(...) call (the instance created with polarion_url and
polarion_token) to also accept and pass through the SOAP credentials read from
the environment (e.g., polarion_username and polarion_password) so
_soap_get_test_steps() can authenticate and return steps for SOAP-only test
cases.
In @.claude/scripts/polarion_client.py:
- Around line 186-188: Replace the unsafe
xml.etree.ElementTree.fromstring(response.text) call used when parsing the SOAP
“get test steps” response: import and use defusedxml.ElementTree.fromstring
instead of xml.etree.ElementTree to prevent XXE/DoS attacks (locate the parsing
site where `root = ET.fromstring(response.text)` and the `ET` import), and
update the module imports to `import defusedxml.ElementTree as ET` (or
equivalent) for the function that handles the SOAP response; also ensure the
defusedxml package is added to the project dependencies so this script can
import it at runtime.
In @.claude/scripts/setup.sh:
- Around line 125-128: The script currently executes source "$REPO_ROOT/.env"
(bracketed by set -a / set +a) which can run arbitrary code; replace that
sourcing with a safe key/value parser that reads the .env file and exports only
well-formed VAR=VALUE lines (ignore/skip blank lines, comments, and lines with
shell metacharacters) instead of executing them; update the block that currently
contains set -a / source "$REPO_ROOT/.env" / set +a to call or inline a parser
(e.g., a function like parse_env_file) that validates lines and exports
variables securely.
In @.claude/scripts/update_polarion_tc.py:
- Around line 203-233: fix_metadata_fields currently builds defaults and passes
them to update_test_case_fields unconditionally, which can overwrite existing
non-empty metadata; instead, fetch the test case's current metadata via the
PolarionClient (e.g., use client.get_test_case or equivalent to retrieve current
fields for tc_id/project), then only include a field in the updates dict if the
existing value is empty/blank and a default is provided (leave version and
trello_jira untouched unless explicitly passed), and finally call
update_test_case_fields(client, tc_id, project, **updates) with that filtered
updates dict so non-empty fields are not overwritten by defaults.
In @.claude/scripts/validate_tc.py:
- Around line 152-155: The strict flag isn't causing warnings to be treated as
errors because warnings are still appended to self.warnings and the exit code
only checks self.errors; update the logic so that when self.strict is true
warnings are escalated: either append to self.errors instead of self.warnings in
the places that currently do "if self.strict: self.errors.append(msg) else:
self.warnings.append(msg)" or, at program exit where return/exit code is
computed, compute effective_errors = len(self.errors) + (len(self.warnings) if
self.strict else 0) and use effective_errors to decide non-zero exit; refer to
self.strict, self.errors, self.warnings and the exit/return code block currently
around the lines that check errors for non-zero exit.
- Around line 44-45: The validator in .claude/scripts/validate_tc.py currently
allows heredoc-style snippets like "cat <<EOF" (seen in the diff r'cat <<EOF')
even though workflow rules forbid heredocs; update the validation logic (in the
function that classifies command-like content—e.g., the validator function
handling command patterns / is_command_like or validate_test_case content) to
explicitly detect and reject heredoc tokens such as '<<' or '<<EOF' anywhere in
command strings and raise a validation error; ensure the same check is applied
to all places where command-like blocks are validated (the code paths covering
the ranges noted: ~lines 123-156 and 221-230) so heredoc usage is blocked and
reported as invalid.
---
Minor comments:
In @.claude/commands/mco-create-tc-from-jira.md:
- Around line 10-12: The unlabeled code fence containing the slash-command
snippet "/mco-create-tc-from-jira <JIRA-URL>" and the heredoc replacement blocks
should have language tags added to satisfy MD040: add "text" to the single-line
slash-command fence and add "text" or "bash" (as appropriate) to the heredoc
replacement code fences referenced around the heredoc blocks (also update the
similar fences at lines indicated for the other heredoc snippets, e.g., the
blocks around lines 80-86) so each triple-backtick fence includes an explicit
language tag.
In @.claude/commands/mco-create-tc-from-pr.md:
- Around line 10-12: The markdown fenced code blocks containing the command
examples (e.g., the snippet showing `/mco-create-tc-from-pr <PR-URL>`) need
explicit language identifiers to satisfy MD040; update those fences to use an
appropriate language tag such as "bash" or "text" (for example change ``` to
```bash or ```text) for the snippet in this file and the other similar fenced
blocks referenced around the second occurrence (the block covering lines 78-84)
so all unlabeled code fences are consistently labeled.
In @.claude/commands/mco-fetch.md:
- Around line 13-17: The fenced code block showing the example commands (the
lines starting with "/mco-fetch tc OCP-88122", "/mco-fetch pr 5691
[comment-id]", "/mco-fetch jira OCPBUGS-74223") is missing a language tag;
update the opening triple-backtick fence to include a language (e.g., ```bash or
```sh) so the block is lint-clean and resolves MD040.
In @.claude/README.md:
- Around line 165-170: README references to .claude/FETCH_USAGE.md,
.claude/CREATE_TC_USAGE.md, .claude/UPDATE_TC_USAGE.md,
.claude/COMPLETE_WORKFLOW.md, and .claude/FINAL_SUMMARY.md are broken; either
add those missing markdown files with the appropriate quickstart content or
update the links in .claude/README.md to point to the actual existing docs.
Locate the QUICKSTART block in .claude/README.md and for each referenced
filename either (A) create a corresponding file with the expected
usage/test-step content and a brief header, or (B) replace the link target with
the correct existing file names (or inline the content into the README) so the
references resolve; ensure the link text and file names match exactly.
- Line 248: The README references non-existent artifacts named "wrong.pdf" and
"OCP-88122.pdf" in .claude/README.md; either update those references to point to
the actual locations (e.g., correct relative paths, external URLs, or an
artifacts directory), or add the two PDF files to the repo/PR and update the
README to reference their repo paths; ensure the references to "wrong.pdf" and
"OCP-88122.pdf" in .claude/README.md are accurate and accessible.
In @.claude/scripts/create_tc_hybrid.py:
- Around line 163-165: The validation branch currently prints only
validate_result.stderr so the useful report emitted by validate_tc.py (on
stdout) is lost; update the failing branch that calls validate_result to print
validate_result.stdout (or both validate_result.stdout and
validate_result.stderr) to stderr before exiting so users see the full
validation report—locate the print statements around validate_result in
create_tc_hybrid.py and replace or augment printing validate_result.stderr with
printing validate_result.stdout (and preserve stderr output as well).
In @.claude/scripts/setup.sh:
- Around line 102-105: Update the printed next steps so the path points to the
new script location: replace the echo that prints "./scripts/setup.sh --verify"
with ".claude/scripts/setup.sh --verify" (and update the matching echo on the
other occurrence) so the "⚠️ NEXT STEPS:" message reflects the actual script
path; locate the echo statements that output the run command string and change
the path text accordingly.
In @.claude/scripts/validate_tc.py:
- Around line 242-244: The write to filepath =
'/tmp/test-specs/sample_draft.json' can fail if the parent directory doesn't
exist; before opening the file, ensure the parent directory exists by creating
os.path.dirname(filepath) with os.makedirs(..., exist_ok=True) (and add an
import for os if missing), then proceed to open and json.dump(sample, ...).
---
Nitpick comments:
In @.claude/skills/mco-automate-test-workflow.md:
- Around line 33-36: Clarify the "Validate" step by explicitly stating whether
automated validation should run the existing script or remain manual: if
automated, add a line invoking python3 .claude/scripts/validate_tc.py against
the generated spec (e.g., `python3 .claude/scripts/validate_tc.py
/tmp/test-specs/<id>.txt`) and change "Ensure test steps have commands (not just
narrative)" to "Verify test steps have commands (not just narrative)"; if
manual, add a note that reviewers must perform this check and reference the
create-tc workflow as an example. Include the script name validate_tc.py and the
create-tc workflow reference to make the expected action unambiguous.
In @.claude/skills/mco-create-tc-workflow.md:
- Around line 171-216: The hardcoded retry limit in the deletion loop (for
attempt in range(10)) can leave orphaned steps; update the loop around
client._make_request DELETE calls (and the surrounding loop that reads result =
client._make_request("GET", f"projects/OSE/workitems/{TC_ID}/teststeps")) to use
a dynamic limit or higher cap (e.g., compute from initial remaining count or use
100) and add an explicit post-loop verification that remaining is empty—if not,
log/raise an error so callers know deletion failed; keep references to load_env,
TC_ID, and client._make_request when making changes.
- Around line 27-41: The example uses unauthenticated GitHub API calls in the
fetch function which hit low rate limits; update fetch to read a GITHUB_TOKEN
from environment and add an Authorization: token <GITHUB_TOKEN> header (and keep
Accept/User-Agent) so requests are authenticated, and handle the missing token
case by either raising a clear error or falling back with a warning; modify the
fetch function and its header construction (refer to function name fetch and the
existing header dict) so PR and reviews calls use the token.
In @.claude/TESTING.md:
- Around line 126-130: Replace the fragile sys.path hack in the TESTING.md
example with a simpler, safer test for the missing requests dependency: remove
the python -c exec one-liner and instead show a reproducible approach such as
verifying import requests fails (or instructing to use a clean virtualenv) and
then running .claude/scripts/fetch_polarion.py with a sample ticket ID (e.g.,
OCP-88122) to confirm the script emits a clear dependency error; update the
"Expected" text to say the script detects the missing dependency and prints a
helpful error message.
In @.env.example:
- Line 29: The MCO_COMPONENT environment value is a multi-word string and should
be quoted for portability; update the .env example by changing the MCO_COMPONENT
assignment to include quotes around "Machine Config Operator" so the variable
MCO_COMPONENT is defined as MCO_COMPONENT="Machine Config Operator".
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: da26e24c-e329-4d1a-a34a-47eeb715b61c
📒 Files selected for processing (25)
.claude/00_START_HERE.md.claude/JIRA_INTEGRATION.md.claude/PILOT_PR5691.md.claude/QUICKSTART.md.claude/README.md.claude/TESTING.md.claude/TEST_FETCH.md.claude/commands/automate-test.md.claude/commands/mco-create-tc-from-jira.md.claude/commands/mco-create-tc-from-pr.md.claude/commands/mco-fetch.md.claude/scripts/create_polarion_tc.py.claude/scripts/create_tc_hybrid.py.claude/scripts/fetch_github_pr.py.claude/scripts/fetch_jira.py.claude/scripts/fetch_polarion.py.claude/scripts/polarion_client.py.claude/scripts/setup.sh.claude/scripts/update_polarion_tc.py.claude/scripts/validate_tc.py.claude/skills/mco-automate-test-workflow.md.claude/skills/mco-create-tc-workflow.md.claude/skills/mco-fetch-workflow.md.env.example.gitignore
Comment on lines
+130
to
+141
| ### Step 7: Always patch fields + steps separately | ||
|
|
||
| Even if creation reports success, always run: | ||
| ```bash | ||
| python3 .claude/scripts/update_polarion_tc.py OCP-XXXXX \ | ||
| --component "Machine Config Operator" \ | ||
| --sub-team "MCO" \ | ||
| --products "OCP" \ | ||
| --test-type "Functional" \ | ||
| --version "<VERSION>" \ | ||
| --trello-jira "<JIRA>" | ||
| ``` |
There was a problem hiding this comment.
Step 7 says “fields + steps” but command patches only fields.
The example omits --steps-file, so it does not actually perform the documented “+ steps” part. This creates an inconsistent workflow and can leave step updates unapplied.
🤖 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 @.claude/commands/mco-create-tc-from-jira.md around lines 130 - 141, The Step
7 example is inconsistent: it says "Always patch fields + steps separately" but
the shown command to run update_polarion_tc.py only patches fields; add the
steps argument to the example and clarify usage of the --steps-file option.
Update the Step 7 documentation to call the script update_polarion_tc.py with
both the field flags (e.g., --component, --sub-team, --products, --test-type,
--version, --trello-jira) and the --steps-file "<PATH_TO_STEPS>" parameter so
the command actually patches test steps as well.
Contributor
Author
There was a problem hiding this comment.
The --steps-file flag is present on line 140 of the skill file. The heading says "fields + steps" and the command does both. CodeRabbit likely misread the example. No change needed
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== .claude/commands/mco-create-tc-from-jira.md (Step 7 area) ==="
grep -n "steps-file\|steps_file\|Step 7\|patch fields\|update_polarion" .claude/commands/mco-create-tc-from-jira.md
echo ""
echo "=== .claude/skills/mco-create-tc-workflow.md (around line 140) ==="
grep -n "steps-file\|steps_file\|Step 7\|patch fields\|update_polarion" .claude/skills/mco-create-tc-workflow.md
echo ""
echo "=== Line 140 of skill file ==="
sed -n '135,150p' .claude/skills/mco-create-tc-workflow.mdLength of output: 2577
@ptalgulk01, the --steps-file flag is indeed present in the skill file (.claude/skills/mco-create-tc-workflow.md, line 153), but the review comment was specifically about .claude/commands/mco-create-tc-from-jira.md — and in that file, Step 7 does not include --steps-file:
.claude/commands/mco-create-tc-from-jira.md (the file under review):
python3 .claude/scripts/update_polarion_tc.py OCP-XXXXX \
--component "Machine Config Operator" \
--sub-team "MCO" \
--products "OCP" \
--test-type "Functional" \
--version "<VERSION>" \
--trello-jira "<JIRA>"
# no --steps-file here.claude/skills/mco-create-tc-workflow.md (different file):
python3 .claude/scripts/update_polarion_tc.py OCP-XXXXX \
...
--steps-file /tmp/test-specs/drafts/pr-<number>.json # ← present hereThe command file and skill file are inconsistent with each other. The heading in mco-create-tc-from-jira.md says "Always patch fields + steps separately", but the example command only patches fields. Adding --steps-file /tmp/test-specs/drafts/jira-<ISSUE-KEY>.json to that example would make it consistent with both the heading and the skill file.
Comment on lines
+203
to
+233
| def fix_metadata_fields( | ||
| client: PolarionClient, | ||
| tc_id: str, | ||
| project: str, | ||
| component: Optional[str] = None, | ||
| sub_team: Optional[str] = None, | ||
| products: Optional[str] = None, | ||
| test_type: Optional[str] = None, | ||
| version: Optional[str] = None, | ||
| trello_jira: Optional[str] = None | ||
| ) -> Dict[str, Any]: | ||
| """ | ||
| Fix empty metadata fields (like wrong.pdf - OCP-88941). | ||
|
|
||
| Use case: Batch update all required fields at once. | ||
| """ | ||
|
|
||
| # Default values for MCO TCs | ||
| defaults = { | ||
| 'component': component or 'Machine Config Operator', | ||
| 'sub_team': sub_team or 'MCO', | ||
| 'products': products or 'OCP', | ||
| 'test_type': test_type or 'Functional', | ||
| 'version': version, # No default - should be specified | ||
| 'trello_jira': trello_jira, # No default - should be specified | ||
| } | ||
|
|
||
| # Filter out None values | ||
| updates = {k: v for k, v in defaults.items() if v is not None} | ||
|
|
||
| return update_test_case_fields(client, tc_id, project, **updates) |
There was a problem hiding this comment.
--fix-metadata can overwrite existing non-empty fields instead of only fixing empties.
The function name/doc says “fix empty metadata fields,” but it unconditionally sends defaults for component/sub-team/products/test_type when args are omitted. This can silently replace valid existing 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 @.claude/scripts/update_polarion_tc.py around lines 203 - 233,
fix_metadata_fields currently builds defaults and passes them to
update_test_case_fields unconditionally, which can overwrite existing non-empty
metadata; instead, fetch the test case's current metadata via the PolarionClient
(e.g., use client.get_test_case or equivalent to retrieve current fields for
tc_id/project), then only include a field in the updates dict if the existing
value is empty/blank and a default is provided (leave version and trello_jira
untouched unless explicitly passed), and finally call
update_test_case_fields(client, tc_id, project, **updates) with that filtered
updates dict so non-empty fields are not overwritten by defaults.
Adds Claude Code commands, skills, and Python scripts to automate MCO QE workflows for creating and fetching Polarion test cases. Commands: - /mco-fetch: fetch Polarion TCs, GitHub PR comments, Jira issues - /mco-create-tc-from-pr: create TC from GitHub PR QE comment/review - /mco-create-tc-from-jira: create TC from Jira issue/comment - /automate-test: generate Go e2e test from Polarion spec (updated) Scripts (.claude/scripts/): - fetch_polarion.py, fetch_github_pr.py, fetch_jira.py - create_polarion_tc.py, create_tc_hybrid.py - update_polarion_tc.py (called internally; /mco-update-tc is future scope) - validate_tc.py, polarion_client.py, setup.sh Key rules enforced: - Validate → confirm → create (never create without user confirmation) - Always patch fields + steps via update_polarion_tc.py after creation (creation PATCH fails silently; fields may need manual Polarion fix) - Verbatim code blocks; no heredoc in expected results - STOP if parser returns 0 steps; never invent steps Auth: .env at repo root (gitignored); .env.example committed. Jira: MCO-2220 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ptalgulk01
force-pushed
the
claude-prompt
branch
from
62da39b to
d763828
Compare
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ptalgulk01 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 |
openshift-ci
Bot
added
the
approved
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (3)
.claude/commands/mco-create-tc-from-jira.md (1)
130-141:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStep 7: Missing
--steps-fileparameter (inconsistent with PR command and skill file).The heading says "Always patch fields + steps separately," but the example command only patches fields. The
--steps-fileparameter is present in.claude/commands/mco-create-tc-from-pr.md(Step 7, line 148) and.claude/skills/mco-create-tc-workflow.md(Step 7, line 153), but missing here.Add the parameter to align all three:
python3 .claude/scripts/update_polarion_tc.py OCP-XXXXX \ --component "Machine Config Operator" \ --sub-team "MCO" \ --products "OCP" \ --test-type "Functional" \ --version "<VERSION>" \ --trello-jira "<JIRA>" \ --steps-file /tmp/test-specs/drafts/jira-<ISSUE-KEY>.json🤖 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 @.claude/commands/mco-create-tc-from-jira.md around lines 130 - 141, The Step 7 bash command example for the update_polarion_tc.py script is missing the `--steps-file` parameter, which is required to patch both fields and steps as indicated by the section heading "Always patch fields + steps separately". Add the `--steps-file /tmp/test-specs/drafts/jira-<ISSUE-KEY>.json` parameter to the end of the python3 update_polarion_tc.py command to align with the same command in the mco-create-tc-from-pr.md and mco-create-tc-workflow.md files..claude/scripts/update_polarion_tc.py (1)
286-287:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
--fix-metadatahonest about overwriting existing values.The help says it fixes empty fields, but the implementation sends default values for component/sub-team/products/test type and overwrites whatever Polarion currently has. Either implement empty-only behavior or rename/reword the flag as an overwrite operation.
At minimum, align the CLI help with current behavior
parser.add_argument('--fix-metadata', action='store_true', - help='Fix all empty metadata fields with defaults') + help='Overwrite metadata fields with MCO defaults and provided overrides')Also applies to: 337-357
🤖 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 @.claude/scripts/update_polarion_tc.py around lines 286 - 287, The help text for the `--fix-metadata` flag in the argument parser definition states "Fix all empty metadata fields with defaults", but the implementation at lines 337-357 overwrites all metadata values regardless of whether they are empty or not. Either modify the implementation to check if fields are empty before applying defaults, or update the help text to accurately reflect that the flag overwrites existing metadata values with defaults. At minimum, align the CLI help message with the actual current behavior of the flag..claude/scripts/create_polarion_tc.py (1)
45-51:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHonor strict warnings when validating before creation.
TCValidator(strict=True).validate()still returnsTruefor warnings such as “No test steps defined”; strict failure is only reflected byget_exit_code(). This can create a TC with zero steps, contrary to the workflow rule to stop when parsing returns 0 steps.Propagate strict warning failures
def validate_draft(draft: Dict) -> bool: """Run canonical validation from validate_tc.py""" from validate_tc import TCValidator validator = TCValidator(strict=True) is_valid = validator.validate(draft) validator.print_report() - return is_valid + return is_valid and validator.get_exit_code() == 0🤖 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 @.claude/scripts/create_polarion_tc.py around lines 45 - 51, The validate_draft function relies solely on the boolean return value of TCValidator.validate(), but in strict mode this doesn't capture warnings such as "No test steps defined" - those strict validation failures are only reflected by calling get_exit_code() on the validator instance. Modify validate_draft to check not only the return value of validator.validate() but also the exit code from validator.get_exit_code(), returning False if the exit code is non-zero to properly honor strict validation failures and prevent test case creation with zero steps.
🧹 Nitpick comments (10)
.claude/TEST_FETCH.md (1)
62-62: 💤 Low valueAdd language specifier to fenced code block.
Line 62 has a code fence without a language identifier. Add
bashortext.Plus commands: -``` +```bash - `oc get mcp rhel9 -ojsonpath='{.status.osImageStream.name}'`🤖 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 @.claude/TEST_FETCH.md at line 62, The fenced code block on line 62 of .claude/TEST_FETCH.md is missing a language specifier. Add `bash` as the language identifier immediately after the opening triple backticks (```bash) to properly format the code fence in markdown and enable syntax highlighting.Source: Linters/SAST tools
.claude/PILOT_PR5691.md (2)
82-82: 💤 Low valueAdd language specifier to fenced code block.
Line 82 has an opening fence without a language identifier. Add
textorbashas appropriate.-**Expected**: -``` +**Expected**: +```text Validating draft...🤖 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 @.claude/PILOT_PR5691.md at line 82, In the file `.claude/PILOT_PR5691.md`, locate the fenced code block that contains the line "Validating draft..." and add a language specifier to the opening fence. Since this is output text rather than executable code, change the opening ``` to ```text to properly mark the code block with its language identifier.Source: Linters/SAST tools
119-119: 💤 Low valueAdd language specifier to fenced code block.
Line 119 lacks a language specifier on the opening fence. Add
textorbash.-**Expected output**: -``` +**Expected output**: +```text ================================================================================🤖 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 @.claude/PILOT_PR5691.md at line 119, The fenced code block opening fence on line 119 is missing a language specifier. Add `text` or `bash` immediately after the opening triple backticks (```) to properly specify the code block language. The code block contains the expected output with the equals signs separator, so adding the language identifier will ensure proper markdown formatting and syntax highlighting.Source: Linters/SAST tools
.env.example (1)
29-29: 💤 Low valueQuote configuration values for consistency.
Line 29 has an unquoted value. Dotenv convention is to quote values, especially when they contain spaces:
-MCO_COMPONENT=Machine Config Operator +MCO_COMPONENT="Machine Config Operator"🤖 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 @.env.example at line 29, The MCO_COMPONENT configuration value is unquoted in the .env.example file. Following dotenv convention, especially for values containing spaces, wrap the value "Machine Config Operator" in quotes (either single or double quotes) on the MCO_COMPONENT line to ensure consistent formatting and proper parsing by dotenv tools.Source: Linters/SAST tools
.claude/QUICKSTART.md (1)
44-44: 💤 Low valueAdd language specifier to fenced code block.
Line 44 has a code fence without a language identifier. Add
textto denote output.Should see: -``` +```text ✓ Python 3.14.5 found🤖 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 @.claude/QUICKSTART.md at line 44, The code fence on line 44 in .claude/QUICKSTART.md is missing a language specifier. Add the language identifier `text` to the opening code fence to properly denote the output content. Change the opening ``` to ```text on the line before the "✓ Python 3.14.5 found" output.Source: Linters/SAST tools
.claude/JIRA_INTEGRATION.md (4)
224-224: 💤 Low valueAdd language specifier to fenced code block.
Line 224 has an incomplete code fence. Complete the opening fence with a language identifier.
-``` +``` GET https://redhat.atlassian.net/rest/api/3/issue/{issue-key} +```🤖 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 @.claude/JIRA_INTEGRATION.md at line 224, The opening fenced code block on line 224 is missing a language specifier after the three backticks. Add an appropriate language identifier (such as bash, curl, or http) to the opening fence of the code block that contains the GET request to redhat.atlassian.net. This will properly format the code block and enable syntax highlighting for the HTTP request example.Source: Linters/SAST tools
216-216: 💤 Low valueAdd language specifier to fenced code block.
Line 216 lacks a language specifier for the code fence.
-Authorization: Bearer <JIRA_TOKEN> +``` Authorization: Bearer <JIRA_TOKEN> +```🤖 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 @.claude/JIRA_INTEGRATION.md at line 216, The code fence on line 216 is missing a language specifier. Locate the opening code fence (```) that precedes the Authorization header content and add an appropriate language identifier to it. Based on the content showing an Authorization header with Bearer token syntax, use a language specifier like bash, sh, or http to properly syntax-highlight the code block.Source: Linters/SAST tools
189-189: 💤 Low valueAdd language specifier to fenced code block.
Line 189 has a code fence without a language identifier. Add
markdownor appropriate language for clarity.-``` +```markdown🤖 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 @.claude/JIRA_INTEGRATION.md at line 189, The fenced code block at line 189 is missing a language specifier on its opening fence. Add the appropriate language identifier (such as markdown) immediately after the opening triple backticks to enable proper syntax highlighting and improve code clarity in the documentation.Source: Linters/SAST tools
50-50: 💤 Low valueFix broken link reference in markdown table.
Line 50 references
keyin the table header row butkeyis not defined as a reference label in this document. The row uses bare text, not a reference link. If intending a reference, define[key]: <url>elsewhere; otherwise, remove the reference-link brackets.🤖 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 @.claude/JIRA_INTEGRATION.md at line 50, The markdown table at line 50 contains `[MCO][KEY]` which appears to be reference-style link syntax, but there is no corresponding reference definition `[KEY]: <url>` in the document. Since this text is meant to be a literal example of a prefix format, either remove the brackets to display it as plain text `title (with MCO KEY prefix)`, or wrap it in backticks to display it as code ``title (with `[MCO][KEY]` prefix)``. If the reference link was intentional, define `[KEY]: <url>` elsewhere in the document.Source: Linters/SAST tools
.claude/README.md (1)
72-72: 💤 Low valueAdd language specifier to fenced code blocks.
Lines 72, 309, 491, 496, 506, 512, and 600 have code fences without language identifiers. Standardize by adding appropriate language tags.
Examples:
Expected output: -``` +```text ✓ Python 3.14.5 foundAnd:
Step: Check the osstream -``` +```text $ oc get mcp rhel9 -ojsonpath='{.status.osImageStream.name}'🤖 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 @.claude/README.md at line 72, Add appropriate language specifiers to all fenced code blocks in the README.md file that currently lack them. Review each code block (particularly those demonstrating output examples and command-line interfaces) and add the relevant language identifier after the opening fence. For output examples and shell commands, use the "text" language specifier. This standardizes the markdown formatting and improves readability across the documentation.Source: Linters/SAST tools
🤖 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 @.claude/PILOT_PR5691.md:
- Around line 220-230: The documentation file `.claude/PILOT_PR5691.md` (lines
220-230) references two PDF files as format comparison standards: OCP-88122.pdf
(good format example) and wrong.pdf (bad format example), but these files do not
exist in the repository root. Locate or create these two PDF files and commit
them to the repo root so that the referenced comparison examples are available
for developers to review the expected PDF metadata structure and format
requirements outlined in the documentation.
In @.claude/README.md:
- Line 25: Update the example verification output to use a realistic Python
version that is consistent with the documented Python 3.9+ requirement. Replace
the unrealistic Python version 3.14.5 shown in the example output with a
realistic version such as 3.12.1 or 3.13.x to match the documented requirement
and provide users with an accurate reference example.
In @.claude/scripts/create_polarion_tc.py:
- Around line 70-72: HTML content from external sources like PR comments and
Jira descriptions is not being properly escaped before being sent as HTML to
Polarion. In the create_polarion_tc.py file at lines 70-72 where the description
field is built and at lines 151-158 where steps are processed, escape the text
content using Python's html.escape() function first to convert any HTML entities
and special characters to their safe escaped equivalents, then apply the
newline-to-break replacement. This ensures that raw HTML from the draft
descriptions and steps cannot be interpreted as actual HTML tags when sent to
Polarion.
- Around line 233-255: Add a confirmation gate to prevent accidental creation of
Polarion test cases. First, add a new command-line argument (--yes or
--confirmed) to the argument parser. Then, before the actual create_test_case
function call, check if this flag is set—if not set, prompt the user to confirm
the action or exit without creating. This ensures the validate → confirm →
create workflow is enforced while still allowing automation to pass the flag
explicitly to skip the interactive prompt.
In @.claude/scripts/create_tc_hybrid.py:
- Around line 162-165: The validation failure handler in the validate_tc.py
execution block currently only prints stderr, but the actual validation report
is written to stdout, so users cannot see the detailed errors they need to fix.
Modify the error handling when validate_result.returncode is not zero to print
both validate_result.stdout and validate_result.stderr to sys.stderr, ensuring
the validation report is visible to users for debugging.
- Around line 134-140: Add a validation check after printing the test steps
count to fail fast when the PR parser returns zero test steps. After the print
statement that shows the test steps count from pr_draft.get('test_steps', []),
verify that the length is greater than zero. If the test steps count is zero,
exit the script or raise an exception before proceeding to the
merge_jira_pr_drafts call, ensuring the workflow stops immediately instead of
continuing to merge, validate, and create.
- Around line 143-148: The code around the draft file creation is vulnerable to
path traversal attacks because args.jira_key is used unsanitized in the file
path construction. Validate args.jira_key to reject any values containing path
traversal characters like forward slashes, double dots, or absolute path
prefixes. Additionally, replace the hardcoded predictable /tmp/test-specs/drafts
directory with a secure unique temporary directory created using
tempfile.mkdtemp() to prevent file writing to unexpected locations. These
changes should be applied where output_dir is defined and draft_file is
constructed using the jira_key argument.
In @.claude/scripts/fetch_github_pr.py:
- Around line 153-175: Replace the regex-based section and bullet parsing logic
to be fence-aware, preventing fenced code blocks from corrupting the steps
extraction. Instead of using regex patterns like `\n[A-Z][a-z]+:` and `^[-*]\s+`
that match inside code blocks, implement line-by-line parsing that tracks fence
state (detecting triple backticks to mark when entering or exiting code blocks).
Only apply section boundary detection and bullet parsing when not inside a
fence. Accumulate lines into the current bullet item and only when a new bullet
is detected at the top level (outside fences) should you finalize and append the
previous bullet to the bullets list. This prevents YAML fields like
`apiVersion:` or `kind:` and list markers like `- op:` inside code blocks from
prematurely terminating or incorrectly splitting the steps content.
In @.claude/scripts/fetch_polarion.py:
- Around line 328-334: Replace hardcoded `/tmp` paths with a private per-user
directory across three files. Create a helper function
private_output_dir(*parts: str) that uses $XDG_RUNTIME_DIR/mco-qe as the base
(or ~/.cache/mco-qe as fallback) and creates directories with mode 0o700. In
.claude/scripts/fetch_polarion.py lines 328-334, replace `output_dir =
'/tmp/test-specs'` with `output_dir = private_output_dir('test-specs')`. In
.claude/scripts/fetch_github_pr.py lines 333-338, replace the hardcoded `/tmp`
path for the drafts/pr-<number>.json file with a call to
`private_output_dir('drafts')`. In .claude/scripts/fetch_jira.py lines 241-247,
replace the hardcoded `/tmp` path for the jira/<issue-key>.json file with a call
to `private_output_dir('jira')`. Define or import the private_output_dir helper
in all three files to ensure consistent secure directory creation with
restrictive permissions.
- Around line 29-32: In the fetch_polarion.py file, the error message when the
.env file is not found incorrectly directs users to run ./scripts/setup.sh.
Update the print statement that outputs "Run: ./scripts/setup.sh" to reference
the correct location of the setup script as it exists in the committed PR, so
users following the remediation will run the correct command.
In @.claude/scripts/update_polarion_tc.py:
- Around line 185-198: The code is bypassing canonical validation checks when
updating test steps in an existing test case. Before calling
client.add_test_steps(), add validation to check the steps for empty expected
results, heredocs, and narrative-only content. Reuse TCValidator to validate the
steps list, or promote the private method _validate_test_steps() from
validate_tc.py to a public validate_steps() helper function and call that helper
on the steps before passing them to client.add_test_steps(). Ensure validation
errors are properly handled and returned if validation fails.
In @.claude/scripts/validate_tc.py:
- Around line 249-251: Replace the hardcoded filepath variable that points to
'/tmp/test-specs/sample_draft.json' with a secure temporary file created using
Python's tempfile module. Instead of directly opening the fixed path, use
tempfile.NamedTemporaryFile or tempfile.TemporaryDirectory to create a
user-owned unique temporary location, and write the JSON dump to that secure
temporary file. This eliminates the security risk of predictable paths that
could be pre-created or symlinked to overwrite unintended files. Add the
necessary import statement for tempfile at the top of the file if it is not
already present.
---
Duplicate comments:
In @.claude/commands/mco-create-tc-from-jira.md:
- Around line 130-141: The Step 7 bash command example for the
update_polarion_tc.py script is missing the `--steps-file` parameter, which is
required to patch both fields and steps as indicated by the section heading
"Always patch fields + steps separately". Add the `--steps-file
/tmp/test-specs/drafts/jira-<ISSUE-KEY>.json` parameter to the end of the
python3 update_polarion_tc.py command to align with the same command in the
mco-create-tc-from-pr.md and mco-create-tc-workflow.md files.
In @.claude/scripts/create_polarion_tc.py:
- Around line 45-51: The validate_draft function relies solely on the boolean
return value of TCValidator.validate(), but in strict mode this doesn't capture
warnings such as "No test steps defined" - those strict validation failures are
only reflected by calling get_exit_code() on the validator instance. Modify
validate_draft to check not only the return value of validator.validate() but
also the exit code from validator.get_exit_code(), returning False if the exit
code is non-zero to properly honor strict validation failures and prevent test
case creation with zero steps.
In @.claude/scripts/update_polarion_tc.py:
- Around line 286-287: The help text for the `--fix-metadata` flag in the
argument parser definition states "Fix all empty metadata fields with defaults",
but the implementation at lines 337-357 overwrites all metadata values
regardless of whether they are empty or not. Either modify the implementation to
check if fields are empty before applying defaults, or update the help text to
accurately reflect that the flag overwrites existing metadata values with
defaults. At minimum, align the CLI help message with the actual current
behavior of the flag.
---
Nitpick comments:
In @.claude/JIRA_INTEGRATION.md:
- Line 224: The opening fenced code block on line 224 is missing a language
specifier after the three backticks. Add an appropriate language identifier
(such as bash, curl, or http) to the opening fence of the code block that
contains the GET request to redhat.atlassian.net. This will properly format the
code block and enable syntax highlighting for the HTTP request example.
- Line 216: The code fence on line 216 is missing a language specifier. Locate
the opening code fence (```) that precedes the Authorization header content and
add an appropriate language identifier to it. Based on the content showing an
Authorization header with Bearer token syntax, use a language specifier like
bash, sh, or http to properly syntax-highlight the code block.
- Line 189: The fenced code block at line 189 is missing a language specifier on
its opening fence. Add the appropriate language identifier (such as markdown)
immediately after the opening triple backticks to enable proper syntax
highlighting and improve code clarity in the documentation.
- Line 50: The markdown table at line 50 contains `[MCO][KEY]` which appears to
be reference-style link syntax, but there is no corresponding reference
definition `[KEY]: <url>` in the document. Since this text is meant to be a
literal example of a prefix format, either remove the brackets to display it as
plain text `title (with MCO KEY prefix)`, or wrap it in backticks to display it
as code ``title (with `[MCO][KEY]` prefix)``. If the reference link was
intentional, define `[KEY]: <url>` elsewhere in the document.
In @.claude/PILOT_PR5691.md:
- Line 82: In the file `.claude/PILOT_PR5691.md`, locate the fenced code block
that contains the line "Validating draft..." and add a language specifier to the
opening fence. Since this is output text rather than executable code, change the
opening ``` to ```text to properly mark the code block with its language
identifier.
- Line 119: The fenced code block opening fence on line 119 is missing a
language specifier. Add `text` or `bash` immediately after the opening triple
backticks (```) to properly specify the code block language. The code block
contains the expected output with the equals signs separator, so adding the
language identifier will ensure proper markdown formatting and syntax
highlighting.
In @.claude/QUICKSTART.md:
- Line 44: The code fence on line 44 in .claude/QUICKSTART.md is missing a
language specifier. Add the language identifier `text` to the opening code fence
to properly denote the output content. Change the opening ``` to ```text on the
line before the "✓ Python 3.14.5 found" output.
In @.claude/README.md:
- Line 72: Add appropriate language specifiers to all fenced code blocks in the
README.md file that currently lack them. Review each code block (particularly
those demonstrating output examples and command-line interfaces) and add the
relevant language identifier after the opening fence. For output examples and
shell commands, use the "text" language specifier. This standardizes the
markdown formatting and improves readability across the documentation.
In @.claude/TEST_FETCH.md:
- Line 62: The fenced code block on line 62 of .claude/TEST_FETCH.md is missing
a language specifier. Add `bash` as the language identifier immediately after
the opening triple backticks (```bash) to properly format the code fence in
markdown and enable syntax highlighting.
In @.env.example:
- Line 29: The MCO_COMPONENT configuration value is unquoted in the .env.example
file. Following dotenv convention, especially for values containing spaces, wrap
the value "Machine Config Operator" in quotes (either single or double quotes)
on the MCO_COMPONENT line to ensure consistent formatting and proper parsing by
dotenv tools.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2466560e-02ed-4b5c-8696-143155813a79
📒 Files selected for processing (25)
.claude/00_START_HERE.md.claude/JIRA_INTEGRATION.md.claude/PILOT_PR5691.md.claude/QUICKSTART.md.claude/README.md.claude/TESTING.md.claude/TEST_FETCH.md.claude/commands/automate-test.md.claude/commands/mco-create-tc-from-jira.md.claude/commands/mco-create-tc-from-pr.md.claude/commands/mco-fetch.md.claude/scripts/create_polarion_tc.py.claude/scripts/create_tc_hybrid.py.claude/scripts/fetch_github_pr.py.claude/scripts/fetch_jira.py.claude/scripts/fetch_polarion.py.claude/scripts/polarion_client.py.claude/scripts/setup.sh.claude/scripts/update_polarion_tc.py.claude/scripts/validate_tc.py.claude/skills/mco-automate-test-workflow.md.claude/skills/mco-create-tc-workflow.md.claude/skills/mco-fetch-workflow.md.env.example.gitignore
✅ Files skipped from review due to trivial changes (4)
- .claude/00_START_HERE.md
- .claude/skills/mco-automate-test-workflow.md
- .claude/commands/automate-test.md
- .claude/TESTING.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- .claude/scripts/setup.sh
Comment on lines
+220
to
+230
| | Field | wrong.pdf (bad) | PR 5691 (expected good) | | ||
| |---|---|---| | ||
| | Title prefix | ❌ Missing | ✓ [MCO][JIRA-ID] | | ||
| | Component | ❌ Empty | ✓ Machine Config Operator | | ||
| | Sub Team | ❌ Empty | ✓ MCO | | ||
| | Products | ❌ Empty | ✓ OCP | | ||
| | Test Type | ❌ Empty | ✓ Functional | | ||
| | Version | ❌ Empty | ✓ 4.22 | | ||
| | Trello/Jira | ❌ Empty | ✓ OCPBUGS-xxxxx | | ||
| | Expected Results | ❌ Narrative | ✓ Commands | | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify PDF reference files exist in repo root
echo "Checking for reference PDF files..."
test -f "OCP-88122.pdf" && echo "✓ OCP-88122.pdf found" || echo "✗ OCP-88122.pdf missing"
test -f "wrong.pdf" && echo "✓ wrong.pdf found" || echo "✗ wrong.pdf missing"
# Optionally list all .pdf files in repo root
echo ""
echo "=== All PDF files in repo root ==="
find . -maxdepth 1 -name "*.pdf" -type f | sortRepository: openshift/machine-config-operator
Length of output: 193
Commit the missing reference PDF files to the repo root.
The documentation in .claude/PILOT_PR5691.md (lines 220-230) references two PDF files as comparison standards, but neither exists in the repository:
OCP-88122.pdf(good format example)wrong.pdf(bad format example, OCP-88941)
Add these files to the repo root to complete the PR.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 222-222: Reference links and images should use a label that is defined
Missing link or image reference definition: "jira-id"
(MD052, reference-links-images)
🤖 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 @.claude/PILOT_PR5691.md around lines 220 - 230, The documentation file
`.claude/PILOT_PR5691.md` (lines 220-230) references two PDF files as format
comparison standards: OCP-88122.pdf (good format example) and wrong.pdf (bad
format example), but these files do not exist in the repository root. Locate or
create these two PDF files and commit them to the repo root so that the
referenced comparison examples are available for developers to review the
expected PDF metadata structure and format requirements outlined in the
documentation.
|
|
||
| ### Prerequisites | ||
|
|
||
| - Python 3.9+ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Python version requirement in scripts
echo "=== Checking Python version requirements in code ==="
# Look for version checks in setup.sh
echo "setup.sh version check:"
rg "python3\s+--version|sys\.version_info|3\.[0-9]+" ".claude/scripts/setup.sh" | head -5
# Check if any scripts use Python 3.10+ features (match, structural patterns)
echo ""
echo "Searching for modern Python syntax (3.10+ features):"
rg "match\s+\(|case\s+\(" ".claude/scripts/" | head -5 || echo " (none found)"
# List Python version requirement if present in any config
echo ""
echo "Python version in any pyproject.toml or setup.py:"
rg "python_requires|requires-python" . --glob "*.toml" --glob "*.py" 2>/dev/null | head -3 || echo " (none found)"Repository: openshift/machine-config-operator
Length of output: 512
🏁 Script executed:
cat -n .claude/README.mdRepository: openshift/machine-config-operator
Length of output: 20841
Update the example Python version in the verification output to reflect the documented requirement.
Line 73 shows an unrealistic Python version (3.14.5). Since the documented requirement is Python 3.9+, update the example output to show a version that matches this range and is realistic (e.g., Python 3.12.x or 3.13.x):
Example fix
Expected output:-✓ Python 3.14.5 found
+✓ Python 3.12.1 found
✓ requests already installed
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/README.md at line 25, Update the example verification output to use
a realistic Python version that is consistent with the documented Python 3.9+
requirement. Replace the unrealistic Python version 3.14.5 shown in the example
output with a realistic version such as 3.12.1 or 3.13.x to match the documented
requirement and provide users with an accurate reference example.
</details>
<!-- fingerprinting:phantom:triton:mongoose -->
<!-- cr-comment:v1:eec9225d4e49f23780426c33 -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+70
to
+72
| "description": { | ||
| "type": "text/html", | ||
| "value": draft.get('description', '').replace("\n", "<br/>") |
There was a problem hiding this comment.
Escape GitHub/Jira-derived content before sending it as text/html.
Replacing newlines with <br/> still leaves raw HTML intact in descriptions and steps. Since these drafts can come from PR comments or Jira, escape the text first and then add line breaks. As per coding guidelines, .claude scripts are HIGH RISK and should be reviewed with a security and malware lens.
Escape text before building Polarion HTML fields
import json
+import html
import os
import sys
from typing import Dict, Any
from polarion_client import PolarionClient
@@
+def to_polarion_html(value: str) -> str:
+ """Render plain text safely as Polarion HTML."""
+ return html.escape(value or '').replace("\n", "<br/>")
+
+
def create_test_case(
@@
"description": {
"type": "text/html",
- "value": draft.get('description', '').replace("\n", "<br/>")
+ "value": to_polarion_html(draft.get('description', ''))
},
@@
{
"type": "text/html",
- "value": step.get("step", "").replace("\n", "<br/>")
+ "value": to_polarion_html(step.get("step", ""))
},
{
"type": "text/html",
- "value": step.get("expected_result", "").replace("\n", "<br/>")
+ "value": to_polarion_html(step.get("expected_result", ""))
}Also applies to: 151-158
🤖 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 @.claude/scripts/create_polarion_tc.py around lines 70 - 72, HTML content
from external sources like PR comments and Jira descriptions is not being
properly escaped before being sent as HTML to Polarion. In the
create_polarion_tc.py file at lines 70-72 where the description field is built
and at lines 151-158 where steps are processed, escape the text content using
Python's html.escape() function first to convert any HTML entities and special
characters to their safe escaped equivalents, then apply the newline-to-break
replacement. This ensures that raw HTML from the draft descriptions and steps
cannot be interpreted as actual HTML tags when sent to Polarion.
Source: Coding guidelines
Comment on lines
+233
to
+255
| if args.dry_run: | ||
| print("✓ DRY RUN: Draft is valid and ready for creation", file=sys.stderr) | ||
| sys.exit(0) | ||
|
|
||
| # Load environment | ||
| env = load_env() | ||
| polarion_url = env.get('POLARION_URL', 'https://polarion.engineering.redhat.com') | ||
| polarion_token = env.get('POLARION_TOKEN', '') | ||
|
|
||
| if not polarion_token or polarion_token == 'your_personal_access_token_here': | ||
| print("Error: POLARION_TOKEN not configured in .env", file=sys.stderr) | ||
| print("Generate token at: https://polarion.engineering.redhat.com/polarion/#/user/me", file=sys.stderr) | ||
| sys.exit(1) | ||
|
|
||
| # Create client | ||
| client = PolarionClient( | ||
| url=polarion_url, | ||
| token=polarion_token, | ||
| verify_ssl=True | ||
| ) | ||
|
|
||
| # Create test case | ||
| result = create_test_case(client, draft, args.project) |
There was a problem hiding this comment.
Require an explicit confirmation flag before creating Polarion work items.
This user-facing script creates a real TC whenever --dry-run is omitted. Add a non-interactive --yes/--confirmed flag and require the workflow to pass it only after the user confirms; this preserves automation while enforcing the validate → confirm → create rule.
Add a confirmation gate that automation can satisfy explicitly
parser.add_argument('--project', default='OSE', help='Polarion project ID (default: OSE)')
parser.add_argument('--dry-run', action='store_true', help='Validate only, do not create')
+ parser.add_argument('--yes', action='store_true',
+ help='Confirm creation after reviewing the summary')
@@
if args.dry_run:
print("✓ DRY RUN: Draft is valid and ready for creation", file=sys.stderr)
sys.exit(0)
+
+ if not args.yes:
+ print("Error: creation requires explicit confirmation. Re-run with --yes after reviewing the summary.", file=sys.stderr)
+ sys.exit(1)
# Load environment
env = load_env()🧰 Tools
🪛 Ruff (0.15.15)
[error] 242-242: Possible hardcoded password assigned to: "polarion_token"
(S105)
🤖 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 @.claude/scripts/create_polarion_tc.py around lines 233 - 255, Add a
confirmation gate to prevent accidental creation of Polarion test cases. First,
add a new command-line argument (--yes or --confirmed) to the argument parser.
Then, before the actual create_test_case function call, check if this flag is
set—if not set, prompt the user to confirm the action or exit without creating.
This ensures the validate → confirm → create workflow is enforced while still
allowing automation to pass the flag explicitly to skip the interactive prompt.
Comment on lines
+134
to
+140
| pr_draft = run_script('fetch_github_pr.py', pr_args) | ||
| print(f" ✓ Test Steps: {len(pr_draft.get('test_steps', []))}", file=sys.stderr) | ||
| print() | ||
|
|
||
| # Step 3: Merge drafts | ||
| print("[3/5] Merging Jira + PR data...", file=sys.stderr) | ||
| merged_draft = merge_jira_pr_drafts(jira_draft, pr_draft) |
There was a problem hiding this comment.
Stop when the PR parser returns zero test steps.
The workflow explicitly says to stop if the parser returns 0 steps, but this path only prints the count and continues into merge/validation/create. Fail fast here before writing or creating anything.
Fail fast on empty PR steps
pr_draft = run_script('fetch_github_pr.py', pr_args)
- print(f" ✓ Test Steps: {len(pr_draft.get('test_steps', []))}", file=sys.stderr)
+ pr_steps = pr_draft.get('test_steps', [])
+ print(f" ✓ Test Steps: {len(pr_steps)}", file=sys.stderr)
+ if not pr_steps:
+ print(" ❌ No test steps parsed from PR; refusing to continue.", file=sys.stderr)
+ sys.exit(1)
print()🤖 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 @.claude/scripts/create_tc_hybrid.py around lines 134 - 140, Add a validation
check after printing the test steps count to fail fast when the PR parser
returns zero test steps. After the print statement that shows the test steps
count from pr_draft.get('test_steps', []), verify that the length is greater
than zero. If the test steps count is zero, exit the script or raise an
exception before proceeding to the merge_jira_pr_drafts call, ensuring the
workflow stops immediately instead of continuing to merge, validate, and create.
Comment on lines
+153
to
+175
| # Find Test Steps or Testing Steps section (with or without ##) | ||
| steps_match = re.search( | ||
| r'(?:##\s*)?(?:Test(?:ing)?\s+)?Steps:\s*\n(.*?)(?=\n##|\n[A-Z][a-z]+:|\Z)', | ||
| content, | ||
| re.DOTALL | re.IGNORECASE | ||
| ) | ||
|
|
||
| if not steps_match: | ||
| # Try standard ## header format | ||
| steps_match = re.search( | ||
| r'##\s*(?:Test(?:ing)?\s+Steps|Steps)\s*\n(.*?)(?=\n##|\Z)', | ||
| content, | ||
| re.DOTALL | re.IGNORECASE | ||
| ) | ||
|
|
||
| if not steps_match: | ||
| return steps | ||
|
|
||
| steps_section = steps_match.group(1) | ||
|
|
||
| # Split by bullets | ||
| bullet_pattern = r'^[-*]\s+(.+?)(?=^[-*]\s+|\Z)' | ||
| bullets = re.findall(bullet_pattern, steps_section, re.MULTILINE | re.DOTALL) |
There was a problem hiding this comment.
Do not let fenced code terminate or split the steps parser.
The generic \n[A-Z][a-z]+: boundary runs inside fenced expected-result blocks, so YAML like apiVersion: or kind: can truncate the section before the code block is captured. The bullet splitter has the same issue with code lines such as - op:. That corrupts test_steps before the hybrid workflow validates and merges the draft.
Make section and bullet parsing fence-aware
# Find Test Steps or Testing Steps section (with or without ##)
+ section_boundary = r'(?=\n##\s+|\n(?:Environment Setup|Notes?|Results?):\s*\n|\Z)'
steps_match = re.search(
- r'(?:##\s*)?(?:Test(?:ing)?\s+)?Steps:\s*\n(.*?)(?=\n##|\n[A-Z][a-z]+:|\Z)',
+ rf'(?:##\s*)?(?:Test(?:ing)?\s+)?Steps:\s*\n(.*?){section_boundary}',
content,
re.DOTALL | re.IGNORECASE
)
if not steps_match:
# Try standard ## header format
steps_match = re.search(
- r'##\s*(?:Test(?:ing)?\s+Steps|Steps)\s*\n(.*?)(?=\n##|\Z)',
+ rf'##\s*(?:Test(?:ing)?\s+Steps|Steps)\s*\n(.*?){section_boundary}',
content,
re.DOTALL | re.IGNORECASE
)
@@
- # Split by bullets
- bullet_pattern = r'^[-*]\s+(.+?)(?=^[-*]\s+|\Z)'
- bullets = re.findall(bullet_pattern, steps_section, re.MULTILINE | re.DOTALL)
+ # Split by top-level bullets, ignoring bullet-looking lines inside fenced code.
+ bullets = []
+ current = []
+ in_fence = False
+ for line in steps_section.splitlines():
+ if line.strip().startswith('```'):
+ in_fence = not in_fence
+
+ if not in_fence and re.match(r'^[-*]\s+', line):
+ if current and any(part.strip() for part in current):
+ bullets.append('\n'.join(current).strip())
+ current = [re.sub(r'^[-*]\s+', '', line, count=1)]
+ elif current or line.strip():
+ current.append(line)
+
+ if current and any(part.strip() for part in current):
+ bullets.append('\n'.join(current).strip())🤖 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 @.claude/scripts/fetch_github_pr.py around lines 153 - 175, Replace the
regex-based section and bullet parsing logic to be fence-aware, preventing
fenced code blocks from corrupting the steps extraction. Instead of using regex
patterns like `\n[A-Z][a-z]+:` and `^[-*]\s+` that match inside code blocks,
implement line-by-line parsing that tracks fence state (detecting triple
backticks to mark when entering or exiting code blocks). Only apply section
boundary detection and bullet parsing when not inside a fence. Accumulate lines
into the current bullet item and only when a new bullet is detected at the top
level (outside fences) should you finalize and append the previous bullet to the
bullets list. This prevents YAML fields like `apiVersion:` or `kind:` and list
markers like `- op:` inside code blocks from prematurely terminating or
incorrectly splitting the steps content.
Comment on lines
+29
to
+32
| if not os.path.exists(env_file): | ||
| print(f"Error: .env not found at {env_file}", file=sys.stderr) | ||
| print("Run: ./scripts/setup.sh", file=sys.stderr) | ||
| sys.exit(1) |
There was a problem hiding this comment.
Point the missing .env remediation at the committed setup script.
./scripts/setup.sh does not match the PR’s setup script location; users following this error path will run the wrong command.
Fix the setup command
- print("Run: ./scripts/setup.sh", file=sys.stderr)
+ print("Run: .claude/scripts/setup.sh", file=sys.stderr)🤖 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 @.claude/scripts/fetch_polarion.py around lines 29 - 32, In the
fetch_polarion.py file, the error message when the .env file is not found
incorrectly directs users to run ./scripts/setup.sh. Update the print statement
that outputs "Run: ./scripts/setup.sh" to reference the correct location of the
setup script as it exists in the committed PR, so users following the
remediation will run the correct command.
Comment on lines
+328
to
+334
| output_dir = '/tmp/test-specs' | ||
| os.makedirs(output_dir, exist_ok=True) | ||
|
|
||
| txt_file = os.path.join(output_dir, f"{tc_id}.txt") | ||
| txt_content = format_for_automate_test(tc_data) | ||
|
|
||
| with open(txt_file, 'w') as f: |
There was a problem hiding this comment.
Move generated drafts/specs out of predictable shared /tmp paths.
These scripts write internal QE artifacts to stable paths under /tmp/test-specs*, which can be world-readable on shared systems and is easy to pre-create or symlink. Use a per-user private directory, create it with 0700, and keep --output for explicit overrides.
.claude/scripts/fetch_polarion.py#L328-L334: replace/tmp/test-specswith a private per-user base such as$XDG_RUNTIME_DIR/mco-qe/test-specsor~/.cache/mco-qe/test-specs, and create it with restrictive permissions..claude/scripts/fetch_github_pr.py#L333-L338: use the same private base fordrafts/pr-<number>.json..claude/scripts/fetch_jira.py#L241-L247: use the same private base forjira/<issue-key>.json.
Example helper for a private default output base
from pathlib import Path
def private_output_dir(*parts: str) -> str:
base = os.environ.get("XDG_RUNTIME_DIR")
root = Path(base) / "mco-qe" if base else Path.home() / ".cache" / "mco-qe"
output_dir = root.joinpath(*parts)
output_dir.mkdir(mode=0o700, parents=True, exist_ok=True)
os.chmod(output_dir, 0o700)
return str(output_dir)As per coding guidelines, .claude scripts are high-risk and changes granting broad filesystem access require close scrutiny.
🧰 Tools
🪛 Ruff (0.15.15)
[error] 328-328: Probable insecure usage of temporary file or directory: "/tmp/test-specs"
(S108)
📍 Affects 3 files
.claude/scripts/fetch_polarion.py#L328-L334(this comment).claude/scripts/fetch_github_pr.py#L333-L338.claude/scripts/fetch_jira.py#L241-L247
🤖 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 @.claude/scripts/fetch_polarion.py around lines 328 - 334, Replace hardcoded
`/tmp` paths with a private per-user directory across three files. Create a
helper function private_output_dir(*parts: str) that uses
$XDG_RUNTIME_DIR/mco-qe as the base (or ~/.cache/mco-qe as fallback) and creates
directories with mode 0o700. In .claude/scripts/fetch_polarion.py lines 328-334,
replace `output_dir = '/tmp/test-specs'` with `output_dir =
private_output_dir('test-specs')`. In .claude/scripts/fetch_github_pr.py lines
333-338, replace the hardcoded `/tmp` path for the drafts/pr-<number>.json file
with a call to `private_output_dir('drafts')`. In .claude/scripts/fetch_jira.py
lines 241-247, replace the hardcoded `/tmp` path for the jira/<issue-key>.json
file with a call to `private_output_dir('jira')`. Define or import the
private_output_dir helper in all three files to ensure consistent secure
directory creation with restrictive permissions.
Sources: Coding guidelines, Linters/SAST tools
Comment on lines
+185
to
+198
| # Ensure steps is a list | ||
| if isinstance(steps_data, dict): | ||
| steps = steps_data.get('test_steps', []) | ||
| elif isinstance(steps_data, list): | ||
| steps = steps_data | ||
| else: | ||
| return { | ||
| 'status': 'failed', | ||
| 'error': 'Steps file must contain array or object with test_steps key' | ||
| } | ||
|
|
||
| # Use add_test_steps from PolarionClient (handles REST vs SOAP logic) | ||
| # Note: For existing TCs, this will attempt REST first, then fall back to SOAP | ||
| result = client.add_test_steps(tc_id, steps, project, force_soap=force_soap) |
There was a problem hiding this comment.
Validate steps before updating an existing TC.
This path accepts a list or test_steps object and sends it straight to Polarion, bypassing the canonical checks for empty expected results, heredocs, and narrative-only content. Reuse TCValidator here before client.add_test_steps().
Gate step updates through the validator
elif isinstance(steps_data, list):
steps = steps_data
else:
return {
'status': 'failed',
'error': 'Steps file must contain array or object with test_steps key'
}
+
+ from validate_tc import TCValidator
+ validator = TCValidator(strict=True)
+ validator._validate_test_steps(steps)
+ if validator.errors or validator.warnings:
+ validator.print_report()
+ return {
+ 'status': 'failed',
+ 'error': 'Steps failed validation; fix the steps file before updating Polarion'
+ }
# Use add_test_steps from PolarionClient (handles REST vs SOAP logic)If you want to avoid calling a private method, promote _validate_test_steps() to a public validate_steps() helper in validate_tc.py and call that here.
🤖 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 @.claude/scripts/update_polarion_tc.py around lines 185 - 198, The code is
bypassing canonical validation checks when updating test steps in an existing
test case. Before calling client.add_test_steps(), add validation to check the
steps for empty expected results, heredocs, and narrative-only content. Reuse
TCValidator to validate the steps list, or promote the private method
_validate_test_steps() from validate_tc.py to a public validate_steps() helper
function and call that helper on the steps before passing them to
client.add_test_steps(). Ensure validation errors are properly handled and
returned if validation fails.
Comment on lines
+249
to
+251
| filepath = '/tmp/test-specs/sample_draft.json' | ||
| with open(filepath, 'w') as f: | ||
| json.dump(sample, f, indent=2) |
There was a problem hiding this comment.
Avoid writing the sample draft to a fixed /tmp path.
A predictable path under /tmp can be pre-created or symlinked and can overwrite an unintended file when this script is run locally. Use tempfile to create a user-owned unique directory/file instead. As per coding guidelines, .claude scripts are HIGH RISK and should be reviewed with a security and malware lens.
Safer sample-file creation
import json
import re
import sys
+import tempfile
from typing import Dict, List, Any, Tuple
@@
- filepath = '/tmp/test-specs/sample_draft.json'
+ output_dir = tempfile.mkdtemp(prefix='mco-tc-sample-')
+ filepath = os.path.join(output_dir, 'sample_draft.json')
with open(filepath, 'w') as f:
json.dump(sample, f, indent=2)This also requires adding import os if you keep os.path.join; alternatively use pathlib.Path(output_dir) / 'sample_draft.json'.
🧰 Tools
🪛 Ruff (0.15.15)
[error] 249-249: Probable insecure usage of temporary file or directory: "/tmp/test-specs/sample_draft.json"
(S108)
🤖 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 @.claude/scripts/validate_tc.py around lines 249 - 251, Replace the hardcoded
filepath variable that points to '/tmp/test-specs/sample_draft.json' with a
secure temporary file created using Python's tempfile module. Instead of
directly opening the fixed path, use tempfile.NamedTemporaryFile or
tempfile.TemporaryDirectory to create a user-owned unique temporary location,
and write the JSON dump to that secure temporary file. This eliminates the
security risk of predictable paths that could be pre-created or symlinked to
overwrite unintended files. Add the necessary import statement for tempfile at
the top of the file if it is not already present.
Sources: Coding guidelines, Linters/SAST tools
Contributor
|
@ptalgulk01: The following tests failed, say
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. |
ptalgulk01
changed the title
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@ptalgulk01: This pull request references MCO-2220 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds Claude Code commands, skills, and Python scripts to automate MCO QE workflows for creating and fetching Polarion test cases.
Commands:
Scripts (.claude/scripts/):
Key rules enforced:
Summary by CodeRabbit
.envtemplate, plus repository ignore rules.