Skip to content

fix: avoid shared mutable default args in GitHost helpers - #283

Closed
eve-ci-cd[bot] wants to merge 1 commit into
mainfrom
fix/git-host-mutable-default-args
Closed

eve-ci-cd[bot] wants to merge 1 commit into
mainfrom
fix/git-host-mutable-default-args

Conversation

@eve-ci-cd

@eve-ci-cd eve-ci-cd Bot commented Sep 30, 2026

Copy link
Copy Markdown

Problem

GitHostObject.get/list/create/update in bert_e/git_host/base.py used params={} / headers={} as default arguments. GitHubClient._get() mutates the headers it receives (kwargs.setdefault('headers', {}).update(headers)), so the conditional-request headers (If-None-Match, If-Modified-Since) were written into one dict shared by all calls and all server threads.

It is currently masked because the update overwrites both keys on every call, but it is a latent thread-safety/leak bug that would surface as soon as any other header is added.

Fix

Default to None and create a fresh dict per call. No behaviour change otherwise.

Testing

Unit tests pass locally (excluding test_github_app_auth.py, which needs network access in my sandbox).

🤖 Generated with Claude Code

GitHubClient._get() mutates the headers dict it receives
(kwargs.setdefault('headers', {}).update(...)). Since GitHostObject.get()
and list() defaulted to a single shared headers={} dict, conditional
request headers (If-None-Match / If-Modified-Since) were written into
state shared across every call and thread. Default to None and build a
fresh dict per call.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@eve-ci-cd
eve-ci-cd Bot requested a review from a team as a code owner September 30, 2026 09:50
@codecov

codecov Bot commented Sep 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.19%. Comparing base (cfe0bc2) to head (4cf0c8f).

Files with missing lines Patch % Lines
bert_e/git_host/base.py 70.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #283      +/-   ##
==========================================
- Coverage   90.21%   90.19%   -0.03%     
==========================================
  Files          82       82              
  Lines       11293    11299       +6     
==========================================
+ Hits        10188    10191       +3     
- Misses       1105     1108       +3     
Flag Coverage Δ
integration 87.62% <40.00%> (-0.06%) ⬇️
tests 87.58% <40.00%> (-0.06%) ⬇️
tests-BuildFailedTest 25.69% <40.00%> (-0.02%) ⬇️
tests-QuickTest 32.99% <40.00%> (-0.03%) ⬇️
tests-RepositoryTests 25.37% <40.00%> (-0.02%) ⬇️
tests-TaskQueueTests 49.57% <40.00%> (-0.04%) ⬇️
tests-TestBertE 66.93% <40.00%> (-0.05%) ⬇️
tests-TestQueueing 51.65% <40.00%> (-0.04%) ⬇️
tests-api-mock 14.44% <40.00%> (-0.01%) ⬇️
tests-noqueue 78.33% <40.00%> (-0.06%) ⬇️
tests-noqueue-BuildFailedTest 25.69% <40.00%> (-0.02%) ⬇️
tests-noqueue-QuickTest 32.99% <40.00%> (-0.03%) ⬇️
tests-noqueue-RepositoryTests 25.37% <40.00%> (-0.02%) ⬇️
tests-noqueue-TaskQueueTests 49.57% <40.00%> (-0.04%) ⬇️
tests-noqueue-TestBertE 63.50% <40.00%> (-0.05%) ⬇️
tests-noqueue-TestQueueing 25.40% <40.00%> (-0.02%) ⬇️
tests-server 26.70% <40.00%> (-0.02%) ⬇️
unittests 43.42% <70.00%> (+<0.01%) ⬆️
utests 29.11% <70.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant