fix(gitea): return repo settings as bytes so .pr_agent.toml loads#2435
fix(gitea): return repo settings as bytes so .pr_agent.toml loads#2435IsmaelMartinez wants to merge 1 commit into
Conversation
Code Review by Qodo
Context used 1. get_repo_settings() assumes str content
|
GiteaProvider.get_repo_settings() returned a str, but utils.apply_repo_settings() writes the value with os.write() and later calls .decode() on it, both of which require bytes — the contract the GitHub, GitLab and Bitbucket providers already follow. On Gitea this raised "a bytes-like object is required, not 'str'" and broke repo-level .pr_agent.toml loading. Encode the content before returning, mirroring the Bitbucket provider, and add regression tests for the bytes contract and the empty cases. See The-PR-Agent#2347. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
a22c23f to
b4b61be
Compare
|
Code review by qodo was updated up to the latest commit b4b61be |
|
Good catch — fixed. The two early-return branches now return |
| # utils.apply_repo_settings() writes this via os.write() and later | ||
| # calls .decode() on it, so it must be bytes to match the GitHub/ | ||
| # GitLab/Bitbucket contract. get_file_content() decodes the raw bytes | ||
| # to str, so re-encode here (see issue #2347). | ||
| return response.encode('utf-8') |
There was a problem hiding this comment.
1. get_repo_settings() assumes str content 📎 Requirement gap ☼ Reliability
get_repo_settings() unconditionally calls response.encode('utf-8'), which will raise
AttributeError if get_file_content() (or a mock/SDK change) returns bytes. This violates the
requirement to accept both str and bytes repo settings content for Gitea without type errors.
Agent Prompt
## Issue description
`GiteaProvider.get_repo_settings()` must accept repository settings content returned as either `str` or `bytes`. The current implementation always does `response.encode('utf-8')`, which will crash if `response` is already `bytes`.
## Issue Context
Compliance requires Gitea repo settings loading to be robust to `str`/`bytes` variations to avoid `os.write()`/`.decode()` type errors.
## Fix Focus Areas
- pr_agent/git_providers/gitea_provider.py[624-628]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Two findings on the latest commit (b4b61be): 1. 2. "Bytes annotation returns str" — resolved in |
|
I've opened a few small contributions recently, including this fix and a regression test on PR #2258. I'd genuinely appreciate it if the maintainers could let me know whether these fit the way the team likes to work, and I'm very happy to adjust them or change my approach if you'd prefer something different. |
PR Summary by Qodo
Fix Gitea repo settings to return bytes so .pr_agent.toml loads
🐞 Bug fix🧪 Tests🕐 10-20 MinutesWalkthroughs
User Description
What
GiteaProvider.get_repo_settings()returned astr, bututils.apply_repo_settings()writes the value withos.write(fd, repo_settings)andhandle_configurations_errors()later calls.decode()on it — both requirebytes. This is the contract the GitHub (decoded_content), GitLab (python-gitlab.decode()) and Bitbucket (response.text.encode('utf-8')) providers already follow.As a result, loading a repo-level
.pr_agent.tomlon Gitea failed witha bytes-like object is required, not 'str', and the error handler then crashed in turn with'str' object has no attribute 'decode', so repo-level configuration was effectively unusable.Fix
Encode the content to
bytesinget_repo_settings()before returning, mirroring the Bitbucket provider (which also starts from astr).RepoApi.get_file_content()is intentionally left returningstr, since its other callers (diff and language analysis) legitimately consume text.Tests
Added regression tests in
tests/unittest/test_gitea_provider.py:test_get_repo_settings_returns_bytes— asserts the return isbytesand round-trips through the exactos.write/.decodeoperationsutils.pyperforms.test_get_repo_settings_falsy_when_unset_or_missing— locks the falsy-on-missing behaviour shared with the other providers.Fixes #2347.
AI Description
Diagram
graph TD A[GiteaProvider] --> B["get_repo_settings()"] --> C[RepoApi] --> D["get_file_content()"] --> E["encode('utf-8')"] --> F["utils.apply_repo_settings()"] subgraph Legend direction LR _mod([Module/Class]) ~~~ _fn[Function] ~~~ _ext[[External/Helper]] endHigh-Level Assessment
The following are alternative approaches to this PR:
1. Make utils.apply_repo_settings accept str as well as bytes
2. Return empty bytes (b'') consistently on error/missing settings
Recommendation: The chosen approach (encode to bytes in GiteaProvider.get_repo_settings) is the best fit because it restores the existing provider contract relied upon by utils.apply_repo_settings and aligns Gitea with GitHub/GitLab/Bitbucket behavior. Consider a follow-up to return b'' (instead of '') on failure/missing cases to fully match the new bytes return annotation and avoid mixed-type falsy values.
File Changes
Bug fix (1)
Tests (1)