fix: restore Dynaconf fresh vars support

[pdecat]

User description

The changes in PR https://github.com/qodo-ai/pr-agent/pull/2087 which are included in v0.31 prevent Dynaconf fresh vars from working properly.

For context, here's my setup with the codiumai/pr-agent:0.31-gitlab_webhook image:

In kubernetes pod, this environment variable is set:

FRESH_VARS_FOR_DYNACONF='["GITLAB"]'

Note: that this has to be uppercase as mentioned here.

In /app/pr_agent/settings_prod/.secrets.toml which is mounted from a kubernetes secret:

[gitlab]
personal_access_token = "xxx"
shared_secret = "xxx"

With v0.30, every time the secret is updated (every day in our case as we use short lived Gitlab tokens), the file is refreshed, and Dynaconf picks the changes perfectly without restarting neither the pod nor the pr-agent process.

The first commit in this PR only introduces unit tests to demonstrate the issue in Github workflow execution.

The fix is in second commit pushed after the first workflow execution: https://github.com/qodo-ai/pr-agent/pull/2104/commits/312134ae97825dd5c689b75dc2aa3c0e2ab16763

---

PR Type

Tests, Bug fix

---

Description

• Add comprehensive unit tests for Dynaconf fresh_vars functionality

• Tests verify fresh_vars works with custom_merge_loader configuration

• Detect regression where fresh_vars stopped working after PR #2087

• Cover GitLab credentials reload scenario with file modifications

---

Diagram Walkthrough

flowchart LR
  A["Test Suite"] --> B["Fresh Vars Functionality"]
  B --> C["GitLab Scenario Tests"]
  B --> D["Custom Loader Integration"]
  B --> E["Basic Functionality Tests"]
  C --> F["Token Reload Detection"]
  D --> G["Core Loaders Disabled"]
  E --> H["Environment Variable Config"]

File Walkthrough

Relevant files

Tests

test_fresh_vars_functionality.py Comprehensive fresh_vars functionality test suite                tests/unittest/test_fresh_vars_functionality.py Add 520 lines of comprehensive unit tests for Dynaconf fresh_vars feature Create three test classes covering GitLab credentials scenario, custom loader integration, and basic functionality Include critical test test_fresh_vars_without_core_loaders to detect if fresh_vars is broken with custom_merge_loader Test file modification detection and value reloading with multiple scenarios and edge cases +520/-0

---

[qodo-free-for-open-source-projects[bot]]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Consistent Naming Conventions Objective: All new variables, functions, and classes must follow the project's established naming standards Status: Passed
	Single Responsibility for Functions Objective: Each function should have a single, well-defined responsibility Status: Passed
	When relevant, utilize early return Objective: In a code snippet containing multiple logic conditions (such as 'if-else'), prefer an early return on edge cases than deep nesting Status: Passed
🔴	No Dead or Commented-Out Code Objective: Keep the codebase clean by ensuring all submitted code is active and necessary Status: Commented-out code present: Line 42 contains commented-out code os.utime(file_path, (new_time, new_time)) that should be removed or uncommented if needed. Referred Code # os.utime(file_path, (new_time, new_time))
⚪	Robust Error Handling Objective: Ensure potential errors and edge cases are anticipated and handled gracefully throughout the code Status: Missing error handling: File operations like write_text() and rmtree() lack error handling for potential I/O failures, though this may be acceptable in test code. Referred Code self.secrets_file.write_text(content)
[ ] Update <!-- /compliance --update_compliance=true -->

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[pdecat]

First test workflow execution:

=========================== short test summary info ============================
FAILED tests/unittest/test_fresh_vars_functionality.py::TestFreshVarsGitLabScenario::test_gitlab_personal_access_token_reload
FAILED tests/unittest/test_fresh_vars_functionality.py::TestFreshVarsGitLabScenario::test_gitlab_shared_secret_reload
FAILED tests/unittest/test_fresh_vars_functionality.py::TestFreshVarsGitLabScenario::test_gitlab_multiple_fields_reload
FAILED tests/unittest/test_fresh_vars_functionality.py::TestFreshVarsCustomLoaderIntegration::test_fresh_vars_without_core_loaders
FAILED tests/unittest/test_fresh_vars_functionality.py::TestFreshVarsCustomLoaderIntegration::test_custom_loader_respects_fresh_vars
FAILED tests/unittest/test_fresh_vars_functionality.py::TestFreshVarsBasicFunctionality::test_fresh_vars_from_environment_variable
FAILED tests/unittest/test_fresh_vars_functionality.py::TestFreshVarsBasicFunctionality::test_gitlab_credentials_not_cached_when_fresh
============ 7 failed, 192 passed, 6 skipped, 24 warnings in 5.71s =============

https://github.com/qodo-ai/pr-agent/actions/runs/19370299280/job/55424179340?pr=2104#step:5:579

[qodo-free-for-open-source-projects[bot]]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Use correct helper method for tests In TestFreshVarsGitLabScenario, replace calls to the global create_dynaconf_with_custom_loader with the class method self.create_dynaconf_with_custom_loader to prevent circular import errors. tests/unittest/test_fresh_vars_functionality.py [150-168] def test_gitlab_personal_access_token_reload(self): """ Test that gitlab.personal_access_token is reloaded when marked as fresh. This is the critical test for the user's use case. It verifies that: 1. Initial value is loaded correctly 2. After modifying the file, the new value is returned (not cached) 3. This works with the custom_merge_loader """ # Create initial secrets file self.create_secrets_toml(personal_access_token="token_v1", shared_secret="secret_v1") # Create Dynaconf with GITLAB marked as fresh - settings = create_dynaconf_with_custom_loader(self.temp_dir, self.secrets_file, fresh_vars=["GITLAB"]) + settings = self.create_dynaconf_with_custom_loader(fresh_vars=["GITLAB"]) # First access - should return initial value first_token = settings.GITLAB.PERSONAL_ACCESS_TOKEN assert first_token == "token_v1", "Initial personal_access_token should be 'token_v1'" ... [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical bug where tests in TestFreshVarsGitLabScenario call a global helper instead of the required class method, which would lead to test failures due to circular imports.	High
	Fix broken file timestamp update Uncomment the os.utime(...) call in the ensure_file_timestamp_changes function to ensure it correctly updates file timestamps, which is critical for the reliability of the configuration reloading tests. tests/unittest/test_fresh_vars_functionality.py [28-44] def ensure_file_timestamp_changes(file_path): """ Force file timestamp to change by using os.utime with explicit time. This is much faster than sleeping and works on all filesystems. Args: file_path: Path to the file to update """ # Get current time and add a small increment to ensure it's different current_time = time.time() new_time = current_time + 0.001 # Add 1ms # Set both access time and modification time - # os.utime(file_path, (new_time, new_time)) + os.utime(file_path, (new_time, new_time)) [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that the os.utime call is commented out, making the ensure_file_timestamp_changes function ineffective and potentially causing the tests to be unreliable.	High
Learned best practice	Add exception handling in teardown Wrap the cleanup operation in a try-except block to handle potential permission or file system errors gracefully, preventing test teardown failures from masking actual test failures. tests/unittest/test_fresh_vars_functionality.py [93-98] def teardown_method(self): """Clean up temporary files after each test.""" import shutil if hasattr(self, "temp_dir") and Path(self.temp_dir).exists(): - shutil.rmtree(self.temp_dir) + try: + shutil.rmtree(self.temp_dir) + except Exception as e: + # Log but don't fail teardown + print(f"Warning: Failed to clean up temp dir: {e}") [ ] Apply / Chat <!-- /improve --apply_suggestion=2 --> Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Improve exception handling by preserving context (raise from), capturing exceptions into variables, sanitizing logs to avoid secrets leakage, and converting returns of exception objects into proper raises.	Low
[ ] Update <!-- /improve_multi --more_suggestions=true -->

• [ ] Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

[pdecat]

Second test workflow with the fix:

================= 199 passed, 6 skipped, 24 warnings in 6.17s ==================

https://github.com/qodo-ai/pr-agent/actions/runs/19370355959/job/55424387444?pr=2104#step:5:302

[pdecat]

Note: I cannot check the boxes in the qodo-merge-for-open-source comment, they are disabled:

[maxdata]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Fresh Vars Matching The fresh_vars comparison uses case-insensitive matching via key.upper() == k.upper(). Confirm that this aligns with Dynaconf's expectations and does not unintentionally match similarly named keys differing only by case across environments or nested structures. # For fresh_vars support: key parameter is uppercase, but accumulated_data keys are lowercase if key is None or key.upper() == k.upper(): obj.set(k, v) Test Robustness Tests rely on os.utime with a 1ms increment to trigger reloads. On some filesystems timestamp granularity may be coarser; consider increasing the delta or adding a fallback sleep to avoid flakes in CI. def ensure_file_timestamp_changes(file_path): """ Force file timestamp to change by using os.utime with explicit time. This is much faster than sleeping and works on all filesystems. Args: file_path: Path to the file to update """ # Get current time and add a small increment to ensure it's different current_time = time.time() new_time = current_time + 0.001 # Add 1ms # Set both access time and modification time os.utime(file_path, (new_time, new_time)) Env Var Scope Only one test sets FRESH_VARS_FOR_DYNACONF via patch.dict; ensure no leakage between tests and consider adding a negative test confirming behavior when the env var is absent or malformed. # Set FRESH_VARS_FOR_DYNACONF environment variable with patch.dict(os.environ, {"FRESH_VARS_FOR_DYNACONF": '["GITLAB"]'}): # Create Dynaconf (should pick up fresh_vars from env) settings = create_dynaconf_with_custom_loader(self.temp_dir, self.secrets_file) # First access first_value = settings.GITLAB.PERSONAL_ACCESS_TOKEN assert first_value == "env_token_v1" # Modify file ensure_file_timestamp_changes(self.secrets_file) self.create_secrets_toml(personal_access_token="env_token_v2") # Second access - should be reloaded second_value = settings.GITLAB.PERSONAL_ACCESS_TOKEN assert second_value == "env_token_v2", "fresh_vars from environment variable should cause reload" assert first_value != second_value, "Values should be different when fresh_vars is set via env var"

[ofir-frd]

@sharoneyal please have a look here.

[Abdulwahid84]

🤖 Automated PR Review

Summary

✅ Code review complete. Analyzed 2 files. No major issues found. The code looks good!

✅ No issues found!

---

Generated by AI-powered PR Review Agent

[Abdulwahid84]

🤖 Automated PR Review

Summary

✅ Code Review Complete PR #2104: fix: restore Dynaconf fresh vars support Analyzed: 2 files Issues Found: 0 total ✅ No major issues detected. Code looks good!

Detailed Findings

📄 pr_agent/custom_merge_loader.py

📄 tests/unittest/test_fresh_vars_functionality.py

---

Generated by AI-powered PR Review Agent

[Abdulwahid84]

🤖 Automated PR Review

Summary

✅ Code Review Complete PR #2104: fix: restore Dynaconf fresh vars support Analyzed: 2 files Issues Found: 0 total ✅ No major issues detected. Code looks good!

Detailed Findings

📄 pr_agent/custom_merge_loader.py

📄 tests/unittest/test_fresh_vars_functionality.py

---

Generated by AI-powered PR Review Agent

[Abdulwahid84]

🤖 Automated PR Review

Summary

✅ Code Review Complete PR #2104: fix: restore Dynaconf fresh vars support Analyzed: 2 files Issues Found: 0 total ✅ No major issues detected. Code looks good!

Detailed Findings

📄 pr_agent/custom_merge_loader.py

📄 tests/unittest/test_fresh_vars_functionality.py

---

Generated by AI-powered PR Review Agent

[Abdulwahid84]

🤖 Automated PR Review

Summary

✅ Code Review Complete PR #2104: fix: restore Dynaconf fresh vars support Analyzed: 2 files Issues Found: 0 total ✅ No major issues detected. Code looks good!

Detailed Findings

📄 pr_agent/custom_merge_loader.py

📄 tests/unittest/test_fresh_vars_functionality.py

---

Generated by AI-powered PR Review Agent

[Abdulwahid84]

🤖 Automated PR Review

Summary

✅ Code Review Complete PR #2104: fix: restore Dynaconf fresh vars support Analyzed: 2 files Issues Found: 0 total ✅ No major issues detected. Code looks good!

Detailed Findings

📄 pr_agent/custom_merge_loader.py

📄 tests/unittest/test_fresh_vars_functionality.py

---

Generated by AI-powered PR Review Agent