[Pulse demo] psf/requests PR #6048: Fixing unicode surrogates in POST data lead to exception

[dominic097]

    Pulse demo seed recreated this as a synthetic PR in the fork on `2026-06-24`.

    Original PR: https://github.com/psf/requests/pull/6048
    Original number: #6048
    Original author: @mftb
    Original head: `fix/issue-6008`
    Original base: `main`
    Original labels: none

    Representative upstream files:
    - `requests/adapters.py` (MODIFIED)

• requests/models.py (MODIFIED)

• tests/test_requests.py (MODIFIED)

    ---
  
    Closes #6008
  

Fixing the unicode surrogates issue

[dominic097]

🐞 correctness · 🟠 Major

PR claims a surrogate-handling bug fix, but no production code is changed

This PR is titled and described as fixing 'unicode surrogates' in POST data, but the actual repository diff does not modify any runtime code. The only substantive addition here is metadata that references upstream changes in requests/adapters.py and requests/models.py (files array in this JSON), while this PR itself changes neither of those files; the only other changed file in the diff is tests/test_requests.py, and it only adds a comment. As a result, the bug described by the PR remains present and merging this would incorrectly signal that issue psf#6008 is fixed. Please include the actual library code changes in the request preparation/encoding path (and any required adapter handling) rather than only recording upstream PR metadata.

P2 · Impact high (4/5) · Confidence high

🤖 Prompt for AI agents

Port the real surrogate-handling implementation into the affected runtime files in this repository, ensuring the POST data preparation path no longer raises the reported exception for unpaired surrogates. Keep the metadata file only if desired, but do not rely on it as the fix.

_{🔒 Secure confirmation — opens an authenticated page; the fix is attributed to you. Or reply @aziron fix finding:root-issue-not-implemented}

[dominic097]

🧪 test · 🟡 Moderate

No executable regression test was added for the surrogate POST-data failure

The only change in tests/test_requests.py is a comment (# Pulse demo seed 2026-06-24: metadata from psf/requests PR #6048), so this PR adds no runnable coverage for the issue it claims to fix. Given the PR title/body say POST data with Unicode surrogates previously raised an exception, there should be a regression test that exercises that exact input through request preparation/sending and asserts the intended non-crashing behavior. Without such a test, the bug is neither demonstrated nor protected against future regressions.

P3 · Impact medium (3/5) · Confidence high

🤖 Prompt for AI agents

Add a focused regression test in `tests/test_requests.py` that constructs POST data containing an unpaired Unicode surrogate, runs it through the affected Requests code path, and asserts the expected behavior instead of the prior exception. Include any necessary companion test updates if the fix changes request-preparation semantics.

_{🔒 Secure confirmation — opens an authenticated page; the fix is attributed to you. Or reply @aziron fix finding:missing-regression-test}

[dominic097]

🔍 Aziron Pulse Review

❌ Changes requested · Confidence ⭐ 2/5 · 2 file(s) · 2 finding(s)

This PR does not implement the referenced surrogate-handling fix in the library code; it only adds a demo metadata JSON file and a comment in an existing test file. As submitted, the root issue described by the PR title/body remains unaddressed and no validating tests were added.

Confidence Score: 2/5

The diff is extremely small and fully visible: it adds one metadata JSON file and one comment line block, with no production-code modifications. That makes it clear the described bug fix is not implemented in this PR.

💬 Pre-merge checks · ✅ 1 · ❌ 3

❌ Failed checks

Check	Status	Explanation	Resolution
Description check	⚠️ Warning	The description says 'Closes psf#6008' and 'Fixing the unicode surrogates issue', but the submitted diff only adds metadata/comment changes and does not support that claim.	Update the PR description to match the actual scope or, preferably, include the missing implementation and regression tests before merge.
Linked issues check	⚠️ Warning	The PR claims to close issue psf#6008, but the current diff does not contain the corresponding fix.	Do not auto-close the issue until the actual runtime change and tests are included.
Out of scope changes check	⚠️ Warning	The added .pulse-demo metadata file is ancillary/demo content and does not contribute to the stated runtime fix.	Separate demo-seed/metadata additions from bug-fix PRs, or clearly scope the PR as metadata-only.

✅ Passed checks

Check	Status	Explanation
Title check	✅ Passed	The title describes the intended bug fix scope, even though the diff does not implement it.

📝 Walkthrough

Engineering Review

What changed: The diff adds a new metadata file at .pulse-demo/sampled-prs/2026-06-24/upstream-pr-6048.json containing copied information about upstream psf/requests PR psf#6048, including references to changes that are not actually present in this repository diff (requests/adapters.py, requests/models.py, and upstream tests/test_requests.py). The only codebase file changed here is tests/test_requests.py, where lines 3095-3096 add a non-functional comment: # Pulse demo seed 2026-06-24: metadata from psf/requests PR #6048. No runtime Python logic, request encoding path, or actual test behavior changed in this PR.

Correctness: The change is not correct relative to the stated purpose. The added JSON says the upstream PR fixes 'unicode surrogates' by modifying requests/adapters.py and requests/models.py, but those files are not part of this diff, and there is no corresponding implementation in the changed repository files. In the actual diff, tests/test_requests.py only gains a comment at lines 3095-3096, which cannot affect POST encoding behavior or prevent exceptions. Therefore the issue described by the PR title/body ('Fixing unicode surrogates in POST data lead to exception') is not fixed by the submitted changes.

Completeness: The PR is incomplete for its stated scope. A complete fix would require the production code changes in the request preparation/adapter path plus regression tests exercising POST data containing unpaired Unicode surrogates. Instead, the diff only records upstream metadata in .pulse-demo/sampled-prs/2026-06-24/upstream-pr-6048.json and adds a seed comment in tests/test_requests.py; neither implements nor verifies the behavior change.

Risks: Primary risk is merge-time false confidence: the PR title/body indicate a bug fix, but the repository would ship unchanged runtime behavior. This can mislead maintainers or downstream users into believing the surrogate exception path has been addressed when it has not. There is no direct runtime regression from the comment/metadata additions themselves, but there is a process/traceability risk because the repository state would claim closure of an issue without corresponding code.

Missing tests: A regression test should be added in tests/test_requests.py that constructs POST data with an unpaired surrogate and asserts request preparation/sending follows the intended behavior instead of raising the original exception. If the real fix touches both request preparation and adapter/network paths, tests should cover at least: form-encoded POST data, JSON payload behavior if applicable, and the exact exception/encoding boundary that previously failed. No such executable test was added in this diff.

Follow-up work: Apply the actual upstream code changes (or an equivalent local fix) in the real affected runtime files, not just metadata. Add targeted regression coverage for surrogate-containing POST bodies. If .pulse-demo content is intentionally tracked for demonstrations, consider separating demo-seed PRs from bug-fix PRs so issue-closing titles do not imply runtime changes that are absent.

Decision — REQUEST_CHANGES: Per the decision criteria, request_changes is required because the root issue is not addressed and significant correctness/completeness concerns remain. The only functional repository file change is a comment, while the PR title/body claim a production bug fix; that mismatch is blocking. Test coverage is also inadequate because no executable test for the surrogate scenario was added.

📁 Files changed

Important Files Changed

Filename	Overview
.pulse-demo/sampled-prs/2026-06-24/upstream-pr-6048.json	Adds demo metadata describing an upstream PR, including references to runtime file changes that are not part of this repository diff; this is documentation/demo data only, not an implementation of the stated fix.
tests/test_requests.py	Adds only a seed comment near existing tests; no assertions, fixtures, or executable regression coverage were introduced.

Actionable comments posted: 2 (0 suggested change(s), 2 fix-session candidate(s))

Fix all findings: comment @aziron fix

📋 Reviewed against: lint (python): pyproject.toml

_{Aziron Pulse — sandbox + MCP-assisted LLM review with native code intelligence.}

[dominic097]

🪄 Autofix — generate a fix for all Aziron Pulse findings on this PR:

• Create a new PR with the fixes

_{Tick the box above (or comment @aziron fix) to start a fix. Aziron opens the fix as a new PR back to this branch.}