[RaphaelFakhri]

Description of Changes

validateCompositeCommand resolves subcommand references case-insensitively and stores the lowercase command ID in parentCommands, but it removes the entry with the raw reference. When a reference is not lowercase (for example Composite-A for composite-a), the entry is never removed, and a later reference to the same command fails with composite command cannot indirectly reference itself, even though the devfile has no cycle.

This change deletes the lowercase key, matching how the key is added and looked up. It also adds a test case to TestValidateCompositeCommand with a mixed-case subcommand reference.

Related Issue(s)

Fixes #1805

Acceptance Criteria

• Unit/Functional tests

  New case Valid Composite command with mixed-case subcommand references in pkg/validation/commands_test.go.

• QE Integration test

  Not needed: the change only removes a false validation error.

• Documentation

  Not needed: no documented behavior changes.

• Client Impact

  None expected: devfiles that previously failed with a false cycle error now pass. Devfiles with real cycles still fail.

Tests Performed

• go test ./pkg/validation/ -run TestValidateCompositeCommand -v: the new case fails without the fix (the command "composite-b" is invalid - composite command cannot indirectly reference itself) and passes with it (7 of 7 subtests pass).
• go test ./pkg/validation/... ./pkg/utils/... ./pkg/devfile/... ./pkg/apis/... passes.
• gofmt -l pkg/validation and go vet ./pkg/validation/ are clean.

How To Test

go test ./pkg/validation/ -run TestValidateCompositeCommand -v

Notes To Reviewer

The other lookups in validateCompositeCommand already use strings.ToLower(cmd). This change makes the delete call consistent with them.

Summary by CodeRabbit

• Bug Fixes
  • Composite-command validation now matches subcommand references without regard to capitalization. Valid commands are no longer rejected when the referenced command ID uses different letter casing, such as Composite-A versus composite-a. This makes validation more consistent when command IDs are written with different capitalization.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: RaphaelFakhri

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Composite command validation now removes parent-tracking entries using the lowercased subcommand ID. A new test checks that validation accepts a mixed-case reference.

Changes: Composite command validation

Layer / File(s)	Summary
Case-normalized parent tracking pkg/validation/commands.go, pkg/validation/commands_test.go	Validation deletes the lowercased subcommand ID from parentCommands. The test checks validation with the mixed-case reference Composite-A to composite-a.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: aobuchow

Merge Risk: Minimal · up to faad6

The change aligns parent-command cleanup with case-normalized lookup behavior and adds coverage for mixed-case references.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: case-insensitive tracking of composite subcommand IDs.
Linked Issues check		Issue #1805 requires valid case-insensitive nested composite references to pass when a command appears again without a cycle, while real cycles remain invalid. validateCompositeCommand now deletes `…
Out of Scope Changes check		The pull request changes only the parent-command deletion key in pkg/validation/commands.go and adds the targeted regression case in pkg/validation/commands_test.go. Both changes directly support …

Full details: Linked Issues check

Explanation

Issue #1805 requires valid case-insensitive nested composite references to pass when a command appears again without a cycle, while real cycles remain invalid. validateCompositeCommand now deletes strings.ToLower(cmd), which matches the normalized map lookup and storage path. The new TestValidateCompositeCommand case references composite-a as Composite-A and then references composite-b; it verifies the reported false cycle does not occur. The existing parent tracking and cycle checks remain in place.

Full details: Out of Scope Changes check

Explanation

The pull request changes only the parent-command deletion key in pkg/validation/commands.go and adds the targeted regression case in pkg/validation/commands_test.go. Both changes directly support issue #1805. No unrelated production behavior or unrelated test scope is shown.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

ast-grep (0.51.1)

ast-grep could not parse rule config: /ast-grep-rules/ast-grep-essentials/rules/kotlin/security/open-redirect-spring-redirect-prefix-kotlin.yml

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: deab0bda-a3d4-488d-a30b-0829c8537e67

Commits

Reviewing files that changed from the base of the PR and between 36c51bf and faad666.

Files selected for processing: 2

• pkg/validation/commands.go
• pkg/validation/commands_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.