fix(EC-1994): correct off-by-one in validate image worker loop by cuipinghuo · Pull Request #3425 · conforma/cli

cuipinghuo · 2026-07-20T20:19:16Z

Summary

Fix off-by-one in worker loop at cmd/validate/image.go:404: <= changed to < so the loop spawns exactly numWorkers goroutines instead of numWorkers+1
Bump task worker defaults by 1 to preserve the effective concurrency users had before the fix (since the old <= loop spawned N+1 workers for a configured value of N):
- verify-conforma-konflux-ta: 4 → 5
- verify-enterprise-contract: 1 → 2
Update READMEs and generated docs to match new defaults; fix stale documentation that incorrectly claimed WORKERS was unused and defaulted to 35

Test plan

go build ./cmd/validate/ passes
go test -tags=unit ./cmd/validate/ -run TestValidateImage passes
make generate produces no additional diff

Resolves: https://redhat.atlassian.net/browse/EC-1994

🤖 Generated with Claude Code

coderabbitai · 2026-07-20T20:20:01Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 49547f5c-09b2-44b0-9903-2a264c3f09e0

📥 Commits

Reviewing files that changed from the base of the PR and between ec4d985 and bff1983.

📒 Files selected for processing (2)

tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml

🚧 Files skipped from review as they are similar to previous changes (2)

tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The image validator now starts exactly the configured number of workers. The Konflux and enterprise contract pipeline defaults and documentation now use updated WORKERS values.

Changes

Worker count alignment

Layer / File(s)	Summary
Match configured worker count `cmd/validate/image.go`	The worker loop now starts exactly `numWorkers` workers.
Update pipeline worker defaults `tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml`, `docs/modules/ROOT/pages/verify-conforma-konflux-ta.adoc`, `tasks/verify-conforma-konflux-ta/0.1/README.md`, `pipelines/enterprise-contract/0.1/enterprise-contract.yaml`, `tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml`, `docs/modules/ROOT/pages/verify-enterprise-contract.adoc`, `tasks/verify-enterprise-contract/0.1/README.md`	The Konflux default changes from 4 to 5. The enterprise contract default changes from 1 to 2. The documentation reflects these values and the Konflux parallel evaluation behavior.

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

Merge Risk: ⚪ Minimal · up to bff19

The worker loop now starts the intended number of workers, while task defaults and documentation preserve the expected effective concurrency. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main fix: correcting the off-by-one error in the validate image worker loop.
Description check	✅ Passed	The description explains the change, rationale, testing, and linked ticket, although it uses Summary and Test plan headings instead of the template headings.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-07-20T20:33:17Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag	Coverage Δ
acceptance	`54.38% <100.00%> (-0.02%)`	⬇️
generative	`12.28% <0.00%> (-4.09%)`	⬇️
integration	`23.59% <100.00%> (-3.99%)`	⬇️
unit	`72.20% <100.00%> (+0.22%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
cmd/validate/image.go	`91.31% <100.00%> (ø)`

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

fullsend-ai-review · 2026-07-23T14:07:39Z

🤖 Review · ⚠️ Cancelled · Started 2:07 PM UTC · Ended 2:15 PM UTC
Commit: 87c4a29 · View workflow run →

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml`:
- Line 180: Synchronize the README worker defaults with the Task definitions:
for tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml lines
180-180, document the effective default as 5 instead of 35/unused; for
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml lines
186-186, update the README default from 1 to 2.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8ffe48a4-a59b-47ad-aa6c-96664ec08be3

📥 Commits

Reviewing files that changed from the base of the PR and between ea9f3be and 44b18de.

📒 Files selected for processing (2)

tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml
tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml

fullsend-ai-review · 2026-07-23T14:16:36Z

🤖 Finished Review · ✅ Success · Started 2:16 PM UTC · Completed 2:31 PM UTC
Commit: 87c4a29 · View workflow run →

fullsend-ai-review · 2026-07-23T14:31:30Z

Review

Findings

Low

[behavioral-contract-change] cmd/validate/image.go:404 — The --workers flag now spawns exactly N workers instead of N+1. The task YAML defaults are bumped to compensate (4→5, 1→2), preserving effective concurrency for default users. Direct CLI users with the default of 5 will go from 6 effective workers to 5. Downstream systems that override WORKERS to a specific value (e.g., 35) will get exactly that many workers instead of N+1. No action required — this is the correct fix aligning behavior with the documented contract.

Previous run

Review

Findings

Medium

[stale-default-value] docs/modules/ROOT/pages/verify-conforma-vsa-release-ta.adoc:55 — The WORKERS parameter default is documented as 4, but this PR bumps the related verify-conforma-konflux-ta task default from 4 to 5 to preserve effective concurrency after the off-by-one fix. If the VSA release task inherits or mirrors the same default, this doc is now stale and the VSA release task's YAML (defined externally) may also need its WORKERS default bumped from 4 to 5.
Remediation: Confirm whether the VSA release task shares the same default lineage as verify-conforma-konflux-ta. If so, update the doc and the external task YAML. If the VSA release task has its own independent default, confirm it is still correct and leave as-is.

Low

[test-adequacy] cmd/validate/image.go:404 — No test covers the worker loop to verify that exactly numWorkers goroutines are spawned. The off-by-one existed since the loop was introduced and was not caught by tests. A regression test would prevent reintroduction.
Remediation: Consider adding a test that asserts the number of concurrent workers matches the --workers flag value (e.g., via an atomic counter in a test-only worker function).

Previous run (2)

Review

Findings

Medium

[stale-default-value] docs/modules/ROOT/pages/verify-conforma-vsa-release-ta.adoc:55 — The WORKERS parameter default is documented as 4, but the verify-conforma-konflux-ta task YAML default was bumped from 4 to 5 in this PR. The sibling page verify-conforma-konflux-ta.adoc was correctly updated, but this page was missed.
Remediation: Update the WORKERS default from 4 to 5 in verify-conforma-vsa-release-ta.adoc to match the updated task YAML default, unless this task has its own separate WORKERS override.

Low

[default-consistency] cmd/validate/image.go:59 — The Go-code default for workers remains 5, but before the off-by-one fix the effective worker count was 6 (<= 5 spawns workers 0–5). The task YAML defaults were each bumped by 1 to preserve effective concurrency (4→5 and 1→2), yet the CLI's own default was not bumped (5→6). Users invoking the CLI directly will silently drop from 6 to 5 effective workers. This appears to be an intentional choice (the default was always meant to be 5), but is worth confirming.
[documentation-accuracy] tasks/verify-conforma-konflux-ta/0.1/README.md:34 — The README update replaces the old claim that WORKERS was unused ("All policy evaluations are run with 35 workers") with a note about managed release pipelines overriding via conforma.workerCount in konflux-release-data. This corrects stale documentation and is valuable cleanup, though it goes slightly beyond the minimal off-by-one fix scope.

Previous run (3)

Review

Findings

Low

[scope alignment] tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml:186 — The PR description only mentions fixing the off-by-one bug in the worker loop, but the PR also changes default WORKERS values across two task definitions and their documentation, and removes the "currently not used" note from the verify-conforma-konflux-ta README. These changes preserve the previous effective worker counts (since the old <= loop spawned N+1 workers for a configured value of N), but this rationale is not stated in the PR description.
Remediation: Update the PR description to explain that the default values are bumped by 1 to preserve the previous effective worker counts after the off-by-one fix.

Labels: Bug fix in CLI validate command with documentation updates

joejstuart · 2026-07-23T18:19:27Z

The change looks good, I'm just unsure of the impact to the users that modify the workers outside of the task (ie: in konflux-release-data). All configs that modify the count are set to 35, which means we're only taking away 1 worker, so it should be negligible. They could see a little slower runtimes though. In theory it should slow a 10 minute Conforma run by 17 seconds.
The alternative is we live with this bug.

fullsend-ai-review · 2026-07-23T20:26:33Z

🤖 Finished Review · ❌ Failure · Started 8:26 PM UTC · Completed 8:40 PM UTC
Commit: 87c4a29 · View workflow run →

An off-by-one fix in conforma/cli (conforma/cli#3425) corrected the worker loop from <= to <, reducing actual worker count by 1. Bump the fallback default to preserve the effective concurrency users had before the fix. This only affects releases where conforma.workerCount is not set in konflux-release-data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: cuipinghuo <chuo@redhat.com>

fullsend-ai-review · 2026-08-13T18:54:29Z

🤖 Finished Review · ✅ Success · Started 6:54 PM UTC · Completed 7:08 PM UTC

Commit: 87c4a29 · View workflow run →

simonbaird · 2026-08-13T20:59:42Z

Rebase should fix the EC violation now that #3496 is merged.

An off-by-one fix in conforma/cli (conforma/cli#3425) corrected the worker loop from <= to <, reducing actual worker count by 1. Bump the default to preserve the effective concurrency users had before the fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The worker loop used <= instead of <, spawning numWorkers+1 goroutines instead of the intended numWorkers. Bump task defaults to preserve the effective concurrency users had before the fix: - verify-conforma-konflux-ta: 4 → 5 - verify-enterprise-contract: 1 → 2 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fullsend-ai-review · 2026-08-17T13:19:37Z

🤖 Finished Review · ✅ Success · Started 1:19 PM UTC · Completed 1:34 PM UTC

Commit: 87c4a29 · View workflow run →

github-actions Bot added the size: XS label Jul 20, 2026

jsmid1 previously approved these changes Jul 23, 2026

View reviewed changes

cuipinghuo dismissed jsmid1’s stale review via 44b18de July 23, 2026 14:06

coderabbitai Bot reviewed Jul 23, 2026

View reviewed changes

Comment thread tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml

cuipinghuo force-pushed the EC-1994 branch from 44b18de to 0dc5948 Compare July 23, 2026 14:15

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge bug Something isn't working cli labels Jul 23, 2026

simonbaird reviewed Jul 23, 2026

View reviewed changes

Comment thread tasks/verify-conforma-konflux-ta/0.1/README.md

cuipinghuo force-pushed the EC-1994 branch from 0dc5948 to bd52d43 Compare July 23, 2026 20:25

jsmid1 previously approved these changes Jul 28, 2026

View reviewed changes

BohdanMar previously approved these changes Aug 12, 2026

View reviewed changes

This was referenced Aug 13, 2026

fix(EC-1994): bump WORKERS default from 1 to 2 konflux-ci/build-definitions#3789

Merged

fix(EC-1994): bump conforma.workerCount fallback default from 4 to 5 konflux-ci/release-service-catalog#2463

Open

cuipinghuo dismissed stale reviews from BohdanMar and jsmid1 via ec4d985 August 13, 2026 18:53

cuipinghuo force-pushed the EC-1994 branch from bd52d43 to ec4d985 Compare August 13, 2026 18:53

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed ready-for-merge All reviewers approved — ready to merge labels Aug 13, 2026

simonbaird approved these changes Aug 13, 2026

View reviewed changes

cuipinghuo force-pushed the EC-1994 branch from ec4d985 to bff1983 Compare August 17, 2026 13:18

fullsend-ai-review Bot approved these changes Aug 17, 2026

View reviewed changes

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed requires-manual-review Review requires human judgment labels Aug 17, 2026

Uh oh!

Conversation

cuipinghuo commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

coderabbitai Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fullsend-ai-review Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Low

Uh oh!

This comment was marked as outdated.

Uh oh!

joejstuart commented Jul 23, 2026

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

simonbaird commented Aug 13, 2026

Uh oh!

fullsend-ai-review Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cuipinghuo commented Jul 20, 2026 •

edited

Loading

coderabbitai Bot commented Jul 20, 2026 •

edited

Loading

codecov Bot commented Jul 20, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jul 23, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jul 23, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jul 23, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jul 23, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 13, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 17, 2026 •

edited

Loading