Skip to content

fix(resolver): make resolution cache values immutable to prevent corr…#1095

Merged
mergify[bot] merged 1 commit intomainfrom
fix/immutable-resolution-cache
Apr 29, 2026
Merged

fix(resolver): make resolution cache values immutable to prevent corr…#1095
mergify[bot] merged 1 commit intomainfrom
fix/immutable-resolution-cache

Conversation

@smoparth
Copy link
Copy Markdown
Contributor

@smoparth smoparth commented Apr 29, 2026

fix(resolver): make resolution cache values immutable to prevent corruption

Store session-level cache values as tuples instead of lists so that callers cannot accidentally mutate shared cache state. Return defensive list copies from resolve() and resolve_versions() to keep the public API unchanged.

Add tests verifying that cached values are immutable and that repeated resolve() calls return independent lists.

Closes: #1021

Pull Request Description

What

Why

…uption

Store session-level cache values as tuples instead of lists so that
callers cannot accidentally mutate shared cache state. Return defensive
list copies from resolve() and resolve_versions() to keep the public
API unchanged.

Add tests verifying that cached values are immutable and that repeated
resolve() calls return independent lists.

Closes: #1021
Co-Authored-By: Claude <claude@anthropic.com>
Signed-off-by: Shanmukh Pawan <smoparth@redhat.com>
@smoparth smoparth requested a review from a team as a code owner April 29, 2026 13:01
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3712a92c-ea81-4a21-82be-c39d304d510d

📥 Commits

Reviewing files that changed from the base of the PR and between 478701b and ddd1ea1.

📒 Files selected for processing (3)
  • src/fromager/bootstrap_requirement_resolver.py
  • src/fromager/bootstrapper.py
  • tests/test_bootstrap_requirement_resolver.py

📝 Walkthrough

Walkthrough

The change converts the internal cache in BootstrapRequirementResolver from storing mutable lists to immutable tuples, preventing accidental corruption when callers modify returned values. The cache accessor and field type annotations are updated accordingly. When return_all_versions=True, callers convert the cached tuple back to a list to maintain existing return type contracts. Three new test cases validate cache immutability, isolation between calls, and proper caching behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: making resolution cache values immutable to prevent corruption.
Description check ✅ Passed The description is directly related to the changeset, explaining what was changed (cache values as tuples), why (prevent mutation of shared state), and how (defensive copies in public API).
Linked Issues check ✅ Passed The PR fully addresses issue #1021 by converting mutable list cache values to immutable tuples, preventing accidental corruption of shared cache state at the type level.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the cache immutability objective: tuple storage, defensive list returns, and validation tests with no extraneous modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@rd4398 rd4398 left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify mergify Bot merged commit a19dea7 into main Apr 29, 2026
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resolution cache values should be immutable to prevent accidental corruption

2 participants