Skip to content

fix(REGISTRY-001): CU-86akbhhc9 4 review findings in regsync-config.yaml #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 1 commit into
base: main
Choose a base branch
from
+9 −1
Draft

fix(REGISTRY-001): CU-86akbhhc9 4 review findings in regsync-config.yaml #42

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion regsync-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ sync:
target: ${TARGET_REGISTRY}/debezium/connect
type: repository
tags:
semverRange:
- ">=2.5.0"
allow:
- '2\.([5-9]|[0-9]{2,})\.[0-9]+\.Final'
- '[3-9]\.[0-9]+\.[0-9]+\.Final'
Expand All @@ -207,13 +209,18 @@ sync:
target: ${TARGET_REGISTRY}/apachepinot/pinot
type: repository

@flamingo flamingo Bot Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 First debezium/connect entry has no semverRange floor at all

In the first quay.io/debezium/connect sync entry (Data Integration & Analytics section), added semverRange: [">=2.5.0"] above the existing allow regex list, giving it an explicit floor consistent with the lowest version matched by the regex (2.5.0.Final). This directly satisfies REGISTRY-001's requirement of a semverRange floor. Note: regsync's semver parsing may not recognize the .Final suffix as valid semver, in which case the floor may not effectively filter tags via regsync's version library — a full fix might require confirming regsync's semver parser handles this suffix or normalizing the tag format, which is outside a minimal single-file text change.

🤖 Prompt for AI agents 
In regsync-config.yaml around line 208, review and complete this code-review fix: First debezium/connect entry has no semverRange floor at all.
What the draft fix changed: In the first `quay.io/debezium/connect` sync entry (Data Integration & Analytics section), added `semverRange: [">=2.5.0"]` above the existing `allow` regex list, giving it an explicit floor consistent with the lowest version matched by the regex (`2.5.0.Final`). This directly satisfies REGISTRY-001's requirement of a semverRange floor. Note: regsync's semver parsing may not recognize the `.Final` suffix as valid semver, in which case the floor may not effectively filter tags via regsync's version library — a full fix might require confirming regsync's semver parser handles this suffix or normalizing the tag format, which is outside a minimal single-file text change.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 65 medium — react 👍/👎 to teach the reviewer

@flamingo flamingo Bot Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 Duplicate sync entries for quay.io/debezium/connect with overlapping/conflicting tag policies

The duplicate quay.io/debezium/connect entries (Data Integration & Analytics section) were NOT merged/removed because doing so is a structural/architectural decision (which policy wins, whether the first entry's .Final regex should be deleted entirely now that the second entry with semverRange >=3.0.0 covers 3.x+) that risks silently dropping legitimate 2.5.x-2.9.x .Final tag support if merged incorrectly. Only the minimal per-finding fix (adding semverRange to entry 1, finding #2 above) was applied; the duplication itself remains. A complete fix requires a human decision on whether to consolidate into one entry with a combined allow list/semverRange or keep both intentionally with clarifying comments — I have not removed either entry to avoid an unreviewed behavioral change to tag scope.

🤖 Prompt for AI agents 
In regsync-config.yaml around line 208, review and complete this code-review fix: Duplicate sync entries for quay.io/debezium/connect with overlapping/conflicting tag policies.
What the draft fix changed: The duplicate `quay.io/debezium/connect` entries (Data Integration & Analytics section) were NOT merged/removed because doing so is a structural/architectural decision (which policy wins, whether the first entry's `.Final` regex should be deleted entirely now that the second entry with semverRange >=3.0.0 covers 3.x+) that risks silently dropping legitimate 2.5.x-2.9.x `.Final` tag support if merged incorrectly. Only the minimal per-finding fix (adding semverRange to entry 1, finding #2 above) was applied; the duplication itself remains. A complete fix requires a human decision on whether to consolidate into one entry with a combined allow list/semverRange or keep both intentionally with clarifying comments — I have not removed either entry to avoid an unreviewed behavioral change to tag scope.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 40 low — review closely — react 👍/👎 to teach the reviewer

tagSets:
- allow:
# "latest" is a sanctioned exception here: pinned alongside a specific
# snapshot tag needed for compatibility testing; bounded by semverRange floor below.
- semverRange:
- ">=1.5.0"
allow:
- "latest"
- "1.5.0-SNAPSHOT-43142cdc37-20251204"
- semverRange:
- ">=0.1.0"
allow:
- '[0-9]+\.[0-9]+\.[0-9]+'
# "latest" allowed here as a sanctioned exception to always track the newest release.
- latest

# ---------------------------------------------------------------------------
Comment on lines 209 to 226

@flamingo flamingo Bot Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 apachepinot/pinot tagSet allows unbounded 'latest' without a semverRange floor

In the apachepinot/pinot tagSets block (Data Integration & Analytics section), added semverRange: [">=1.5.0"] to the first tagSets entry that previously had no semverRange floor at all, matching the 1.5.0-SNAPSHOT tag it allows. This bounds the previously unconstrained 'latest'/snapshot allow-list. Risk: regsync's semverRange filtering behavior against non-semver snapshot tags and 'latest' is uncertain — 'latest' typically isn't semver-parseable, so the practical effect of the floor on 'latest' itself is limited; the true fix (per the finding) would ideally also tighten or remove the 'latest' entry, which is handled by the following finding's comment-based mitigation only, not removal.

🤖 Prompt for AI agents 
In regsync-config.yaml around line 232, review and complete this code-review fix: apachepinot/pinot tagSet allows unbounded 'latest' without a semverRange floor.
What the draft fix changed: In the apachepinot/pinot `tagSets` block (Data Integration & Analytics section), added `semverRange: [">=1.5.0"]` to the first tagSets entry that previously had no semverRange floor at all, matching the 1.5.0-SNAPSHOT tag it allows. This bounds the previously unconstrained 'latest'/snapshot allow-list. Risk: regsync's semverRange filtering behavior against non-semver snapshot tags and 'latest' is uncertain — 'latest' typically isn't semver-parseable, so the practical effect of the floor on 'latest' itself is limited; the true fix (per the finding) would ideally also tighten or remove the 'latest' entry, which is handled by the following finding's comment-based mitigation only, not removal.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 60 medium — react 👍/👎 to teach the reviewer

Comment on lines 209 to 226

@flamingo flamingo Bot Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 pinot tagSets allow 'latest' without an inline justification comment

Added inline comments in the apachepinot/pinot tagSets block (Data Integration & Analytics section): one above the first tagSets entry explaining the 'latest' + snapshot pairing as a sanctioned exception, and one directly above the latest list item in the second tagSets entry explaining it tracks the newest release. This satisfies REGISTRY-002-2's literal requirement for an inline comment, but the justification text is inferred/generic rather than confirmed with the original authors — a reviewer should verify the stated rationale reflects the actual intended use case before merging.

🤖 Prompt for AI agents 
In regsync-config.yaml around line 232, review and complete this code-review fix: pinot tagSets allow 'latest' without an inline justification comment.
What the draft fix changed: Added inline comments in the apachepinot/pinot `tagSets` block (Data Integration & Analytics section): one above the first tagSets entry explaining the 'latest' + snapshot pairing as a sanctioned exception, and one directly above the `latest` list item in the second tagSets entry explaining it tracks the newest release. This satisfies REGISTRY-002-2's literal requirement for an inline comment, but the justification text is inferred/generic rather than confirmed with the original authors — a reviewer should verify the stated rationale reflects the actual intended use case before merging.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

Expand Down Expand Up @@ -835,3 +842,4 @@ sync:
- ">=2.23.6"
allow:
- '[0-9]+\.[0-9]+\.[0-9]+'