Skip to content

Keep download location when central search api times out - #213

Open
bjk7119 wants to merge 4 commits into
mainfrom
error
Open

Keep download location when central search api times out#213
bjk7119 wants to merge 4 commits into
mainfrom
error

Conversation

@bjk7119

@bjk7119 bjk7119 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Improved dependency verification when Central Search is skipped or times out.
    • Repository checks now run correctly during fallback processing.
    • Added validation to prevent invalid or incomplete Maven coordinates from triggering Central repository lookups.
    • Central repository checks now use verified final coordinates, including coordinates discovered from project metadata.
    • Prevented unverified manifest metadata from affecting repository existence checks.

@bjk7119 bjk7119 self-assigned this Aug 6, 2026
@bjk7119 bjk7119 added the bug fix [PR] Fix the bug label Aug 6, 2026
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bfbe3ba6-1e84-4ea4-9a8e-43e5632631ea

📥 Commits

Reviewing files that changed from the base of the PR and between 7f5533e and 958d10d.

📒 Files selected for processing (1)
  • src/fosslight_binary/_jar_analysis.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/fosslight_binary/_jar_analysis.py

📝 Walkthrough

Walkthrough

The JAR analysis validates Maven coordinates and Central Search results. It tracks trusted coordinates from Central and JAR POM data, excludes manifest-derived coordinates from trust, and performs the final existence check after Central Search is skipped.

Changes

Central coordinate validation and fallback verification

Layer / File(s) Summary
Validate Maven coordinates before Central requests
src/fosslight_binary/_jar_analysis.py
Validates Maven coordinate tokens, rejects incomplete Central Search results, and blocks invalid coordinates before Central requests.
Verify trusted coordinates after skipped search
src/fosslight_binary/_jar_analysis.py
Renames the control parameter, tracks trusted Central and POM coordinates, clears trust for manifest-derived coordinates, and runs the final existence check after skipped search and retry fallback.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: dd-jy

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the timeout-related behavior addressed by the pull request.
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 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch error

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.

@bjk7119
bjk7119 requested a review from dd-jy August 6, 2026 08:26

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@src/fosslight_binary/_jar_analysis.py`:
- Around line 294-297: Move the _exists_in_central check in the JAR analysis
flow to after the _read_manifest_from_jar manifest fallback populates groupId,
artifactId, and version, or rerun it with those coordinates before the download
URL is generated. Preserve the existing Search API timeout behavior and add a
regression test covering a manifest-only JAR whose coordinates exist in Central.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 01358d8c-c939-45c4-8ca8-f80fcc575992

📥 Commits

Reviewing files that changed from the base of the PR and between ef52b85 and e7a0d4e.

📒 Files selected for processing (1)
  • src/fosslight_binary/_jar_analysis.py

Comment thread src/fosslight_binary/_jar_analysis.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@src/fosslight_binary/_jar_analysis.py`:
- Line 26: Update _read_manifest_from_jar and the Central existence-check flow
around _COORD_TOKEN so Bundle-Vendor, Implementation-Vendor,
Bundle-SymbolicName, and Implementation-Title are never used as trusted Maven
coordinates. Only enable groupId:artifactId:version matching when coordinates
come from pom.xml or a trusted SHA-1 search, while retaining manifest values
solely as display metadata.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b8970eec-1b0b-4eec-930e-5734ae556c4f

📥 Commits

Reviewing files that changed from the base of the PR and between e7a0d4e and 81edc6d.

📒 Files selected for processing (1)
  • src/fosslight_binary/_jar_analysis.py

Comment thread src/fosslight_binary/_jar_analysis.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@src/fosslight_binary/_jar_analysis.py`:
- Around line 321-323: In the manifest metadata fallback branch near
trusted_coordinates, also reset confirmed_in_central to False when replacing
POM-derived coordinates. Add a regression test covering an incomplete Central
response with token-shaped MANIFEST.MF fields, ensuring the flow does not skip
_exists_in_central or emit an invalid URL.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 967df92a-37e5-408d-a533-a93e86cf5ec2

📥 Commits

Reviewing files that changed from the base of the PR and between 81edc6d and 7f5533e.

📒 Files selected for processing (1)
  • src/fosslight_binary/_jar_analysis.py

Comment thread src/fosslight_binary/_jar_analysis.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug fix [PR] Fix the bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant