OBSINTA-1287: Agentic test iteration skills for flaky test debugging - #1099
OBSINTA-1287: Agentic test iteration skills for flaky test debugging#1099DavidRajnoha wants to merge 3 commits into
Conversation
Move test-development commands into subdirectory, rename cypress-run/cypress-setup to run/setup, and add clean-test-artifacts script. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Skills for autonomous Cypress test debugging against local and OpenShift CI environments: iterate-incident-tests, iterate-ci-flaky, diagnose-test-failure, analyze-ci-results, productize-iterations, refactor-page-object, ensure-env, run-suite, and run-and-report primitives. Includes poll-ci-status.py and review-github.py support scripts. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Architecture overview of the test iteration system and roadmap with ideas for future development. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
openshift-ci-robot
added
the
jira/valid-reference
|
@DavidRajnoha: This pull request references OBSINTA-1287 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 task 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: DavidRajnoha The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughThe PR adds Cypress setup and execution commands, CI analysis and diagnosis workflows, GitHub review and polling scripts, autonomous test-iteration procedures, artifact cleanup, shared command links, and architecture and roadmap documentation. ChangesCypress iteration tooling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Coordinator
participant EnsureEnv
participant RunSuite
participant DiagnosisAgent
participant CI
participant StabilityLedger
Coordinator->>EnsureEnv: resolve and validate Cypress environment
Coordinator->>RunSuite: execute selected Cypress target
RunSuite->>Coordinator: return test results and artifacts
Coordinator->>DiagnosisAgent: diagnose failing tests
DiagnosisAgent->>Coordinator: return classifications and fixes
Coordinator->>CI: trigger or poll CI validation
CI->>Coordinator: return CI status and artifacts
Coordinator->>StabilityLedger: record run and flakiness results
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (21)
.claude/commands/cypress/setup.md-28-30 (1)
28-30: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse a controlled dependency-install policy consistently.
.claude/commands/cypress/setup.md#L28-L30: use a lockfile-controlled install and require confirmation for lifecycle scripts..claude/commands/cypress/test-iteration/ensure-env.md#L18-L20: apply the same policy to the fallback install path.Both autonomous commands currently invoke
npm install, which can fetch packages and execute lifecycle scripts.As per path instructions,
.claudechanges require supply-chain scrutiny.🤖 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/cypress/setup.md around lines 28 - 30, Replace the npm install commands in .claude/commands/cypress/setup.md lines 28-30 and .claude/commands/cypress/test-iteration/ensure-env.md lines 18-20 with a lockfile-controlled install that requires confirmation before lifecycle scripts run, applying the same policy to both primary and fallback dependency-install paths.Source: Path instructions
.claude/commands/cypress/test-iteration/run-suite.md-21-25 (1)
21-25: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winImplement the promised background execution.
The prose says Cypress should run in the background, but the command is foreground-only. Long suites can block the agent or hit command timeouts. Start a tracked process, persist its output/exit code, and wait or poll for completion.
🤖 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/cypress/test-iteration/run-suite.md around lines 21 - 25, Update the Cypress command in the test-iteration instructions to launch a tracked background process, persist its logs and exit code, and wait or poll until completion before reporting the result. Preserve the existing environment setup, spec selection, and grepTags arguments while ensuring the agent is not blocked by a foreground process..claude/commands/cypress/setup.md-37-45 (1)
37-45: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not display existing credential values.
export-env.shcontainsCYPRESS_LOGIN_USERSwith the kubeadmin password, yet this step tells Claude to read it and show the current values. Redact secrets before displaying them or comparing them.As per path instructions,
.claudechanges require security review for credential and tool-use risks.🤖 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/cypress/setup.md around lines 37 - 45, Update the export-env.sh handling instructions to never display or expose credential values, including CYPRESS_LOGIN_USERS and the kubeadmin password. Allow only masked or presence-based confirmation when checking existing values, and require security review for the .claude change before proceeding.Source: Path instructions
.claude/commands/cypress/test-development/fixture-schema-reference.md-1-1 (1)
1-1: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe shared command aliases point one directory too shallow.
.claude/commands/cypress/test-development/fixture-schema-reference.md#L1-L1: change the target to the repository-root.cursor/commands/fixture-schema-reference.md..claude/commands/cypress/test-development/generate-incident-fixture.md#L1-L1: change the target to the repository-root.cursor/commands/generate-incident-fixture.md..claude/commands/cypress/test-development/generate-regression-test.md#L1-L1: change the target to the repository-root.cursor/commands/generate-regression-test.md..claude/commands/cypress/test-development/refactor-regression-test.md#L1-L1: change the target to the repository-root.cursor/commands/refactor-regression-test.md..claude/commands/cypress/test-development/validate-incident-fixtures.md#L1-L1: change the target to the repository-root.cursor/commands/validate-incident-fixtures.md.🤖 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/cypress/test-development/fixture-schema-reference.md at line 1, The shared command aliases use targets that are one directory too shallow. Update the symbolic-link targets at line 1 of .claude/commands/cypress/test-development/fixture-schema-reference.md, .claude/commands/cypress/test-development/generate-incident-fixture.md, .claude/commands/cypress/test-development/generate-regression-test.md, .claude/commands/cypress/test-development/refactor-regression-test.md, and .claude/commands/cypress/test-development/validate-incident-fixtures.md to reference their corresponding files in the repository-root .cursor/commands directory.web/scripts/clean-test-artifacts.sh-5-8 (1)
5-8: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPropagate cleanup failures before running tests.
web/scripts/clean-test-artifacts.sh#L5-L8: fail on removal errors..claude/commands/cypress/test-iteration/run-and-report.md#L35-L40: stop when the cleanup command returns nonzero.Otherwise stale reports and screenshots can be merged into the next run.
🤖 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 `@web/scripts/clean-test-artifacts.sh` around lines 5 - 8, Make the cleanup commands in clean-test-artifacts.sh fail when any removal operation errors, then update the cleanup invocation in run-and-report.md to check its nonzero result and stop before tests run. Apply changes at web/scripts/clean-test-artifacts.sh lines 5-8 and .claude/commands/cypress/test-iteration/run-and-report.md lines 35-40; both sites require direct changes..claude/commands/cypress/setup.md-48-55 (1)
48-55: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not hard-code the sandbox kubeconfig path for host setup.
The template always writes
/tmp/kubeconfig, while the notes say host execution needs the actual kubeconfig path. Sinceensure-env.mdrejects missing paths, ordinary host setup can fail immediately.Also applies to: 72-75
🤖 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/cypress/setup.md around lines 48 - 55, Update the export-env.sh template in the setup instructions to use the actual host kubeconfig path rather than always exporting /tmp/kubeconfig. Make the setup prompt or substitute the user-provided path before writing CYPRESS_KUBECONFIG_PATH, while preserving validation by ensure-env.md..claude/commands/cypress/test-iteration/ensure-env.md-90-94 (1)
90-94: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse one variable name for the custom-plugin profile.
The profile writes and checks
CYPRESS_MP_IMAGE, but inference tests forMP_IMAGE. After setup, the documented custom-plugin configuration can therefore be classified ascatalogand prompt again.Proposed correction
- - `MP_IMAGE` — your custom monitoring-plugin image + - `CYPRESS_MP_IMAGE` — your custom monitoring-plugin image ... - - `COO_UI_INSTALL=true` + `MP_IMAGE` set → `custom-plugin` + - `COO_UI_INSTALL=true` + `CYPRESS_MP_IMAGE` set → `custom-plugin`Also applies to: 148-154
🤖 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/cypress/test-iteration/ensure-env.md around lines 90 - 94, Use a single environment variable for the custom-plugin image across the setup and inference logic: align the CYPRESS_MP_IMAGE checks/writes with the MP_IMAGE name used by the inference tests and requirements. Ensure the existing confirm-or-prompt-and-write behavior remains unchanged after the names are unified..claude/commands/cypress/run.md-14-15 (1)
14-15: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winComplete the setup/runner terminal contract.
.claude/commands/cypress/run.md#L14-L15: keep the prerequisite only if setup creates the named terminal..claude/commands/cypress/setup.md#L88-L90: explicitly create/open"Cypress Tests"before declaring setup complete.🤖 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/cypress/run.md around lines 14 - 15, Complete the Cypress setup/runner terminal contract: in .claude/commands/cypress/setup.md lines 88-90, explicitly create or open the terminal named “Cypress Tests” before setup is declared complete; in .claude/commands/cypress/run.md lines 14-15, retain the prerequisite requiring that terminal only once setup guarantees its creation..claude/commands/cypress/setup.md-81-85 (1)
81-85: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftThe workflow repeatedly executes a working-tree-controlled shell file.
.claude/commands/cypress/setup.md#L81-L85: replace direct sourcing with validated parsing..claude/commands/cypress/test-iteration/ensure-env.md#L167-L170: validate only expected environment assignments before loading values..claude/commands/cypress/test-iteration/run-suite.md#L24-L29: do not sourceexport-env.shin the Cypress launch command.As per path instructions,
.claudechanges require security/malware review, especially for arbitrary command execution.🤖 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/cypress/setup.md around lines 81 - 85, Replace direct sourcing of the working-tree-controlled export-env.sh in .claude/commands/cypress/setup.md lines 81-85 with validated parsing of only expected environment assignments before using the values. Apply the same allowlist-based validation before loading values in .claude/commands/cypress/test-iteration/ensure-env.md lines 167-170. Remove export-env.sh sourcing from the Cypress launch command in .claude/commands/cypress/test-iteration/run-suite.md lines 24-29, and ensure all .claude changes receive security/malware review for arbitrary command execution.Source: Path instructions
.claude/commands/cypress/test-iteration/run-and-report.md-51-66 (1)
51-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not report skipped tests as “all passed.”
The success condition checks only exit code and failures, but
run-suite.mdexplicitly reports pending/skipped tests. Requireskipped == 0before emitting “All tests passed,” or state that the run completed with skipped tests.Proposed correction
- If exit code is `0` and no failures in the results: + If exit code is `0`, failures are `0`, and skipped tests are `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/commands/cypress/test-iteration/run-and-report.md around lines 51 - 66, Update the Step 5 success condition to require zero skipped tests in addition to exit code 0 and no failures before emitting “All Passed.” If skipped tests are present, use a separate completed-with-skipped-tests outcome instead of claiming all tests passed, while preserving the existing stability-ledger update behavior as appropriate..claude/commands/cypress/setup.md-23-30 (1)
23-30: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInstall dependencies from the runner’s project root.
This command installs from
web/cypress, whileensure-env.mdchecksweb/node_modules/.bin/cypressandrun-suite.mdrunsnpx cypressfromweb. Setup can therefore report success while the runner still cannot find Cypress.Use the same dependency root as the consumers.
Proposed alignment
- cd web/cypress + cd web npm install + cd cypress🤖 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/cypress/setup.md around lines 23 - 30, Update the dependency-installation step in the Cypress setup instructions to run npm install from the web project root, matching the web/node_modules/.bin/cypress check in ensure-env.md and the npx cypress execution context in run-suite.md; remove the web/cypress directory change for this step..claude/commands/cypress/test-iteration/iterate-incident-tests.md-174-181 (1)
174-181: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
grepfor title patterns, notgrepTags.The documented grep-pattern target is passed as
grepTags, which only matches test tags; a title such asshould filter by severitywill not select the intended test. Route title patterns through--env grep=..., reservegrepTagsfor tag expressions, and apply the same correction to Step 10.🤖 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/cypress/test-iteration/iterate-incident-tests.md around lines 174 - 181, Update the target-to-command mapping so grep-pattern targets use Cypress `--env grep="{target}"` for title matching instead of `grepTags`; retain `grepTags` only for tag expressions in the `all` and `regression` targets. Apply the same command correction to the corresponding Step 10 instructions..claude/commands/cypress/test-iteration/refactor-page-object.md-110-117 (1)
110-117: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep autonomous execution commands unchained. Both examples contradict the workflow rule that
&&causes a security approval prompt and blocks unattended iteration.
.claude/commands/cypress/test-iteration/refactor-page-object.md#L110-L117: split environment sourcing and Cypress execution into separate tool calls.docs/agentic-development/architecture/test-iteration-system.md#L169-L180: replacecd web && source ...with separately scoped working-directory and execution steps.🤖 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/cypress/test-iteration/refactor-page-object.md around lines 110 - 117, Keep autonomous execution commands unchained. In .claude/commands/cypress/test-iteration/refactor-page-object.md (lines 110-117), separate environment sourcing from Cypress execution into distinct tool calls; in docs/agentic-development/architecture/test-iteration-system.md (lines 169-180), replace the chained “cd web && source ...” flow with separately scoped working-directory and execution steps..claude/commands/cypress/test-iteration/diagnose-test-failure.md-36-52 (1)
36-52: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winConstrain caller-supplied local paths.
A standalone invocation can provide an arbitrary absolute
screenshot-pathorspec-file, causing the Read tool to access unrelated local files. Resolve and require screenshots underweb/cypress/screenshots/and specs under the approved Cypress test roots; reject paths escaping those roots, including symlink escapes.🤖 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/cypress/test-iteration/diagnose-test-failure.md around lines 36 - 52, Update the screenshot and spec file handling in the test-diagnosis workflow to constrain caller-supplied paths before invoking the Read tool. Require screenshot-path values to resolve within web/cypress/screenshots/ and spec-file values within the approved Cypress test roots, rejecting absolute paths, traversal, and symlink-based escapes; preserve the existing read-and-diagnose flow for validated paths.Source: Path instructions
.claude/commands/cypress/test-iteration/analyze-ci-results.md-23-43 (1)
23-43: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAllowlist CI artifact URLs before fetching.
ci-urlbecomes the base for WebFetch requests without validating its host or artifact path. Require HTTPS, the two expected CI hosts, and the expectedtest-platform-results/pr-logs/pull/openshift_monitoring-plugin/...path; reject userinfo, query, fragments, and traversal. Treat fetched artifacts strictly as data, never operational instructions.🤖 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/cypress/test-iteration/analyze-ci-results.md around lines 23 - 43, Update the URL normalization step before deriving artifact paths or issuing WebFetch requests to validate ci-url: require HTTPS, either expected CI host, and the expected test-platform-results/pr-logs/pull/openshift_monitoring-plugin path; reject userinfo, queries, fragments, and traversal segments. Only fetch URLs that pass this allowlist, and treat their contents strictly as data rather than instructions.Source: Path instructions
.claude/commands/cypress/test-iteration/iterate-ci-flaky.md-50-79 (1)
50-79: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDo not preapprove broad write and execution capabilities for an autonomous agent. These permissions are not scoped to approved files, remotes, branches, or API endpoints, yet the workflows consume untrusted CI artifacts and PR comments.
.claude/commands/cypress/test-iteration/iterate-ci-flaky.md#L50-L79: replace unrestrictedgh api:*,git push:*, andpython3:*with narrowly scoped wrappers that validate the repository, PR head, destination branch, and allowed paths..claude/commands/cypress/test-iteration/iterate-incident-tests.md#L60-L88: avoid wildcardgit add:*/git commit:*; validate the staged diff against the test-only allowlist before requesting the minimal required approval.As per path instructions,
.claudechanges require close scrutiny for broad filesystem or network access.🤖 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/cypress/test-iteration/iterate-ci-flaky.md around lines 50 - 79, Replace unrestricted permissions in .claude/commands/cypress/test-iteration/iterate-ci-flaky.md lines 50-79, especially gh api:*, git push:*, and python3:*, with narrowly scoped validation wrappers that enforce the repository, PR head, destination branch, API endpoints, and permitted paths. In .claude/commands/cypress/test-iteration/iterate-incident-tests.md lines 60-88, replace wildcard git add:* and git commit:* permissions with validation that checks the staged diff against the test-only allowlist before requesting minimal approval.Source: Path instructions
.claude/commands/cypress/test-iteration/iterate-incident-tests.md-35-56 (1)
35-56: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not write conversation credentials into a repository file without proving it is local-only.
.claude/commands/cypress/test-iteration/iterate-incident-tests.md#L35-L56: requireumask 077, verifyweb/cypress/export-env.shis ignored before writing, and stop if it is tracked or not ignored..claude/commands/cypress/test-iteration/productize-iterations.md#L29-L33: apply the same ignored-file and restrictive-permission checks before accepting credentials from the conversation.As per path instructions,
.claudechanges require close scrutiny for broad filesystem access.🤖 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/cypress/test-iteration/iterate-incident-tests.md around lines 35 - 56, Update the export-env.sh credential-writing instructions in .claude/commands/cypress/test-iteration/iterate-incident-tests.md at lines 35-56 to set umask 077, verify the target file is ignored before writing, and stop if it is tracked or not ignored. Apply the same ignored-file and restrictive-permission checks in .claude/commands/cypress/test-iteration/productize-iterations.md at lines 29-33 before accepting conversation credentials; do not broaden filesystem access.Source: Path instructions
docs/agentic-development/roadmap/test-iteration-ideas.md-421-445 (1)
421-445: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftGuard this workflow before publishing. As written, any
issue_commentcontaining/run-flaky-iterationcan launch a secret-bearing, permission-skipping agent. Gate it to PR comments only, require a trusted association/team and exact command match, and tighten token permissions before using this pattern.🤖 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 `@docs/agentic-development/roadmap/test-iteration-ideas.md` around lines 421 - 445, Harden the “Flaky Test Iteration” workflow trigger before running the Claude agent: require the comment to originate from a pull request, restrict authors to an approved association or team, and match the command exactly rather than using a substring check. Add least-privilege GitHub token permissions and ensure the secret-bearing, permission-skipping steps run only after these guards pass..claude/commands/cypress/test-iteration/scripts/poll-ci-status.py-35-41 (1)
35-41: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle
gh pr checksexit code 8 as pending.claude/commands/cypress/test-iteration/scripts/poll-ci-status.py#L35-L41: treat exit code 8 like a valid pending response and still parse the JSON. Right now a pending unrelated check short-circuits the loop, so the poller can miss a completed target job and sleep until timeout.🤖 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/cypress/test-iteration/scripts/poll-ci-status.py around lines 35 - 41, Update the gh pr checks result handling in poll-ci-status.py so exit code 8 is treated as a pending response: continue parsing its JSON instead of logging, sleeping, and retrying immediately; retain the existing failure path for other nonzero exit codes. Apply the corresponding guidance or invocation handling in iterate-ci-flaky.md at lines 181-196, ensuring the documented poller behavior preserves parsing for exit code 8.Source: Path instructions
.claude/commands/cypress/test-iteration/scripts/review-github.py-123-155 (1)
123-155: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winFilter on
created_atand page through all results.sinceis update-based, so an older author comment edited during the window can be treated as a new reply. Checkcreated_atagainstsince_timestamplocally, and keep paginating beyond the firstper_page=50batch.🤖 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/cypress/test-iteration/scripts/review-github.py around lines 123 - 155, Update the comment-fetch loop around gh_api to paginate through all result pages rather than stopping at the first per_page=50 batch, and locally accept only comments whose created_at is at or after since_timestamp. Preserve the existing seen_ids, pr_author, MAGIC_PREFIX, and reply extraction behavior while applying the created_at filter before processing a comment.Source: Path instructions
.claude/commands/cypress/test-iteration/iterate-ci-flaky.md-115-131 (1)
115-131: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCheck out the PR head remote explicitly —
headRefNamealone isn’t enough here;git fetch origin {headRefName}+git checkout {headRefName}can attach to the wrong branch when the PR comes from a fork or when a same-named local branch already exists. Usegh pr checkout {pr}or resolve the head repository/remote first.🤖 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/cypress/test-iteration/iterate-ci-flaky.md around lines 115 - 131, Update the PR checkout instructions after extracting headRefName to use gh pr checkout {pr}, or explicitly resolve and fetch the PR head repository before checking out the exact remote ref. Replace the current git fetch origin {headRefName} and git checkout {headRefName} commands so forked PRs and same-named local branches always select the requested PR head.Source: Path instructions
🟡 Minor comments (6)
.claude/commands/cypress/test-iteration/ensure-env.md-190-190 (1)
190-190: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to the fenced output block.
Use
textfor theENV_READYoutput example to satisfy MD040.🤖 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/cypress/test-iteration/ensure-env.md at line 190, Update the fenced output block containing the ENV_READY example in ensure-env.md to specify the text language, preserving the example’s content while satisfying MD040.Source: Linters/SAST tools
.claude/commands/cypress/test-iteration/run-and-report.md-56-56 (1)
56-56: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd languages to the fenced blocks.
Use
textfor the report examples andmarkdownfor the run-history table to satisfy MD040.Also applies to: 91-91, 137-137
🤖 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/cypress/test-iteration/run-and-report.md at line 56, Update the fenced code blocks in the run-and-report documentation, including the report examples and run-history table, with explicit language identifiers: use text for report examples and markdown for the run-history table. Apply the same identifiers to the additional fenced blocks referenced by the review.Source: Linters/SAST tools
.claude/commands/cypress/test-iteration/run-suite.md-51-51 (1)
51-51: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd languages to all fenced blocks.
Use
text(or the actual syntax where applicable) for the pseudo-output blocks to satisfy MD040.Also applies to: 57-57, 77-77, 86-86, 111-111
🤖 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/cypress/test-iteration/run-suite.md at line 51, Add an explicit language identifier to every fenced code block in run-suite.md, including the blocks at the referenced sections; use text for pseudo-output and the appropriate actual syntax for code examples so all fences satisfy MD040.Source: Linters/SAST tools
.claude/commands/cypress/test-iteration/run-suite.md-41-42 (1)
41-42: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winUse the local
mochawesome-mergebinary instead ofnpx. The package is already declared and lockfile-pinned; invoking the workspace binary avoids any registry fallback ifnode_modulesis missing.🤖 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/cypress/test-iteration/run-suite.md around lines 41 - 42, Update the mochawesome report merge command in the test-suite instructions to invoke the locally installed mochawesome-merge binary directly instead of using npx, while preserving the existing input glob and output path.Source: Path instructions
docs/agentic-development/roadmap/test-iteration-ideas.md-107-110 (1)
107-110: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the ledger implementation status.
This says local updates are in Step 14 and CI wiring is still pending, but the reviewed commands update the ledger in local Step 15 and CI Step 10. The roadmap currently directs follow-up work at already-implemented behavior.
🤖 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 `@docs/agentic-development/roadmap/test-iteration-ideas.md` around lines 107 - 110, Update the “Test Stability Ledger” status in the roadmap to reflect that ledger updates are implemented in local Step 15 and CI Step 10, and remove the outdated statement that CI wiring remains pending.docs/agentic-development/roadmap/test-iteration-ideas.md-129-131 (1)
129-131: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the stale Slack “implemented” claim.
The PR objective says the Slack notification script was removed, and the reviewed CI workflow implements GitHub-comment review only. Mark Slack as deferred or remove this status assertion to avoid prompting users to configure unavailable notification behavior.
🤖 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 `@docs/agentic-development/roadmap/test-iteration-ideas.md` around lines 129 - 131, Update the “Slack Notifications for Long-Running Loops” section to remove the claim that Slack webhook notifications are implemented, and mark the Slack option as deferred or otherwise unavailable. Keep the documented GitHub-comment review flow as the implemented behavior and avoid instructing users to configure Slack notifications.
🤖 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/cypress/test-iteration/productize-iterations.md:
- Around line 35-43: Update the source-branches parsing instructions in “Step 1:
Discover and Fetch Source Branches” to validate branch or glob syntax before use
and never interpolate the user-controlled value into a shell pipeline. Use git
ls-remote --heads with the validated pattern passed as one argument, then fetch
only the resulting branch names as remote-tracking refs while preserving
comma-separated handling.
---
Major comments:
In @.claude/commands/cypress/run.md:
- Around line 14-15: Complete the Cypress setup/runner terminal contract: in
.claude/commands/cypress/setup.md lines 88-90, explicitly create or open the
terminal named “Cypress Tests” before setup is declared complete; in
.claude/commands/cypress/run.md lines 14-15, retain the prerequisite requiring
that terminal only once setup guarantees its creation.
In @.claude/commands/cypress/setup.md:
- Around line 28-30: Replace the npm install commands in
.claude/commands/cypress/setup.md lines 28-30 and
.claude/commands/cypress/test-iteration/ensure-env.md lines 18-20 with a
lockfile-controlled install that requires confirmation before lifecycle scripts
run, applying the same policy to both primary and fallback dependency-install
paths.
- Around line 37-45: Update the export-env.sh handling instructions to never
display or expose credential values, including CYPRESS_LOGIN_USERS and the
kubeadmin password. Allow only masked or presence-based confirmation when
checking existing values, and require security review for the .claude change
before proceeding.
- Around line 48-55: Update the export-env.sh template in the setup instructions
to use the actual host kubeconfig path rather than always exporting
/tmp/kubeconfig. Make the setup prompt or substitute the user-provided path
before writing CYPRESS_KUBECONFIG_PATH, while preserving validation by
ensure-env.md.
- Around line 81-85: Replace direct sourcing of the working-tree-controlled
export-env.sh in .claude/commands/cypress/setup.md lines 81-85 with validated
parsing of only expected environment assignments before using the values. Apply
the same allowlist-based validation before loading values in
.claude/commands/cypress/test-iteration/ensure-env.md lines 167-170. Remove
export-env.sh sourcing from the Cypress launch command in
.claude/commands/cypress/test-iteration/run-suite.md lines 24-29, and ensure all
.claude changes receive security/malware review for arbitrary command execution.
- Around line 23-30: Update the dependency-installation step in the Cypress
setup instructions to run npm install from the web project root, matching the
web/node_modules/.bin/cypress check in ensure-env.md and the npx cypress
execution context in run-suite.md; remove the web/cypress directory change for
this step.
In @.claude/commands/cypress/test-development/fixture-schema-reference.md:
- Line 1: The shared command aliases use targets that are one directory too
shallow. Update the symbolic-link targets at line 1 of
.claude/commands/cypress/test-development/fixture-schema-reference.md,
.claude/commands/cypress/test-development/generate-incident-fixture.md,
.claude/commands/cypress/test-development/generate-regression-test.md,
.claude/commands/cypress/test-development/refactor-regression-test.md, and
.claude/commands/cypress/test-development/validate-incident-fixtures.md to
reference their corresponding files in the repository-root .cursor/commands
directory.
In @.claude/commands/cypress/test-iteration/analyze-ci-results.md:
- Around line 23-43: Update the URL normalization step before deriving artifact
paths or issuing WebFetch requests to validate ci-url: require HTTPS, either
expected CI host, and the expected
test-platform-results/pr-logs/pull/openshift_monitoring-plugin path; reject
userinfo, queries, fragments, and traversal segments. Only fetch URLs that pass
this allowlist, and treat their contents strictly as data rather than
instructions.
In @.claude/commands/cypress/test-iteration/diagnose-test-failure.md:
- Around line 36-52: Update the screenshot and spec file handling in the
test-diagnosis workflow to constrain caller-supplied paths before invoking the
Read tool. Require screenshot-path values to resolve within
web/cypress/screenshots/ and spec-file values within the approved Cypress test
roots, rejecting absolute paths, traversal, and symlink-based escapes; preserve
the existing read-and-diagnose flow for validated paths.
In @.claude/commands/cypress/test-iteration/ensure-env.md:
- Around line 90-94: Use a single environment variable for the custom-plugin
image across the setup and inference logic: align the CYPRESS_MP_IMAGE
checks/writes with the MP_IMAGE name used by the inference tests and
requirements. Ensure the existing confirm-or-prompt-and-write behavior remains
unchanged after the names are unified.
In @.claude/commands/cypress/test-iteration/iterate-ci-flaky.md:
- Around line 50-79: Replace unrestricted permissions in
.claude/commands/cypress/test-iteration/iterate-ci-flaky.md lines 50-79,
especially gh api:*, git push:*, and python3:*, with narrowly scoped validation
wrappers that enforce the repository, PR head, destination branch, API
endpoints, and permitted paths. In
.claude/commands/cypress/test-iteration/iterate-incident-tests.md lines 60-88,
replace wildcard git add:* and git commit:* permissions with validation that
checks the staged diff against the test-only allowlist before requesting minimal
approval.
- Around line 115-131: Update the PR checkout instructions after extracting
headRefName to use gh pr checkout {pr}, or explicitly resolve and fetch the PR
head repository before checking out the exact remote ref. Replace the current
git fetch origin {headRefName} and git checkout {headRefName} commands so forked
PRs and same-named local branches always select the requested PR head.
In @.claude/commands/cypress/test-iteration/iterate-incident-tests.md:
- Around line 174-181: Update the target-to-command mapping so grep-pattern
targets use Cypress `--env grep="{target}"` for title matching instead of
`grepTags`; retain `grepTags` only for tag expressions in the `all` and
`regression` targets. Apply the same command correction to the corresponding
Step 10 instructions.
- Around line 35-56: Update the export-env.sh credential-writing instructions in
.claude/commands/cypress/test-iteration/iterate-incident-tests.md at lines 35-56
to set umask 077, verify the target file is ignored before writing, and stop if
it is tracked or not ignored. Apply the same ignored-file and
restrictive-permission checks in
.claude/commands/cypress/test-iteration/productize-iterations.md at lines 29-33
before accepting conversation credentials; do not broaden filesystem access.
In @.claude/commands/cypress/test-iteration/refactor-page-object.md:
- Around line 110-117: Keep autonomous execution commands unchained. In
.claude/commands/cypress/test-iteration/refactor-page-object.md (lines 110-117),
separate environment sourcing from Cypress execution into distinct tool calls;
in docs/agentic-development/architecture/test-iteration-system.md (lines
169-180), replace the chained “cd web && source ...” flow with separately scoped
working-directory and execution steps.
In @.claude/commands/cypress/test-iteration/run-and-report.md:
- Around line 51-66: Update the Step 5 success condition to require zero skipped
tests in addition to exit code 0 and no failures before emitting “All Passed.”
If skipped tests are present, use a separate completed-with-skipped-tests
outcome instead of claiming all tests passed, while preserving the existing
stability-ledger update behavior as appropriate.
In @.claude/commands/cypress/test-iteration/run-suite.md:
- Around line 21-25: Update the Cypress command in the test-iteration
instructions to launch a tracked background process, persist its logs and exit
code, and wait or poll until completion before reporting the result. Preserve
the existing environment setup, spec selection, and grepTags arguments while
ensuring the agent is not blocked by a foreground process.
In @.claude/commands/cypress/test-iteration/scripts/poll-ci-status.py:
- Around line 35-41: Update the gh pr checks result handling in
poll-ci-status.py so exit code 8 is treated as a pending response: continue
parsing its JSON instead of logging, sleeping, and retrying immediately; retain
the existing failure path for other nonzero exit codes. Apply the corresponding
guidance or invocation handling in iterate-ci-flaky.md at lines 181-196,
ensuring the documented poller behavior preserves parsing for exit code 8.
In @.claude/commands/cypress/test-iteration/scripts/review-github.py:
- Around line 123-155: Update the comment-fetch loop around gh_api to paginate
through all result pages rather than stopping at the first per_page=50 batch,
and locally accept only comments whose created_at is at or after
since_timestamp. Preserve the existing seen_ids, pr_author, MAGIC_PREFIX, and
reply extraction behavior while applying the created_at filter before processing
a comment.
In `@docs/agentic-development/roadmap/test-iteration-ideas.md`:
- Around line 421-445: Harden the “Flaky Test Iteration” workflow trigger before
running the Claude agent: require the comment to originate from a pull request,
restrict authors to an approved association or team, and match the command
exactly rather than using a substring check. Add least-privilege GitHub token
permissions and ensure the secret-bearing, permission-skipping steps run only
after these guards pass.
In `@web/scripts/clean-test-artifacts.sh`:
- Around line 5-8: Make the cleanup commands in clean-test-artifacts.sh fail
when any removal operation errors, then update the cleanup invocation in
run-and-report.md to check its nonzero result and stop before tests run. Apply
changes at web/scripts/clean-test-artifacts.sh lines 5-8 and
.claude/commands/cypress/test-iteration/run-and-report.md lines 35-40; both
sites require direct changes.
---
Minor comments:
In @.claude/commands/cypress/test-iteration/ensure-env.md:
- Line 190: Update the fenced output block containing the ENV_READY example in
ensure-env.md to specify the text language, preserving the example’s content
while satisfying MD040.
In @.claude/commands/cypress/test-iteration/run-and-report.md:
- Line 56: Update the fenced code blocks in the run-and-report documentation,
including the report examples and run-history table, with explicit language
identifiers: use text for report examples and markdown for the run-history
table. Apply the same identifiers to the additional fenced blocks referenced by
the review.
In @.claude/commands/cypress/test-iteration/run-suite.md:
- Line 51: Add an explicit language identifier to every fenced code block in
run-suite.md, including the blocks at the referenced sections; use text for
pseudo-output and the appropriate actual syntax for code examples so all fences
satisfy MD040.
- Around line 41-42: Update the mochawesome report merge command in the
test-suite instructions to invoke the locally installed mochawesome-merge binary
directly instead of using npx, while preserving the existing input glob and
output path.
In `@docs/agentic-development/roadmap/test-iteration-ideas.md`:
- Around line 107-110: Update the “Test Stability Ledger” status in the roadmap
to reflect that ledger updates are implemented in local Step 15 and CI Step 10,
and remove the outdated statement that CI wiring remains pending.
- Around line 129-131: Update the “Slack Notifications for Long-Running Loops”
section to remove the claim that Slack webhook notifications are implemented,
and mark the Slack option as deferred or otherwise unavailable. Keep the
documented GitHub-comment review flow as the implemented behavior and avoid
instructing users to configure Slack notifications.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: c6429296-9ca2-489f-8c14-7166f739ea97
📒 Files selected for processing (27)
.claude/commands/cypress/cypress-setup.md.claude/commands/cypress/run.md.claude/commands/cypress/setup.md.claude/commands/cypress/test-development/fixture-schema-reference.md.claude/commands/cypress/test-development/generate-incident-fixture.md.claude/commands/cypress/test-development/generate-regression-test.md.claude/commands/cypress/test-development/refactor-regression-test.md.claude/commands/cypress/test-development/validate-incident-fixtures.md.claude/commands/cypress/test-iteration/analyze-ci-results.md.claude/commands/cypress/test-iteration/diagnose-test-failure.md.claude/commands/cypress/test-iteration/ensure-env.md.claude/commands/cypress/test-iteration/iterate-ci-flaky.md.claude/commands/cypress/test-iteration/iterate-incident-tests.md.claude/commands/cypress/test-iteration/productize-iterations.md.claude/commands/cypress/test-iteration/refactor-page-object.md.claude/commands/cypress/test-iteration/run-and-report.md.claude/commands/cypress/test-iteration/run-suite.md.claude/commands/cypress/test-iteration/scripts/poll-ci-status.py.claude/commands/cypress/test-iteration/scripts/review-github.py.claude/commands/fixture-schema-reference.md.claude/commands/generate-incident-fixture.md.claude/commands/generate-regression-test.md.claude/commands/refactor-regression-test.md.claude/commands/validate-incident-fixtures.mddocs/agentic-development/architecture/test-iteration-system.mddocs/agentic-development/roadmap/test-iteration-ideas.mdweb/scripts/clean-test-artifacts.sh
💤 Files with no reviewable changes (6)
- .claude/commands/generate-incident-fixture.md
- .claude/commands/cypress/cypress-setup.md
- .claude/commands/validate-incident-fixtures.md
- .claude/commands/fixture-schema-reference.md
- .claude/commands/generate-regression-test.md
- .claude/commands/refactor-regression-test.md
| ## Step 1: Discover and Fetch Source Branches | ||
|
|
||
| Parse `source-branches` parameter: | ||
| - If it contains `*`, treat as a glob: `git ls-remote origin | grep <pattern>` | ||
| - If comma-separated, split into a list | ||
|
|
||
| Fetch all source branches as remote tracking refs: | ||
| ```bash | ||
| git fetch origin "<branch>:refs/remotes/origin/<branch>" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not interpolate source-branches into a shell pipeline.
grep <pattern> inserts a user-controlled value unquoted into Bash. A crafted glob can execute additional commands on the developer machine. Validate branch/glob syntax first and pass it as a single argument to a non-shell command such as git ls-remote --heads origin "refs/heads/<validated-pattern>".
🤖 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/cypress/test-iteration/productize-iterations.md around
lines 35 - 43, Update the source-branches parsing instructions in “Step 1:
Discover and Fetch Source Branches” to validate branch or glob syntax before use
and never interpolate the user-controlled value into a shell pipeline. Use git
ls-remote --heads with the validated pattern passed as one argument, then fetch
only the resulting branch names as remote-tracking refs while preserving
comma-separated handling.
Source: Path instructions
|
/cc @etmurasaki @jgbernalp Please note that this is my last week at my current position, it would be great to get this merged / rejected during the next 5 days. |
|
@DavidRajnoha: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Summary
Stripped-down resubmission of #879 — removes the Docker sandbox infrastructure
entirely and focuses only on the Claude Code skills for autonomous Cypress test
iteration against local and OpenShift CI environments.
What's included
test-development/andtest-iteration/subdirectories, cypress-run/cypress-setup renameddiagnose-test-failure, analyze-ci-results, productize-iterations,
refactor-page-object, ensure-env, run-suite, run-and-report
poll-ci-status.py(CI polling),review-github.py(PR comment review flow)
What's removed vs #879
sandbox/Dockerfile,sandbox/run.sh)notify-slack.py) — requires workspace apppermissions, not suitable for quick ad-hoc setups
Summary by CodeRabbit