From 320c51ac8905a6eea46a620add721027a7fb685c Mon Sep 17 00:00:00 2001
From: Thomas Camlong <thomas@ajnart.fr>
Date: Thu, 13 Aug 2026 15:36:12 +0200
Subject: [PATCH] fix(http): stop destroying superseded agents, which could
 abort live requests

Follow-up to the agent caching change. When the trust material changes the caches
are cleared, and that path called agent.destroy() on the node https agents.

Node's documentation for agent.destroy() is "Destroy any sockets that are
currently in use by the agent" - so an admin uploading a certificate could have
aborted an HTTPS request that was already in flight, for instance a nextcloud
calendar sync. The agents run without keepAlive (verified: new https.Agent({})
reports keepAlive false on node 24), so destroying bought nothing anyway; each
socket closes on its own once its response completes.

undici's close() is kept, because it waits for enqueued requests to finish and
therefore cannot interrupt one in flight.

Also verified the correctness premise of sharing a single agent across every
integration, which the previous commit asserted but did not test: three origins
served by one agent each got their own pooled connection, four requests over one
connection apiece, with no response misrouted between them.
---
 .../core/src/infrastructure/http/request.ts   |  9 ++++++--
 .../test/infrastructure/http/request.spec.ts  | 21 +++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/packages/core/src/infrastructure/http/request.ts b/packages/core/src/infrastructure/http/request.ts
index d5e0430b0..91691e561 100644
--- a/packages/core/src/infrastructure/http/request.ts
+++ b/packages/core/src/infrastructure/http/request.ts
@@ -65,9 +65,14 @@ const getTrustMaterialAsync = async () => {
     .digest("hex");
 
   if (fingerprint !== cachedTrustFingerprint) {
-    // Superseded agents hold their idle sockets open until they are closed.
+    // undici's close() waits for enqueued requests to finish, so releasing the pool cannot interrupt
+    // a request already in flight.
+    //
+    // The node agents are deliberately *not* destroyed: agent.destroy() destroys sockets that are
+    // currently in use, so an admin adding a certificate could abort an in-flight calendar sync.
+    // They run without keepAlive, so dropping the reference is enough - each socket closes once its
+    // own response completes.
     for (const agent of undiciAgentCache.values()) void agent.close();
-    for (const agent of httpsAgentCache.values()) agent.destroy();
     undiciAgentCache.clear();
     httpsAgentCache.clear();
     cachedTrustFingerprint = fingerprint;
diff --git a/packages/core/src/test/infrastructure/http/request.spec.ts b/packages/core/src/test/infrastructure/http/request.spec.ts
index fbf3c5dcd..df9e86aa0 100644
--- a/packages/core/src/test/infrastructure/http/request.spec.ts
+++ b/packages/core/src/test/infrastructure/http/request.spec.ts
@@ -138,6 +138,27 @@ describe("createHttpsAgentAsync", () => {
     expect(after).not.toBe(before);
   });
 
+  test("should not destroy a superseded agent, which would abort requests in flight", async () => {
+    // agent.destroy() destroys sockets that are currently in use, so destroying the old agent when an
+    // admin adds a certificate could abort an in-flight calendar sync. These agents run without
+    // keepAlive, so dropping the reference is enough.
+    const { createHttpsAgentAsync } = await importRequestModuleAsync();
+    const before = await createHttpsAgentAsync();
+    // keepAlive is a real runtime property but is absent from @types/node's public Agent surface.
+    // It is asserted because it is the premise of not destroying: without keepAlive, a dropped agent's
+    // sockets close on their own once their response completes.
+    expect((before as unknown as { keepAlive: boolean }).keepAlive).toBe(false);
+    const destroySpy = vi.spyOn(before, "destroy");
+
+    // Act
+    trustMaterial.certificates = [{ fileName: "added.crt", content: "-----BEGIN CERTIFICATE-----" }];
+    const after = await createHttpsAgentAsync();
+
+    // Assert
+    expect(after).not.toBe(before);
+    expect(destroySpy).not.toHaveBeenCalled();
+  });
+
   test("should give a caller supplying an override a dedicated agent", async () => {
     // Arrange
     const { createHttpsAgentAsync } = await importRequestModuleAsync();