fix: per-request settings clone in bitbucket-server & azuredevops webhooks (#2345)

[alonz101]

User description

Summary

Fixes #2345 — apply_repo_settings() leaking .pr_agent.toml state across PRs from different repositories.

Root cause

global_settings is a module-level Dynaconf singleton. apply_repo_settings() mutates it without resetting between requests, so settings loaded for one repo persist into subsequent reviews of unrelated repos in the same long-lived webhook process.

Fix

5 of 7 webhook servers already isolate per-request settings with context["settings"] = copy.deepcopy(global_settings) (github_app, gitlab_webhook, gitea_app, bitbucket_app, gerrit_server). This PR aligns the last 2 (bitbucket_server_webhook, azuredevops_server_webhook) with that existing pattern. get_settings() already prefers context["settings"] over global_settings, so the per-request clone is sufficient.

Tests

tests/unittest/test_apply_repo_settings.py covers:

• Repo settings get applied within a request
• Repo A's [pr_reviewer] extra_instructions don't leak into a subsequent Repo B request that has no .pr_agent.toml
• Repo A's unknown section (e.g. [my_custom_repo_section]) doesn't leak either

Test plan

• PYTHONPATH=. .venv/bin/pytest tests/unittest/test_apply_repo_settings.py -v (3 passed)
• PYTHONPATH=. .venv/bin/pytest tests/unittest -q (297 passed, no regressions)
• pre-commit run --files <changed> (clean)

---

PR Type

Bug fix, Tests

---

Description

• Fix .pr_agent.toml state leaking across PRs from different repositories

• Add context["settings"] = copy.deepcopy(global_settings) per-request isolation to bitbucket_server_webhook and azuredevops_server_webhook handlers

• Add regression tests covering settings isolation, unknown section leaks, and TOML application

• Minor whitespace/formatting cleanup in both webhook server files

---

Diagram Walkthrough

flowchart LR
  A["Incoming Webhook Request"]
  B["bitbucket_server_webhook\nhandle_webhook()"]
  C["azuredevops_server_webhook\nhandle_webhook()"]
  D["context['settings'] =\ncopy.deepcopy(global_settings)"]
  E["apply_repo_settings()\nmutates context['settings']"]
  F["global_settings\n(module-level singleton)\nremains clean"]
  G["Next request gets\nfresh settings clone"]

  A -- "POST /webhook" --> B
  A -- "POST /" --> C
  B -- "per-request clone" --> D
  C -- "per-request clone" --> D
  D -- "isolated mutation" --> E
  F -- "deep copy source" --> D
  E -- "request ends" --> F
  F -- "next request" --> G

Loading

File Walkthrough

	Relevant files
Bug fix	azuredevops_server_webhook.py Add per-request settings clone to Azure DevOps webhook handler pr_agent/servers/azuredevops_server_webhook.py Added import copy and imported context from starlette_context and global_settings from config_loader Added context["settings"] = copy.deepcopy(global_settings) at the start of handle_webhook() to isolate per-request settings Minor whitespace cleanup (trailing spaces removed in several functions) +7/-4      bitbucket_server_webhook.py Add per-request settings clone to Bitbucket Server webhook handler pr_agent/servers/bitbucket_server_webhook.py Added import copy and imported context from starlette_context and global_settings from config_loader Added context["settings"] = copy.deepcopy(global_settings) at the start of handle_webhook() to isolate per-request settings Minor whitespace cleanup (trailing spaces removed in several functions) Reformatted a long import line for BitbucketServerProvider +12/-8
Tests	test_apply_repo_settings.py Add regression tests for cross-repo settings isolation      tests/unittest/test_apply_repo_settings.py New test file with FakeGitProvider stub and fresh_global_settings fixture test_repo_settings_from_toml_are_applied: verifies TOML settings are applied within a request context test_repo_without_toml_does_not_inherit_previous_repo_settings: verifies repo A's settings don't leak into repo B with no .pr_agent.toml test_unknown_section_does_not_leak_to_next_repo: verifies custom/unknown TOML sections don't persist across requests +121/-0

---

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

Review Summary by Qodo

Fix per-request settings isolation in bitbucket-server and azuredevops webhooks

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Clone global_settings per-request in bitbucket-server and azuredevops webhooks
• Prevents .pr_agent.toml state leakage across different repository PRs
• Aligns last 2 webhook servers with existing pattern used by 5 others
• Adds comprehensive regression tests for settings isolation

Diagram

flowchart LR
  A["Webhook Request"] --> B["Clone global_settings"]
  B --> C["context['settings']"]
  C --> D["apply_repo_settings"]
  D --> E["Isolated per-request"]
  F["Next Request"] --> B
  E -.->|"No leak"| F

Loading

File Changes

1. pr_agent/servers/azuredevops_server_webhook.py 🐞 Bug fix +7/-4

Add per-request settings clone to Azure DevOps webhook

• Added copy import for deep cloning settings
• Imported context from starlette_context for per-request storage
• Imported global_settings from config_loader
• Added context["settings"] = copy.deepcopy(global_settings) in handle_webhook() to isolate
 settings per request
• Fixed minor whitespace issues (trailing spaces)

pr_agent/servers/azuredevops_server_webhook.py

---

2. pr_agent/servers/bitbucket_server_webhook.py 🐞 Bug fix +12/-8

Add per-request settings clone to Bitbucket Server webhook

• Added copy import for deep cloning settings
• Imported context from starlette_context for per-request storage
• Imported global_settings from config_loader
• Added context["settings"] = copy.deepcopy(global_settings) in handle_webhook() to isolate
 settings per request
• Fixed minor whitespace and import formatting issues

pr_agent/servers/bitbucket_server_webhook.py

---

3. tests/unittest/test_apply_repo_settings.py 🧪 Tests +121/-0

Add regression tests for settings isolation

• Created new test file with FakeGitProvider mock for testing settings isolation
• Added fresh_global_settings fixture to restore global state between tests
• Test test_repo_settings_from_toml_are_applied() verifies repo settings load correctly
• Test test_repo_without_toml_does_not_inherit_previous_repo_settings() verifies no cross-repo
 leakage of known sections
• Test test_unknown_section_does_not_leak_to_next_repo() verifies custom sections don't persist
 across requests

tests/unittest/test_apply_repo_settings.py

---

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

Code Review by Qodo

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

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

[alonz101]

PR Description updated to latest commit (fc8ab42)

[alonz101]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 2345 - Partially compliant Compliant requirements: apply_repo_settings() no longer leaks state across repos — per-request clone isolates mutations Settings from repo A do not persist when repo B has no .pr_agent.toml Unknown/custom sections from one repo do not leak to the next repo bitbucket_server_webhook and azuredevops_server_webhook now set context["settings"] = copy.deepcopy(global_settings) per request, matching the other 5 webhook servers Regression tests covering all three scenarios are added Full existing unit test suite passes (297 passed per PR description) Non-compliant requirements: Per-section merge carrying over stale keys when repo B does have a .pr_agent.toml (partial overlap case) is not explicitly tested or addressed — the fix only isolates via the per-request clone, which handles the empty-toml case but the partial-overlap scenario within a single request is still present in apply_repo_settings() logic itself Requires further human verification: Confirm that the other 5 webhook servers (github_app, gitlab_webhook, gitea_app, bitbucket_app, gerrit_server) already set context["settings"] before apply_repo_settings() is called, so no similar leak exists there Verify that background tasks dispatched via background_tasks.add_task(...) correctly inherit the context["settings"] set before the task is enqueued (starlette-context behavior with BackgroundTasks)

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Context not propagated to BackgroundTask context["settings"] is set in the request handler, but the actual work is dispatched via background_tasks.add_task(handle_request_azure, data, log_context). Starlette's BackgroundTasks run after the response is sent and outside the request context, so starlette_context's context object (which is request-scoped) may not be accessible in the background task. If the context is not available, get_settings() will fall back to global_settings, defeating the isolation. The same pattern exists in bitbucket_server_webhook.py. This should be verified against how the other 5 webhook servers handle this (they may pass the cloned settings explicitly or use a different dispatch mechanism). context["settings"] = copy.deepcopy(global_settings) # get_logger().info(json.dumps(data)) background_tasks.add_task(handle_request_azure, data, log_context) Context not propagated to BackgroundTask Same issue as in azuredevops_server_webhook.py: context["settings"] is set before background_tasks.add_task(...) is called, but background tasks in Starlette run outside the request lifecycle. If starlette_context does not propagate the context to background tasks, the per-request clone will not be visible inside the task, and get_settings() will fall back to the shared global_settings singleton — leaving the state-leak bug unfixed at runtime despite the tests passing (tests use request_cycle_context which does propagate correctly). context["settings"] = copy.deepcopy(global_settings) get_logger().info(json.dumps(data)) webhook_secret = get_settings().get("BITBUCKET_SERVER.WEBHOOK_SECRET", None)

[alonz101]

PR Code Suggestions ✨

Category	Suggestion	Impact
Possible issue	Pass settings clone explicitly to background task The context["settings"] is set in the request handler but the actual processing happens in a background task. Since starlette_context is request-scoped, the context (and thus the settings clone) may not be available when the background task runs after the request completes. The deep copy of settings should be passed explicitly as a parameter to the background task function to ensure isolation. pr_agent/servers/azuredevops_server_webhook.py [180] -background_tasks.add_task(handle_request_azure, data, log_context) +settings_copy = copy.deepcopy(global_settings) +context["settings"] = settings_copy +background_tasks.add_task(handle_request_azure, data, log_context, settings_copy) Suggestion importance[1-10]: 7 __ Why: This is a valid concern: starlette_context is request-scoped and may not persist into background tasks after the request completes. Passing the settings_copy explicitly to handle_request_azure would ensure the settings are available during background processing. However, the improved_code changes the function signature of handle_request_azure which isn't shown in the diff, and the actual behavior depends on how starlette_context handles background tasks in this specific setup.	Medium

[alonz101]

Looked into the BackgroundTask concern — false positive

The auto /review worried that context["settings"] might not survive into background_tasks.add_task(...)
because Starlette runs background tasks "after the response." Tested it with the same pattern this PR uses:

@app.post("/webhook")                             
async def handle_webhook(background_tasks: BackgroundTasks, request: Request):
    data = await request.json()                                                                                 
    context["settings"] = f"clone-for-{data['repo']}"
    background_tasks.add_task(check, data["repo"])                                                              
                                                                                                                
Two sequential requests:                          
request 'A' → context['settings'] = 'clone-for-A'  ✅                                                           
request 'B' → context['settings'] = 'clone-for-B'  ✅
                                                     
starlette_context uses a ContextVar and the middleware wraps the request with with request_cycle_context(...).  
Starlette's BackgroundTask is awaited inside Response.__call__, before the middleware's with block exits — so   
background tasks see the per-request clone, not the global singleton. Same reason the 5 existing webhook servers
 using this pattern work fine.  

No code change needed.