Skip to content

feat(config): add --extra_config_url to merge external .pr_agent.toml#2406

Open
kiennt2 wants to merge 7 commits into
The-PR-Agent:mainfrom
kiennt2:feature/extra-config
Open

feat(config): add --extra_config_url to merge external .pr_agent.toml#2406
kiennt2 wants to merge 7 commits into
The-PR-Agent:mainfrom
kiennt2:feature/extra-config

Conversation

@kiennt2
Copy link
Copy Markdown

@kiennt2 kiennt2 commented May 20, 2026

Motivation

.pr_agent.toml today is resolved from one of: the repo's default branch, the wiki, or a pr-agent-settings repo one level above the target repo. None of these scale cleanly to:

  • Deeply-nested GitLab subgroups. The docs explicitly note "the lookup for a pr-agent-settings repo will be only on one level above such repository" — so a single shared file can't be inherited by repos several subgroups deep.
  • Non-git hosting. Teams that want to publish shared defaults from a static site, an internal artifact server, or any HTTPS endpoint have no first-class way to feed it into PR-Agent.
  • CI-time control. No way to layer in defaults from the pipeline without committing a file to the target repo's default branch.

What this PR adds

A new CLI flag --extra_config_url (and matching PR_AGENT_EXTRA_CONFIG_URL env var / CONFIG.EXTRA_CONFIG_URL setting) that points to an additional .pr_agent.toml. The file is resolved at the start of apply_repo_settings()
and merged through the same secure loader used for repo-local config — before the git provider is constructed and before the repo-local file is applied. Repo-local config still overrides the extra file on conflicting
keys, so the new layer acts as defaults.

built-in defaults
  < --extra_config_url           ← new
    < global pr-agent-settings
      < local .pr_agent.toml
        < wiki .pr_agent.toml
          < environment variables

Accepted sources

  • https://… / http://… — fetched via stdlib urllib, written to a tempfile, cleaned up after merge
  • file:///path/to/shared.toml — local filesystem (non-localhost netloc preserved, percent-encoded paths URL-decoded)
  • Bare local path — treated as file://

Authentication for private endpoints

Optional PR_AGENT_EXTRA_CONFIG_AUTH_HEADER env var, formatted as <HeaderName>: <value>:

# GitLab PAT
export PR_AGENT_EXTRA_CONFIG_AUTH_HEADER="PRIVATE-TOKEN: <your-personal-access-token>"

# GitLab CI job token
export PR_AGENT_EXTRA_CONFIG_AUTH_HEADER="JOB-TOKEN: $CI_JOB_TOKEN"

# Generic bearer
export PR_AGENT_EXTRA_CONFIG_AUTH_HEADER="Authorization: Bearer <your-token>"

A malformed header (no :) is dropped and a warning is logged so misconfiguration is diagnosable rather than silent.

Usage

# Public URL
python -m pr_agent.cli \
  --pr_url=<MR/PR URL> \
  --extra_config_url=https://config.example.com/pr-agent/shared.toml \
  review

# Private GitLab API URL with JOB-TOKEN
python -m pr_agent.cli \
  --pr_url=<MR URL> \
  --extra_config_url="${CI_API_V4_URL}/projects/group%2Fpr-agent-shared/repository/files/.pr_agent.toml/raw?ref=main" \
  review

# Local file (dev / fallback after curl in before_script)
python -m pr_agent.cli --pr_url=... --extra_config_url=/tmp/shared.pr_agent.toml review

Security

The external file is loaded through the same custom_merge_loader.py used for repo-local config, so:

  • Forbidden directives (includes, preload, custom loaders, dynaconf_include, …) are rejected by validate_file_security exactly as for any other config source.
  • Only http, https, file schemes are accepted; anything else is dropped with a warning.
  • Responses are capped at 1 MB and the HTTP fetch uses a 10 s timeout.
  • The downloaded tempfile is removed in a finally block after the merge, success or failure.
  • A failed fetch is logged and PR-Agent continues with the remaining configuration layers — it never aborts the run.
  • Logs never carry secrets. URLs are sanitised before logging (userinfo and query string stripped), and post-merge logging emits only the names of merged sections — never values — so secrets in [openai], [gitlab], etc.
    can't leak into CI logs.
  • Inputs are validated at the boundary. A non-string CONFIG.EXTRA_CONFIG_URL is rejected with a warning rather than raising from urlparse.

Implementation notes

  • pr_agent/cli.py — registers --extra_config_url, defaults to the env var, stashes the value on CONFIG.EXTRA_CONFIG_URL so the loader picks it up.
  • pr_agent/settings/configuration.toml — declares extra_config_url="" under [config] so the option appears in the canonical list.
  • pr_agent/git_providers/utils.py — adds _safe_url_for_log (URL sanitiser), _resolve_extra_config_to_file(source) (URL/path → local file, handles file:// host + percent-decoding via url2pathname, HTTP(S) fetch with
    optional auth header and bounded size/timeout), and _apply_settings_from_file(path, label) (merges sections into get_settings(), includes the same Dynaconf TypeError fallback used by the repo-settings loader). Wired into
    apply_repo_settings() before the git provider is constructed so provider initialisers (e.g. GitLabProvider reading GITLAB.PERSONAL_ACCESS_TOKEN) see merged values, and before the repo-settings block so repo-local always
    overrides the extra file.
  • No new dependencies — only urllib and tempfile from stdlib.
  • Matches the existing Gitea precedent (GITEA.REPO_SETTING in gitea_provider.py), generalised across all providers.

Tests

tests/unittest/test_extra_config_url.py — 41 tests, all green:

  • Resolver (15): bare path / file:// / file://localhost/… / percent-encoded file:// / missing path / unsupported scheme / empty input / non-string input; HTTP fetch end-to-end against a real http.server thread (bound
    to port 0, no race); auth header injection from env var; missing-auth → 401 → skipped; HTTP 404 → skipped; >1 MB body → rejected; malformed auth header warns and proceeds; URL with embedded credentials never appears in any log
    line; _safe_url_for_log strips userinfo and query.
  • CLI parser (4): flag captured, env var fallback, neither set, flag overrides env.
  • Direct merge (6): new keys added; overlapping keys overwritten while non-overlapping keys in same section preserved; missing path is a no-op; invalid TOML is a no-op; secret-shaped values from a merged file never appear in
    the post-merge log line; loader falls back when Dynaconf rejects the hardened kwargs.
  • End-to-end precedence via apply_repo_settings (6) with a _FakeGitProvider: repo-local wins over extra on shared keys; extra-only keys survive; repo-only keys survive; works with empty repo settings; unreachable extra URL
    doesn't block repo settings; extra config is merged before the git provider is constructed.

Run:

pytest tests/unittest/test_extra_config_url.py -v
# 41 passed

Docs

docs/docs/usage-guide/configuration_options.md — adds an External configuration URL section covering usage, accepted source shapes, auth header, full precedence chain, and security/limits. Bumps the intro list from 3 → 4
mechanisms and extends the precedence sentence.

Backwards compatibility

Fully backwards compatible. With no --extra_config_url and no PR_AGENT_EXTRA_CONFIG_URL/CONFIG.EXTRA_CONFIG_URL set, apply_repo_settings() behaves exactly as before.

Changed files

File +/-
pr_agent/cli.py +14
pr_agent/git_providers/utils.py +187
pr_agent/settings/configuration.toml +1
docs/docs/usage-guide/configuration_options.md +72 / -2
tests/unittest/test_extra_config_url.py +666 (new)

@github-actions github-actions Bot added the feature 💡 label May 20, 2026
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add external configuration URL support for shared defaults

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add --extra_config_url CLI flag and env var for external config merging
• Support HTTP(S) URLs, file:// paths, and local filesystem sources
• Implement secure config loading with 1 MB size limit and 10s timeout
• Maintain proper precedence: external config < repo-local < wiki < env vars
• Add comprehensive test suite covering resolver, CLI, and precedence scenarios
Diagram
flowchart LR
  CLI["CLI Flag<br/>--extra_config_url"]
  ENV["Environment Var<br/>PR_AGENT_EXTRA_CONFIG_URL"]
  RESOLVE["_resolve_extra_config_to_file<br/>HTTP/File/Local"]
  APPLY["_apply_settings_from_file<br/>Merge via custom_merge_loader"]
  REPO["Repo-local<br/>.pr_agent.toml"]
  FINAL["Final Settings<br/>Repo overrides External"]
  
  CLI --> RESOLVE
  ENV --> RESOLVE
  RESOLVE --> APPLY
  APPLY --> FINAL
  REPO --> FINAL
Loading

Grey Divider

File Changes

1. pr_agent/cli.py ✨ Enhancement +12/-0

Register CLI flag for external config URL

• Added --extra_config_url argument to CLI parser with env var fallback
• Argument accepts HTTP(S) URLs, file:// paths, or local filesystem paths
• Stores resolved URL in CONFIG.EXTRA_CONFIG_URL for downstream processing
• Includes comprehensive help text documenting auth header support

pr_agent/cli.py


2. pr_agent/git_providers/utils.py ✨ Enhancement +112/-0

Implement external config resolution and merging logic

• Added _resolve_extra_config_to_file() to fetch/resolve config from URL or local path
• Supports HTTP(S) with optional auth header from PR_AGENT_EXTRA_CONFIG_AUTH_HEADER env var
• Enforces 1 MB size limit and 10-second timeout for HTTP requests
• Returns tuple of (path, is_temp) to track cleanup responsibility
• Added _apply_settings_from_file() to merge external config using secure custom_merge_loader
• Modified apply_repo_settings() to apply external config before repo-local config
• Ensures temp files are cleaned up in finally block regardless of success/failure

pr_agent/git_providers/utils.py


3. tests/unittest/test_extra_config_url.py 🧪 Tests +450/-0

Add comprehensive test coverage for external config feature

• Comprehensive test suite with 450+ lines covering resolver, CLI, and precedence
• Tests local file resolution (bare path and file:// URLs)
• Tests HTTP(S) fetching with auth header injection and error handling
• Tests size limit enforcement (1 MB cap) and timeout behavior
• Tests CLI parser with flag, env var, and precedence scenarios
• Tests settings merge with overlapping keys and section-level preservation
• Tests end-to-end precedence: external config < repo-local config
• Includes fixtures for temporary files and mock HTTP server

tests/unittest/test_extra_config_url.py


View more (1)
4. docs/docs/usage-guide/configuration_options.md 📝 Documentation +70/-2

Document external configuration URL feature and precedence

• Updated precedence description to include external configuration URL layer
• Added new "External configuration URL" section documenting feature
• Documented usage with CLI flag and environment variable
• Provided examples for HTTP(S), file://, and local path sources
• Documented authentication via PR_AGENT_EXTRA_CONFIG_AUTH_HEADER with examples
• Explained precedence chain and security/limit guarantees
• Clarified use cases: nested subgroups, non-git hosting, CI-time control

docs/docs/usage-guide/configuration_options.md


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 20, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0)

Grey Divider


Action required

1. Env precedence overwritten ✓ Resolved 🐞 Bug ≡ Correctness
Description
_apply_settings_from_file() overwrites existing settings keys with values from the extra config and
writes them back into the global Dynaconf singleton, so values originally loaded from environment
variables can be replaced and will not be restored. This violates the documented/implemented
precedence expectation that env vars take precedence and can cause env-provided secrets (tokens/API
keys) to be silently ignored in favor of the external .toml.
Code

pr_agent/git_providers/utils.py[R184-188]

+            section_dict = copy.deepcopy(get_settings().as_dict().get(section, {}))
+            for key, value in contents.items():
+                section_dict[key] = value
+            get_settings().unset(section)
+            get_settings().set(section, section_dict, merge=False)
Evidence
The settings singleton is created with env_loader specifically so env vars take precedence; however,
the new extra-config merge overwrites keys in the existing singleton without re-running env loading,
so env-derived values can be replaced despite the documented precedence chain placing env vars last
(highest priority).

pr_agent/config_loader.py[12-16]
pr_agent/git_providers/utils.py[180-189]
docs/docs/usage-guide/configuration_options.md[148-155]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_apply_settings_from_file()` merges extra-config values by copying the current section dict and then overwriting keys with values from the file before setting the section back into the global Dynaconf singleton. Because env vars are loaded into the singleton up-front, this merge can overwrite env-sourced values (including secrets), breaking the documented precedence where env vars should win.
### Issue Context
- `config_loader.py` explicitly enables `dynaconf.loaders.env_loader` “to take precedence”.
- Docs added in this PR show environment variables as the highest-precedence layer.
### Fix Focus Areas
- Re-apply environment-variable overrides after applying `--extra_config_url` (and ideally after other config-file merges) so env vars remain the highest-precedence source.
- Ensure this re-application happens **before** constructing the git provider (since provider `__init__` may read auth settings).
**Code pointers**
- pr_agent/git_providers/utils.py[184-188]
- pr_agent/git_providers/utils.py[201-229]
- pr_agent/config_loader.py[12-16]
- docs/docs/usage-guide/configuration_options.md[148-155]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. AUTO_CAST_FOR_DYNACONF not restored ✓ Resolved 📘 Rule violation ☼ Reliability
Description
New unit tests call apply_repo_settings(), which mutates the process environment by setting
AUTO_CAST_FOR_DYNACONF, but the test sandbox fixture does not snapshot/restore this env var. This
can leak global state across tests and cause order-dependent failures.
Code

tests/unittest/test_extra_config_url.py[R296-320]

+@pytest.fixture
+def settings_sandbox():
+    """Snapshot a few settings keys/sections, yield, restore on teardown."""
+    settings = get_settings()
+    saved = {}
+    for section, key in _TEST_KEYS_TO_RESTORE:
+        if key is None:
+            saved[section] = settings.as_dict().get(section, None)
+        else:
+            saved[(section, key)] = settings.get(f"{section}.{key}", None)
+    try:
+        yield settings
+    finally:
+        # Restore sections/keys to pre-test state
+        for section, key in _TEST_KEYS_TO_RESTORE:
+            if key is None:
+                settings.unset(section)
+                if saved[section] is not None:
+                    settings.set(section, saved[section], merge=False)
+            else:
+                val = saved[(section, key)]
+                if val is None:
+                    settings.unset(f"{section}.{key}")
+                else:
+                    settings.set(f"{section}.{key}", val)
Evidence
PR Compliance ID 15 requires tests to explicitly control and restore environment/global state. The
new tests invoke apply_repo_settings() without restoring the environment, while
apply_repo_settings() sets AUTO_CAST_FOR_DYNACONF, meaning the test suite's global env can be
altered after these tests run.

tests/unittest/test_extra_config_url.py[296-320]
tests/unittest/test_extra_config_url.py[460-531]
pr_agent/git_providers/utils.py[201-203]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new tests call `apply_repo_settings()`, which sets `os.environ["AUTO_CAST_FOR_DYNACONF"] = "false"`. The `settings_sandbox` fixture restores Dynaconf settings only, so the env var can remain modified for subsequent tests, causing flakiness.
## Issue Context
`get_settings()` is a process-wide singleton and environment variables are also process-global. Tests that change either should restore prior state to avoid cross-test pollution.
## Fix Focus Areas
- tests/unittest/test_extra_config_url.py[296-320]
- tests/unittest/test_extra_config_url.py[460-531]
- pr_agent/git_providers/utils.py[201-203]
## Suggested approach
- In `settings_sandbox`, snapshot the prior value of `AUTO_CAST_FOR_DYNACONF` (including the case where it is unset) before yielding.
- In the `finally:` block, restore the previous value (or `del os.environ["AUTO_CAST_FOR_DYNACONF"]` if it was unset).
- Alternatively, add a `monkeypatch`-based fixture used by the `apply_repo_settings(...)` tests to ensure env restoration.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Single quotes in new code ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
New Python changes introduce single-quoted string literals (e.g., CLI help= text and resolver
constants/dicts), which violates the repo’s Ruff style requirement to prefer double quotes and may
fail linting/pre-commit.
Code

pr_agent/cli.py[R55-63]

+    parser.add_argument(
+        '--extra_config_url',
+        type=str,
+        default=os.environ.get('PR_AGENT_EXTRA_CONFIG_URL'),
+        help='URL or local path of an additional .pr_agent.toml to merge before the '
+             'repo-local config (e.g. shared/org defaults). Accepts http(s):// URLs or '
+             'a filesystem path. For private endpoints, set PR_AGENT_EXTRA_CONFIG_AUTH_HEADER '
+             '(e.g. "PRIVATE-TOKEN: xxx" or "JOB-TOKEN: $CI_JOB_TOKEN"). '
+             'Repo-local .pr_agent.toml overrides values set here.'
Evidence
PR Compliance ID 8 requires Python changes to follow Ruff/isort style including preferring double
quotes. The added CLI argument help= text uses single quotes, and the new resolver code also
introduces multiple single-quoted literals (e.g., scheme handling and headers dict).

AGENTS.md
pr_agent/cli.py[55-63]
pr_agent/git_providers/utils.py[62-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New/modified Python code uses single-quoted string literals, but the repo compliance requires Ruff/isort conventions including preferring double quotes.
## Issue Context
The CLI flag definition and extra-config resolver add several single-quoted literals (including multi-line `help=` fragments and dict keys/values). This may break style checks.
## Fix Focus Areas
- pr_agent/cli.py[55-63]
- pr_agent/git_providers/utils.py[62-78]
- pr_agent/git_providers/utils.py[100-103]
- pr_agent/git_providers/utils.py[119-123]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (6)
4. EXTRA_CONFIG_URL missing configuration.toml ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The PR introduces the CONFIG.EXTRA_CONFIG_URL setting, but it is not added to
pr_agent/settings/configuration.toml, which is documented as containing all available
configuration options. This creates config/documentation drift for the new behavior.
Code

pr_agent/cli.py[R55-64]

+    parser.add_argument(
+        '--extra_config_url',
+        type=str,
+        default=os.environ.get('PR_AGENT_EXTRA_CONFIG_URL'),
+        help='URL or local path of an additional .pr_agent.toml to merge before the '
+             'repo-local config (e.g. shared/org defaults). Accepts http(s):// URLs or '
+             'a filesystem path. For private endpoints, set PR_AGENT_EXTRA_CONFIG_AUTH_HEADER '
+             '(e.g. "PRIVATE-TOKEN: xxx" or "JOB-TOKEN: $CI_JOB_TOKEN"). '
+             'Repo-local .pr_agent.toml overrides values set here.'
+    )
Evidence
PR Compliance ID 15 requires keeping prompt/config mirrors in sync when behavior/configuration
changes. This PR adds the new --extra_config_url flag and reads CONFIG.EXTRA_CONFIG_URL, but
pr_agent/settings/configuration.toml (the repo’s enumerated config-options file) does not include
an extra_config_url option under [config].

AGENTS.md
pr_agent/cli.py[55-64]
pr_agent/git_providers/utils.py[173-178]
pr_agent/settings/configuration.toml[1-45]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new configuration surface (`CONFIG.EXTRA_CONFIG_URL` / `--extra_config_url`) was added, but the canonical configuration options list (`pr_agent/settings/configuration.toml`) was not updated.
## Issue Context
`pr_agent/settings/configuration.toml` states it contains all available configuration options. Without documenting the new key there, users relying on that file will not discover the option and mirrors will drift.
## Fix Focus Areas
- pr_agent/cli.py[55-64]
- pr_agent/git_providers/utils.py[173-178]
- pr_agent/settings/configuration.toml[1-45]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Tests pollute global settings ✓ Resolved 🐞 Bug ☼ Reliability
Description
test_apply_settings_file_does_not_log_secret_values() writes TOML sections named [gitlab] and
[openai] and applies them to the process-wide Dynaconf singleton, but settings_sandbox only
snapshots/restores CONFIG.EXTRA_CONFIG_URL and the bespoke test section. This leaves mutated
GITLAB/OPENAI settings behind for later tests, creating order-dependent failures/flakiness.
Code

tests/unittest/test_extra_config_url.py[R366-372]

+    path = _write_toml(tmp_path, "extra.toml", f"""
+[gitlab]
+personal_access_token = "{secret_token}"
+
+[openai]
+key = "{openai_secret}"
+""")
Evidence
The sandbox fixture explicitly restores only CONFIG.EXTRA_CONFIG_URL and the bespoke test section,
but the secret-logging test applies settings under the real [gitlab] and [openai] sections;
those changes are therefore not reverted.

tests/unittest/test_extra_config_url.py[274-277]
tests/unittest/test_extra_config_url.py[350-372]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New tests modify real configuration sections (`[gitlab]`, `[openai]`) in the global settings singleton via `_apply_settings_from_file()`, but the `settings_sandbox` fixture does not restore those sections. This can leak state between tests and make the suite order-dependent.
### Issue Context
`get_settings()` returns a process-wide singleton in unit tests (no per-test isolation). Any modifications must be reverted.
### Fix Focus Areas
- tests/unittest/test_extra_config_url.py[274-305]
- tests/unittest/test_extra_config_url.py[350-396]
### Fix options
1) **Preferred:** Change the secret-logging test to use bespoke section names (not `[gitlab]` / `[openai]`) while still asserting secret values never appear in logs.
2) Alternatively, expand `_TEST_KEYS_TO_RESTORE` to snapshot/restore the additional sections mutated by tests (at least `("GITLAB", None)` and `("OPENAI", None)`), and restore any other sections the file touches.
### Acceptance criteria
- After the test, `get_settings().as_dict()` for `GITLAB`/`OPENAI` is identical to pre-test state.
- Tests remain independent of execution order.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. extra_source type not validated ✓ Resolved 📘 Rule violation ☼ Reliability
Description
apply_repo_settings() passes CONFIG.EXTRA_CONFIG_URL directly into
_resolve_extra_config_to_file() without validating/coercing it to a string, which can raise an
unhandled exception (e.g., if configured as a non-string in TOML). This violates the requirement to
validate/normalize config inputs at boundaries and can abort the run.
Code

pr_agent/git_providers/utils.py[R121-125]

+    # Apply external/shared config first so repo-local .pr_agent.toml overrides it.
+    extra_source = get_settings().get("CONFIG.EXTRA_CONFIG_URL", None)
+    if extra_source:
+        extra_path, extra_is_temp = _resolve_extra_config_to_file(extra_source)
+        if extra_path:
Evidence
The checklist requires validating/normalizing config inputs at boundaries with safe defaults. The
new code retrieves CONFIG.EXTRA_CONFIG_URL and calls _resolve_extra_config_to_file(extra_source)
without checking it is a str, while _resolve_extra_config_to_file() immediately calls
urlparse(source) and will error on non-string inputs.

pr_agent/git_providers/utils.py[121-125]
pr_agent/git_providers/utils.py[19-37]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CONFIG.EXTRA_CONFIG_URL` is assumed to be a string. If it is set via a config layer (TOML/wiki) to a non-string type (int/list/etc.), `urlparse(source)` in `_resolve_extra_config_to_file()` can raise and crash `apply_repo_settings()`.
## Issue Context
This value is a configuration input and should be validated/normalized at the boundary. Invalid values should result in a targeted warning and a safe no-op (skip extra config) rather than an exception.
## Fix Focus Areas
- pr_agent/git_providers/utils.py[121-125]
- pr_agent/git_providers/utils.py[19-37]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Extra config applied after provider ✓ Resolved 🐞 Bug ≡ Correctness
Description
apply_repo_settings() constructs the Git provider before applying the extra config, so
provider-critical settings (e.g., GITLAB.PERSONAL_ACCESS_TOKEN) from the external .toml cannot
influence provider initialization and can cause early failures.
Code

pr_agent/git_providers/utils.py[R117-124]

def apply_repo_settings(pr_url):
os.environ["AUTO_CAST_FOR_DYNACONF"] = "false"
git_provider = get_git_provider_with_context(pr_url)
+
+    # Apply external/shared config first so repo-local .pr_agent.toml overrides it.
+    extra_source = get_settings().get("CONFIG.EXTRA_CONFIG_URL", None)
+    if extra_source:
+        extra_path, extra_is_temp = _resolve_extra_config_to_file(extra_source)
Evidence
apply_repo_settings creates git_provider first, then applies the extra config. GitLabProvider
reads GITLAB.PERSONAL_ACCESS_TOKEN during initialization and raises if absent, so an extra config
intended to provide that token cannot take effect before the provider is constructed.

pr_agent/git_providers/utils.py[117-126]
pr_agent/git_providers/gitlab_provider.py[32-40]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`apply_repo_settings()` instantiates the git provider (`get_git_provider_with_context(pr_url)`) before resolving/merging `CONFIG.EXTRA_CONFIG_URL`. This prevents the external config from supplying settings required during provider initialization (e.g., tokens/URLs), and may cause runs to fail before the extra config is ever applied.
### Issue Context
This is observable with providers like GitLab, which raise immediately if `GITLAB.PERSONAL_ACCESS_TOKEN` is missing during `__init__`.
### Fix Focus Areas
- pr_agent/git_providers/utils.py[117-136]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Extra config URL leaks to logs ✓ Resolved 🐞 Bug ⛨ Security
Description
_resolve_extra_config_to_file() logs the full extra-config URL on success and error paths, which can
leak credentials if they are embedded in the URL (userinfo or query tokens).
Code

pr_agent/git_providers/utils.py[R58-73]

+        req = Request(source, headers=headers, method='GET')
+        with urlopen(req, timeout=_FETCH_TIMEOUT_SECONDS) as resp:
+            data = resp.read(_MAX_EXTRA_CONFIG_BYTES + 1)
+        if len(data) > _MAX_EXTRA_CONFIG_BYTES:
+            get_logger().warning(
+                f"Extra config exceeds {_MAX_EXTRA_CONFIG_BYTES} bytes, skipping: {source}"
+            )
+            return None, False
+        fd, tmp_path = tempfile.mkstemp(suffix='.toml')
+        with os.fdopen(fd, 'wb') as f:
+            f.write(data)
+        get_logger().info(f"Fetched extra config from {source} ({len(data)} bytes)")
+        return tmp_path, True
+    except Exception as e:
+        get_logger().warning(f"Failed to fetch extra config from {source}: {e}")
+        return None, False
Evidence
The function interpolates {source} directly into INFO/WARN logs for oversized responses,
successful fetch, and exceptions, which would emit any embedded credentials verbatim.

pr_agent/git_providers/utils.py[61-73]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_resolve_extra_config_to_file()` logs the full `source` URL in both success and failure messages. If `--extra_config_url` includes credentials (e.g., `https://user:pass@...` or `...?token=...`), those secrets will be written to logs.
### Issue Context
This code runs in CI/server contexts where logs are often retained and broadly accessible.
### Fix Focus Areas
- pr_agent/git_providers/utils.py[57-73]
### Implementation notes
Create a helper to produce a safe-to-log URL (e.g., `scheme://netloc/path` with userinfo removed and query/fragment dropped), and use that in all log lines instead of the raw `source`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. Logs full settings dict ✓ Resolved 📘 Rule violation ⛨ Security
Description
_apply_settings_from_file() logs new_settings.as_dict() at INFO, which can expose secrets (API
keys/tokens) stored in external or repo-local .toml configuration such as .pr_agent.toml. This
creates a credential-leak risk in CI logs and shared log aggregation backends, especially given the
settings schema includes explicit secret fields (e.g., openai.key,
gitlab.personal_access_token).
Code

pr_agent/git_providers/utils.py[R96-105]

+        for section, contents in new_settings.as_dict().items():
+            if not contents:
+                continue
+            section_dict = copy.deepcopy(get_settings().as_dict().get(section, {}))
+            for key, value in contents.items():
+                section_dict[key] = value
+            get_settings().unset(section)
+            get_settings().set(section, section_dict, merge=False)
+        get_logger().info(f"Applied {label} settings from {path}:\n{new_settings.as_dict()}")
+    except Exception as e:
Evidence
PR Compliance ID 8 prohibits exposing secrets/tokens in committed code paths, and the cited code
logs the entire merged/resolved settings dictionary via new_settings.as_dict() at info level.
Because extra/repo .toml config files commonly include credentials—as indicated by the
repository’s secrets template and the presence of known secret fields like [openai].key and
[gitlab].personal_access_token—printing the full dict can leak these sensitive values into
application/CI logs.

AGENTS.md
pr_agent/git_providers/utils.py[96-105]
pr_agent/git_providers/utils.py[76-106]
pr_agent/settings/.secrets_template.toml[9-88]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pr_agent/git_providers/utils.py::_apply_settings_from_file()` logs the full resolved settings dictionary (`new_settings.as_dict()`) at INFO, which can leak secrets (API keys/tokens) from external or repo-local `.toml` config (e.g., `.pr_agent.toml`) into CI logs and log aggregation systems.
## Issue Context
Extra/repo configuration files may contain sensitive credentials, and the project’s settings/secrets template includes explicit secret fields such as `[openai].key` and `[gitlab].personal_access_token`, making it likely that the logged dictionary contains raw secret values. PR Compliance ID 8 prohibits exposing secrets/tokens in committed code paths, so emitting the full merged config at INFO violates this requirement.
## Fix Focus Areas
- pr_agent/git_providers/utils.py[96-105]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

10. Stale extra config persists ✓ Resolved 🐞 Bug ☼ Reliability
Description
In pr_agent.cli.run(), CONFIG.EXTRA_CONFIG_URL is only set when args.extra_config_url is truthy, so
an earlier in-process call can leave a stale extra-config URL/path in the global settings and
unintentionally apply it on later runs. This is possible because get_settings() falls back to a
process-wide singleton outside starlette_context.
Code

pr_agent/cli.py[R91-92]

+    if getattr(args, "extra_config_url", None):
+        get_settings().set("CONFIG.EXTRA_CONFIG_URL", args.extra_config_url)
Evidence
The CLI only sets EXTRA_CONFIG_URL behind a truthy guard, while settings mutations persist because
get_settings() returns a process-wide singleton outside a request context; run_command()
demonstrates in-process re-entry into run().

pr_agent/cli.py[72-93]
pr_agent/config_loader.py[47-60]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pr_agent/cli.py::run()` only writes `CONFIG.EXTRA_CONFIG_URL` when `args.extra_config_url` is truthy. In long-lived processes (or programmatic callers) that invoke `run()` multiple times, a previously-set extra-config URL can remain in the process-wide Dynaconf singleton and be applied unexpectedly in subsequent runs.
## Issue Context
- `get_settings()` returns a global singleton (`global_settings`) when not running under `starlette_context`, so mutations persist within the Python process.
- `run_command()` calls `run()` in-process, making this scenario realistic for programmatic usage.
## Fix Focus Areas
- pr_agent/cli.py[81-93]
- Set `CONFIG.EXTRA_CONFIG_URL` unconditionally when the attribute exists (including setting it to `None`/empty to clear), rather than only when it is truthy.
- Example approaches:
- `get_settings().set("CONFIG.EXTRA_CONFIG_URL", getattr(args, "extra_config_url", None))`
- or explicitly `unset` when `args.extra_config_url` is falsy to ensure prior values don’t persist.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


11. Windows path misparsed ✓ Resolved 🐞 Bug ≡ Correctness
Description
_resolve_extra_config_to_file() rejects bare Windows drive-letter paths (e.g., "C:\\shared.toml")
because urlparse() interprets the drive letter as a URL scheme and the function only treats scheme
"" as a bare local path. This breaks the documented “bare filesystem path” support and prevents
using --extra_config_url with local paths on Windows.
Code

pr_agent/git_providers/utils.py[R61-83]

+    parsed = urlparse(source)
+    scheme = (parsed.scheme or "").lower()
+
+    # Local path (bare or file://)
+    if scheme in ("", "file"):
+        if scheme == "file":
+            # Preserve any non-localhost netloc (UNC-style file://host/share/...)
+            # and URL-decode percent-encoded path components via url2pathname.
+            netloc = parsed.netloc or ""
+            raw = parsed.path
+            if netloc and netloc.lower() != "localhost":
+                raw = f"//{netloc}{raw}"
+            local_path = url2pathname(raw)
+        else:
+            local_path = source
+        if os.path.isfile(local_path):
+            return local_path, False
+        get_logger().warning(f"Extra config not found at local path: {local_path}")
+        return None, False
+
+    if scheme not in ("http", "https"):
+        get_logger().warning(f"Unsupported scheme for extra config: {scheme}")
+        return None, False
Evidence
The docs promise that a bare filesystem path is accepted, but the resolver only treats `scheme in
("", "file")` as local, and rejects any other scheme. For Windows drive-letter paths, the first
component before : becomes scheme, so they fall into the unsupported-scheme rejection branch.

docs/docs/usage-guide/configuration_options.md[123-128]
pr_agent/git_providers/utils.py[61-83]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_resolve_extra_config_to_file()` treats any parsed scheme other than `""`, `file`, `http`, `https` as unsupported. On Windows, `urlparse("C:\\x.toml")` yields `scheme="c"`, so the function rejects valid bare paths despite docs stating a bare filesystem path is accepted.
### Issue Context
Docs explicitly say a “bare filesystem path” is accepted, but the resolver’s scheme gate only considers `scheme==""` as bare-local.
### Fix Focus Areas
- pr_agent/git_providers/utils.py[61-83]
### Expected fix
Before rejecting unsupported schemes, detect Windows drive-letter paths and treat them as local paths (equivalent to `scheme==""`). For example:
- If the input matches `^[A-Za-z]:[\\/]` (or use `pathlib.PureWindowsPath` heuristics / `os.path.splitdrive`) then set `local_path = source` and continue the local-file branch.
- Add/adjust a unit test covering a drive-letter path input to ensure it resolves as a local file.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


12. Secure loader bypass fallback ✓ Resolved 🐞 Bug ⛨ Security
Description
_apply_settings_from_file() falls back to Dynaconf(settings_files=[path]) on TypeError, which does
not use pr_agent.custom_merge_loader, so validate_file_security() checks (forbidden
includes/preloads/loaders, etc.) won’t run in that mode. This contradicts the documentation’s claim
that external config is always loaded through the secure loader and can allow forbidden directives
to slip through on affected Dynaconf versions.
Code

pr_agent/git_providers/utils.py[R140-151]

+        except TypeError as e:
+            # Older Dynaconf versions don't accept load_dotenv / merge_enabled.
+            # Mirror the fallback used by the repo-settings loader so the extra
+            # config still applies (without those security-hardening flags).
+            get_logger().warning(
+                "Your Dynaconf version does not support disabled "
+                "'load_dotenv'/'merge_enabled' parameters. Loading extra config "
+                "without these security features. Please upgrade Dynaconf for "
+                "better security.",
+                artifact={"error": e, "traceback": traceback.format_exc()},
+            )
+            new_settings = Dynaconf(settings_files=[path])
Evidence
custom_merge_loader is where forbidden-directive checks are enforced via
validate_file_security(). The TypeError fallback path constructs Dynaconf without specifying this
loader, so those security checks won’t execute, despite docs stating the secure loader is used for
external config.

pr_agent/git_providers/utils.py[128-151]
pr_agent/custom_merge_loader.py[41-80]
docs/docs/usage-guide/configuration_options.md[159-166]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When the hardened Dynaconf constructor raises `TypeError`, `_apply_settings_from_file()` falls back to `Dynaconf(settings_files=[path])`, which omits `loaders=["pr_agent.custom_merge_loader"]`. That bypasses `custom_merge_loader.validate_file_security()`.
### Issue Context
The docs state the external file is loaded through the same secure loader as repo-local config. In the current fallback branch, that’s not true.
### Fix Focus Areas
- pr_agent/git_providers/utils.py[128-151]
- pr_agent/custom_merge_loader.py[41-80]
- docs/docs/usage-guide/configuration_options.md[159-166]
### Expected fix
Adjust the fallback strategy to keep using `pr_agent.custom_merge_loader` even when some Dynaconf kwargs are unsupported. Options:
1. Retry `Dynaconf()` with a reduced kwarg set but still passing `core_loaders=[]` and `loaders=["pr_agent.custom_merge_loader"]`.
2. Detect which kwargs caused `TypeError` and remove only those, keeping the custom loader.
Also update docs (or add an explicit note) if you intentionally support an insecure degraded mode—right now docs imply the secure loader always applies.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (5)
13. file:// host dropped ✓ Resolved 🐞 Bug ≡ Correctness
Description
_resolve_extra_config_to_file() converts file:// URLs to a local path using only urlparse(...).path,
discarding urlparse(...).netloc, so valid file URLs like file://<host>/share/config.toml cannot be
resolved and are incorrectly treated as missing. It also passes the raw URL path through to
os.path.isfile() without URL-decoding, so file URLs containing encoded characters won’t resolve to
real filesystem paths.
Code

pr_agent/git_providers/utils.py[R65-70]

+    if scheme in ('', 'file'):
+        local_path = parsed.path if scheme == 'file' else source
+        if os.path.isfile(local_path):
+            return local_path, False
+        get_logger().warning(f"Extra config not found at local path: {local_path}")
+        return None, False
Evidence
The resolver advertises file:// support, but for file-scheme sources it only uses parsed.path, so
the host portion of file://host/... is lost and the resulting path cannot exist. Additionally, no
URL-decoding is performed before calling os.path.isfile().

pr_agent/git_providers/utils.py[61-70]
docs/docs/usage-guide/configuration_options.md[123-128]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_resolve_extra_config_to_file()` mishandles `file://` URLs by ignoring `parsed.netloc` and by not URL-decoding the path before checking `os.path.isfile()`. This breaks legitimate file URLs (notably UNC-style `file://host/share/...`) and any file URL containing percent-encoded characters.
### Issue Context
The docs explicitly advertise `file:///path/to/shared.toml` and “bare filesystem path” support; `file://` parsing should round-trip correctly for common file URL forms.
### Fix Focus Areas
- pr_agent/git_providers/utils.py[61-70]
### Implementation notes
- For `scheme == 'file'`, build the filesystem path from both `netloc` and `path`:
- If `parsed.netloc` is set, treat it as a UNC host and construct `//{netloc}{unquoted_path}`.
- Otherwise use `unquoted_path`.
- URL-decode `parsed.path` (e.g., `urllib.parse.unquote`).
- Consider using `urllib.request.url2pathname()` for better cross-platform conversion (especially Windows drive letters).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


14. No Dynaconf fallback for extra ✓ Resolved 🐞 Bug ☼ Reliability
Description
_apply_settings_from_file() lacks the TypeError fallback used by repo settings loading, so
environments that require that fallback will skip applying the extra config (only logging a
warning).
Code

pr_agent/git_providers/utils.py[R84-95]

+    try:
+        dynconf_kwargs = {
+            'core_loaders': [],
+            'loaders': ['pr_agent.custom_merge_loader'],
+            'merge_enabled': True,
+        }
+        new_settings = Dynaconf(
+            settings_files=[path],
+            load_dotenv=False,
+            envvar_prefix=False,
+            **dynconf_kwargs,
+        )
Evidence
Extra-config loading calls Dynaconf with disabled dynamic-loading parameters without a TypeError
fallback, while the repo-settings loader in the same function has an explicit TypeError fallback
path to keep configuration loading working.

pr_agent/git_providers/utils.py[84-95]
pr_agent/git_providers/utils.py[167-181]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Repo-local settings loading includes a `TypeError` fallback for older Dynaconf versions that don’t support parameters like `load_dotenv`/`merge_enabled`. `_apply_settings_from_file()` does not, so extra config may not load on the same environments where repo settings still work.
### Issue Context
The same file already contains a compatibility fallback for repo settings; extra config should behave consistently.
### Fix Focus Areas
- pr_agent/git_providers/utils.py[84-115]
- pr_agent/git_providers/utils.py[160-181]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


15. Malformed auth header silent ✓ Resolved 📘 Rule violation ☼ Reliability
Description
PR_AGENT_EXTRA_CONFIG_AUTH_HEADER is silently ignored when it does not contain :, with no
warning or error, making misconfiguration hard to detect. This violates the requirement to validate
and normalize configuration inputs at boundaries with targeted warnings.
Code

pr_agent/git_providers/utils.py[R51-56]

+    headers = {'Accept': 'text/plain, application/toml, */*'}
+    auth_header = os.environ.get('PR_AGENT_EXTRA_CONFIG_AUTH_HEADER')
+    if auth_header and ':' in auth_header:
+        name, value = auth_header.split(':', 1)
+        headers[name.strip()] = value.strip()
+
Evidence
PR Compliance ID 21 requires validation/normalization with targeted warnings for invalid inputs; the
new code drops malformed auth headers silently, and the new unit test codifies that silent behavior.

pr_agent/git_providers/utils.py[51-56]
tests/unittest/test_extra_config_url.py[191-204]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`PR_AGENT_EXTRA_CONFIG_AUTH_HEADER` is consumed without validation beyond checking for `:`, and invalid values are silently ignored.
## Issue Context
Per configuration-boundary validation requirements, invalid env/config inputs should produce a targeted warning/error and fall back safely.
## Fix Focus Areas
- pr_agent/git_providers/utils.py[51-56]
- tests/unittest/test_extra_config_url.py[191-204]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


16. Token-like glpat- examples ✓ Resolved 📘 Rule violation ⛨ Security
Description
New docs/tests include GitLab PAT-looking strings (e.g., glpat-...) as examples, which can be
flagged by secret scanners and conflicts with the no-hardcoded-tokens requirement. Use neutral
placeholders that do not match real token formats.
Code

docs/docs/usage-guide/configuration_options.md[R133-141]

+```bash
+# GitLab Personal Access Token
+export PR_AGENT_EXTRA_CONFIG_AUTH_HEADER="PRIVATE-TOKEN: glpat-xxxxxxxxxxxxxxxxxxxx"
+
+# GitLab CI job token
+export PR_AGENT_EXTRA_CONFIG_AUTH_HEADER="JOB-TOKEN: $CI_JOB_TOKEN"
+
+# Generic bearer token
+export PR_AGENT_EXTRA_CONFIG_AUTH_HEADER="Authorization: Bearer xxxxx"
Evidence
PR Compliance ID 8 requires that tokens/secrets not appear in committed materials; the added
examples use a real token prefix pattern (glpat-...) in both documentation and tests.

AGENTS.md
docs/docs/usage-guide/configuration_options.md[133-141]
tests/unittest/test_extra_config_url.py[147-160]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Docs/tests include token-looking example strings (`glpat-...`) that may trigger secret scanning and violate the no-hardcoded-token policy.
## Issue Context
Even if the values are placeholders, many scanners flag real token prefixes/patterns.
## Fix Focus Areas
- docs/docs/usage-guide/configuration_options.md[133-141]
- tests/unittest/test_extra_config_url.py[147-160]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


17. Flaky test port selection ✓ Resolved 🐞 Bug ☼ Reliability
Description
The http_server fixture selects a “free” port by binding and closing a socket, then later binds
HTTPServer to that port; another process/thread can claim the port in between and cause
intermittent bind failures. This can make the new unit tests flaky in parallelized or busy CI
environments.
Code

tests/unittest/test_extra_config_url.py[R57-76]

+def _free_port():
+    s = socket.socket()
+    s.bind(("127.0.0.1", 0))
+    port = s.getsockname()[1]
+    s.close()
+    return port
+
+
+@pytest.fixture
+def http_server():
+    """Spin up _CapturingHandler on a free port for the duration of one test."""
+    # Reset handler-level state so tests don't pollute each other.
+    _CapturingHandler.body = SAMPLE_TOML
+    _CapturingHandler.expected_path = "/shared.pr_agent.toml"
+    _CapturingHandler.require_header = None
+    _CapturingHandler.captured_headers = {}
+
+    port = _free_port()
+    server = HTTPServer(("127.0.0.1", port), _CapturingHandler)
+    thread = threading.Thread(target=server.serve_forever, daemon=True)
Evidence
The code explicitly closes the socket after discovering the port and then binds the HTTP server
afterward, creating a race window that can lead to sporadic bind failures.

tests/unittest/test_extra_config_url.py[57-83]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`tests/unittest/test_extra_config_url.py` uses `_free_port()` to find an available port, closes the socket, and then binds `HTTPServer` to that port. This is a classic TOCTOU race and can flake if the port is taken between the two steps.
### Issue Context
The fixture is used by multiple tests; avoiding nondeterministic port binding failures improves CI reliability.
### Fix Focus Areas
- tests/unittest/test_extra_config_url.py[57-83]
### Expected fix
- Remove `_free_port()` and instead create the server with port `0` (`HTTPServer(("127.0.0.1", 0), handler)`), then read the chosen port from `server.server_address[1]`.
- Keep the rest of the fixture behavior unchanged (thread start, shutdown, join).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit 2bc9179

Results up to commit N/A


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)


Action required
1. Env precedence overwritten ✓ Resolved 🐞 Bug ≡ Correctness
Description
_apply_settings_from_file() overwrites existing settings keys with values from the extra config and
writes them back into the global Dynaconf singleton, so values originally loaded from environment
variables can be replaced and will not be restored. This violates the documented/implemented
precedence expectation that env vars take precedence and can cause env-provided secrets (tokens/API
keys) to be silently ignored in favor of the external .toml.
Code

pr_agent/git_providers/utils.py[R184-188]

+            section_dict = copy.deepcopy(get_settings().as_dict().get(section, {}))
+            for key, value in contents.items():
+                section_dict[key] = value
+            get_settings().unset(section)
+            get_settings().set(section, section_dict, merge=False)
Evidence
The settings singleton is created with env_loader specifically so env vars take precedence; however,
the new extra-config merge overwrites keys in the existing singleton without re-running env loading,
so env-derived values can be replaced despite the documented precedence chain placing env vars last
(highest priority).

pr_agent/config_loader.py[12-16]
pr_agent/git_providers/utils.py[180-189]
docs/docs/usage-guide/configuration_options.md[148-155]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_apply_settings_from_file()` merges extra-config values by copying the current section dict and then overwriting keys with values from the file before setting the section back into the global Dynaconf singleton. Because env vars are loaded into the singleton up-front, this merge can overwrite env-sourced values (including secrets), breaking the documented precedence where env vars should win.
### Issue Context
- `config_loader.py` explicitly enables `dynaconf.loaders.env_loader` “to take precedence”.
- Docs added in this PR show environment variables as the highest-precedence layer.
### Fix Focus Areas
- Re-apply environment-variable overrides after applying `--extra_config_url` (and ideally after other config-file merges) so env vars remain the highest-precedence source.
- Ensure this re-application happens **before** constructing the git provider (since provider `__init__` may read auth settings).
**Code pointers**
- pr_agent/git_providers/utils.py[184-188]
- pr_agent/git_providers/utils.py[201-229]
- pr_agent/config_loader.py[12-16]
- docs/docs/usage-guide/configuration_options.md[148-155]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. AUTO_CAST_FOR_DYNACONF not restored ✓ Resolved 📘 Rule violation ☼ Reliability
Description
New unit tests call apply_repo_settings(), which mutates the process environment by setting
AUTO_CAST_FOR_DYNACONF, but the test sandbox fixture does not snapshot/restore this env var. This
can leak global state across tests and cause order-dependent failures.
Code

tests/unittest/test_extra_config_url.py[R296-320]

+@pytest.fixture
+def settings_sandbox():
+    """Snapshot a few settings keys/sections, yield, restore on teardown."""
+    settings = get_settings()
+    saved = {}
+    for section, key in _TEST_KEYS_TO_RESTORE:
+        if key is None:
+            saved[section] = settings.as_dict().get(section, None)
+        else:
+            saved[(section, key)] = settings.get(f"{section}.{key}", None)
+    try:
+        yield settings
+    finally:
+        # Restore sections/keys to pre-test state
+        for section, key in _TEST_KEYS_TO_RESTORE:
+            if key is None:
+                settings.unset(section)
+                if saved[section] is not None:
+                    settings.set(section, saved[section], merge=False)
+            else:
+                val = saved[(section, key)]
+                if val is None:
+                    settings.unset(f"{section}.{key}")
+                else:
+                    settings.set(f"{section}.{key}", val)
Evidence
PR Compliance ID 15 requires tests to explicitly control and restore environment/global state. The
new tests invoke apply_repo_settings() without restoring the environment, while
apply_repo_settings() sets AUTO_CAST_FOR_DYNACONF, meaning the test suite's global env can be
altered after these tests run.

tests/unittest/test_extra_config_url.py[296-320]
tests/unittest/test_extra_config_url.py[460-531]
pr_agent/git_providers/utils.py[201-203]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new tests call `apply_repo_settings()`, which sets `os.environ["AUTO_CAST_FOR_DYNACONF"] = "false"`. The `settings_sandbox` fixture restores Dynaconf settings only, so the env var can remain modified for subsequent tests, causing flakiness.
## Issue Context
`get_settings()` is a process-wide singleton and environment variables are also process-global. Tests that change either should restore prior state to avoid cross-test pollution.
## Fix Focus Areas
- tests/unittest/test_extra_config_url.py[296-320]
- tests/unittest/test_extra_config_url.py[460-531]
- pr_agent/git_providers/utils.py[201-203]
## Suggested approach
- In `settings_sandbox`, snapshot the prior value of `AUTO_CAST_FOR_DYNACONF` (including the case where it is unset) before yielding.
- In the `finally:` block, restore the previous value (or `del os.environ["AUTO_CAST_FOR_DYNACONF"]` if it was unset).
- Alternatively, add a `monkeypatch`-based fixture used by the `apply_repo_settings(...)` tests to ensure env restoration.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Single quotes in new code ✓ Resolved 📘 Rule violation ⚙ Maintainability

<...

Comment thread pr_agent/git_providers/utils.py
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 20, 2026

Persistent review updated to latest commit 248358b

Comment thread pr_agent/git_providers/utils.py Outdated
Comment thread pr_agent/git_providers/utils.py
Comment thread pr_agent/git_providers/utils.py Outdated
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 20, 2026

Persistent review updated to latest commit 6cba663

Comment thread pr_agent/cli.py Outdated
Comment thread pr_agent/cli.py
Comment thread tests/unittest/test_extra_config_url.py
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 20, 2026

Persistent review updated to latest commit 6f9db0b

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 20, 2026

Persistent review updated to latest commit 422a07a

Comment thread tests/unittest/test_extra_config_url.py
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 20, 2026

Persistent review updated to latest commit 1886406

Comment thread pr_agent/git_providers/utils.py
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 20, 2026

Persistent review updated to latest commit 2bc9179

@tminhtri1910
Copy link
Copy Markdown

Preparing review...

1 similar comment
@tminhtri1910
Copy link
Copy Markdown

Preparing review...

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.

2 participants