fix(authenticator): publish separate body cache entries atomically

[kokhlo]

Fixes #11048

Problem

A SeparateBodyFileCache entry is stored in two files: metadata in self._fn(key) and the
body in self._fn(key) + ".body". CacheController._cache_set() publishes them one after the
other, metadata first:

self.cache.set(cache_url, self.serializer.dumps(request, response, b""))
# body is None can happen when, for example, we're only updating
# headers, as is the case in update_cached_response().
if body is not None:
    self.cache.set_body(cache_url, body)

Each file is written atomically on its own (tempfile.mkstemp + os.replace under a
FileLock), but the pair is not published atomically. A lookup that runs between the two calls
in CacheController._load_from_cache() reads:

cache_data = self.cache.get(cache_url)      # published
body_file = self.cache.get_body(cache_url)  # not published yet -> None

The entry deserializes into a response with an empty body. For a package index that reads as
"this package has no releases at all", so a parallel install concludes that a dependency does
not exist — the Package setuptools (...) not found failure reported in #11048.

The same state is left behind permanently when a write is interrupted between the two calls:
every later lookup finds the metadata and no body, so the entry never recovers on its own.

Fix

AtomicSeparateBodyFileCache publishes the body first and the metadata last, so an entry only
becomes readable once it is complete:

• set() holds the metadata back instead of writing it, unless the key already has a stored
  body. The existing-body case is a revalidation that only refreshes headers, where no
  set_body() follows and the metadata must go out right away.
• set_body() writes the body, then publishes the held-back metadata.
• get() reports a miss when the body file is absent, which covers entries that are still in
  flight and entries a crashed write left behind.
• delete() drops the held-back metadata so a purged entry cannot come back.

The overridden methods keep the signatures of the base class, so callers in CacheControl,
including update_cached_response() which never calls set_body(), keep working.

Tests

tests/utils/test_atomic_separate_body_file_cache.py:

• an entry is not readable before its body is written;
• a concurrent reader during publication sees either a miss or the complete entry, never
  metadata without a body;
• the same window on the unmodified SeparateBodyFileCache is asserted as well, so the premise
  of the fix is covered by a test rather than only by the description;
• metadata left behind without a body counts as a miss;
• a revalidation that only refreshes headers keeps the stored body;
• delete() removes a held-back entry.

Note on the upstream fix

psf/cachecontrol#465 addresses the same race by locking once around the metadata and the body
and by routing the controller through new combined get_with_body() / set_with_body() calls.
That change is not released, so the >=0.14.0,<0.15.0 constraint cannot be tightened yet. This
fix covers the reported failure on Poetry's side and stays independent of the upstream
interface: the sub-class only uses set(), set_body(), get() and delete(). If #465 lands
and ships, the follow-up is to raise the lower bound and drop this sub-class.

This fix does not address mixed generations (metadata of one update paired with the body of
another). Closing that needs a shared lock across the controller's read and write, which is the
part #465 changes.

[sourcery-ai]

Hey - I've found 2 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/poetry/utils/authenticator.py" line_range="162-164" />
<code_context>
+    def set_body(self, key: str, body: bytes) -> None:
+        super().set_body(key, body)
+
+        metadata = self._pending_metadata.pop(key, None)
+        if metadata is not None:
+            super().set(key, metadata)
+
+    def get(self, key: str) -> bytes | None:
</code_context>
<issue_to_address>
**issue (bug_risk):** If the final metadata write raises after the body has been written, `_pending_metadata.pop()` has already discarded the metadata, so the completed body is left orphaned and a retry of `set_body()` cannot publish the entry.

**Triggers:** When the metadata file write fails after the body file write succeeds.

**Suggested fix:** Only remove the pending metadata after `super().set()` succeeds, or restore it if publication fails.

```suggestion
        metadata = self._pending_metadata.get(key)
        if metadata is not None:
            super().set(key, metadata)
            self._pending_metadata.pop(key, None)
```
</issue_to_address>

### Comment 2
<location path="tests/utils/test_atomic_separate_body_file_cache.py" line_range="44-64" />
<code_context>
+    cache = AtomicSeparateBodyFileCache(tmp_path)
+    reader = AtomicSeparateBodyFileCache(tmp_path)
+
+    metadata_published = threading.Event()
+    publish_body = threading.Event()
+
+    def writer() -> None:
+        cache.set(KEY, METADATA)
+        metadata_published.set()
+        publish_body.wait(timeout=10)
+        cache.set_body(KEY, BODY)
+
+    thread = threading.Thread(target=writer)
+    thread.start()
+    assert metadata_published.wait(timeout=10)
+
+    assert reader.get(KEY) is None
+    assert reader.get_body(KEY) is None
+
+    publish_body.set()
+    thread.join(timeout=10)
+    assert not thread.is_alive()
+
+    assert reader.get(KEY) == METADATA
+
+
</code_context>
<issue_to_address>
**issue (testing):** The concurrency test never reads during the actual publication window: it blocks before `set_body()`, then joins the writer before asserting the final value. It therefore passes even if `set_body()` publishes metadata before the body or exposes an intermediate state.

**Triggers:** When the implementation has a race between the body write and the delayed metadata write.

**Suggested fix:** Add a barrier or monkeypatch around the base body/metadata writes so the reader runs after one publication step and before the other.
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 2 findings to address first, and if the publication logic is wrong, cache metadata or bodies can be left inconsistent on disk, causing bounded cache misses or stale entries after a revert. Clearing or rebuilding the affected cache repairs the state; no permanent data or access decision is altered.

Blocking findings: src/poetry/utils/authenticator.py:164, tests/utils/test_atomic_separate_body_file_cache.py:64

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

[sourcery-ai]

issue (bug_risk): If the final metadata write raises after the body has been written, _pending_metadata.pop() has already discarded the metadata, so the completed body is left orphaned and a retry of set_body() cannot publish the entry.

Triggers: When the metadata file write fails after the body file write succeeds.

Suggested fix: Only remove the pending metadata after super().set() succeeds, or restore it if publication fails.

Suggested change

	metadata = self._pending_metadata.pop(key, None)
	if metadata is not None:
	super().set(key, metadata)
	metadata = self._pending_metadata.get(key)
	if metadata is not None:
	super().set(key, metadata)
	self._pending_metadata.pop(key, None)

[sourcery-ai]

issue (testing): The concurrency test never reads during the actual publication window: it blocks before set_body(), then joins the writer before asserting the final value. It therefore passes even if set_body() publishes metadata before the body or exposes an intermediate state.

Triggers: When the implementation has a race between the body write and the delayed metadata write.

Suggested fix: Add a barrier or monkeypatch around the base body/metadata writes so the reader runs after one publication step and before the other.