Read the conversation that asked before taking a lock on the one being answered in

CHANGELOG.md

@@ -8,6 +8,16 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### A hop that cannot read the conversation no longer locks one on its way out
+
+Delivering a hop took the run lock on the conversation the answer lands in, and only then read the
+conversation that asked. A platform that answers that read with anything other than a missing thread
+— an outage, a bad key, a dropped connection — threw from between the lock and the `finally` that
+gives it back, so the lock was left held and unrenewed. For the next couple of minutes the person
+could not start a run in that conversation, and the hop's own retry collided with the lock it was
+still holding itself and spent one of its attempts reporting the conversation as busy. The read now
+happens before the lock is taken, which it never needed: it is a different conversation.
+
 ### A Bot's shell can no longer reach the embedded database without a password
 
 In the all-in-one image the cluster was `trust`-auth on loopback, and the Bot's shell runs in the

server/src/agents/handoff-delivery.ts

@@ -202,6 +202,23 @@ export function createHandoffDelivery(options: {
         ? { threadId: work.answerIn }
         : await answerIn({ actorId: work.actorId, botId: work.toBotId });
 
+      /*
+       * The conversation that ASKED, read BEFORE the lock is taken.
+       *
+       * The addressed Bot is joining something already in progress and has to have read it; its own
+       * conversation is new and empty, and reading that would tell it nothing. This is a different
+       * thread from the one about to be locked, so the lock never protected this read — and holding
+       * one across it means a read that throws leaks it. `historyOrEmpty` answers a missing thread
+       * with nothing and rethrows everything else on purpose ("a 500 from the platform means an
+       * outage or a bad key"), so one 500 left the addressed Bot's conversation locked, unrenewed and
+       * unreleasable until the platform's own TTL expired. For those two minutes the person could not
+       * start a run there, and the hop's own retry a minute later collided with the lock it was still
+       * holding itself, reported the conversation as busy, and spent one of its five attempts on it.
+       */
+      const prior = conversationOnly(
+        await history({ threadId: work.threadId, actorId: work.actorId }),
+      );
+
       /*
        * The conversation's lock, before a single event is streamed.
        *
@@ -239,14 +256,7 @@ export function createHandoffDelivery(options: {
        * displayed the question directly above the answer.
        */
       const asked = [
-        /*
-         * The conversation that ASKED, not the one it is answering in. The addressed Bot is joining
-         * something already in progress and has to have read it; its own conversation is new and
-         * empty, and reading that would tell it nothing.
-         */
-        ...conversationOnly(
-          await history({ threadId: work.threadId, actorId: work.actorId }),
-        ),
+        ...prior,
         { id: `handoff-${runId}`, role: "user", content: message },
       ];
       agent.threadId = where.threadId;

server/tests/agent-handoff-delivery.test.ts

@@ -44,7 +44,17 @@ function delivery(
   events: BaseEvent[],
   agent: AbstractAgent | null = stubAgent(),
   lockHeld = true,
-  options: { history?: readonly unknown[]; deadlineMs?: number } = {},
+  options: {
+    history?: readonly unknown[];
+    deadlineMs?: number;
+    /**
+     * The platform refusing to hand back the asking conversation.
+     *
+     * `historyOrEmpty` answers a missing thread with nothing and rethrows everything else, on
+     * purpose: a 500 is an outage or a bad key, not an empty conversation.
+     */
+    historyError?: Error;
+  } = {},
 ) {
   const requests: Array<{
     threadId: string;
@@ -62,7 +72,10 @@ function delivery(
         ? {}
         : { deadlineMs: options.deadlineMs }),
       agentFor: async () => agent,
-      history: async () => options.history ?? PRIOR,
+      history: async () => {
+        if (options.historyError) throw options.historyError;
+        return options.history ?? PRIOR;
+      },
       newRunId: () => "run-2",
       answerIn: async () => ({ threadId: "answer-thread" }),
       lock: {
@@ -99,6 +112,37 @@ function delivery(
 }
 
 describe("turning a hop into a turn", () => {
+  /*
+   * A hop that cannot read the conversation it was asked in has to fail without taking anything with
+   * it. The lock is on the conversation being ANSWERED in, the read is of the one that ASKED, so the
+   * lock never protected this read — and holding one across it means the failure costs a person the
+   * use of a conversation they are not even part of, for as long as the platform's own TTL.
+   */
+  test("a history the platform will not hand back leaves no lock behind", async () => {
+    const { delivery: deliver, lockCalls } = delivery(
+      FINISHED,
+      stubAgent(),
+      true,
+      { historyError: new Error("the platform answered 500") },
+    );
+
+    await expect(
+      deliver.deliver({
+        work: WORK,
+        message: "m",
+        shown: "s",
+        assertion: "signed",
+      }),
+    ).rejects.toThrow("the platform answered 500");
+
+    /*
+     * Nothing was taken, so nothing was left held. Asserting the whole sequence rather than the
+     * absence of a release: a delivery that acquired and then released would also be correct, and
+     * this says which of the two happened.
+     */
+    expect(lockCalls).toEqual([]);
+  });
+
   test("the addressed Bot reads the conversation before the ask", async () => {
     const { delivery: deliver, requests } = delivery(FINISHED);