fix(server): a secretRef is handed out once even under concurrent use

consume read the stored value and deleted it in two store calls, so two
concurrent calls with one ref could both read it. Consumption is now
serialized. The client's single-flight key for answering a card encodes its
ids structurally, so ids containing a colon can't collide.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Author: juliusmarminge

apps/server/src/secrets/SecretRequests.test.ts

@@ -49,7 +49,8 @@ const withService = <A, E>(
       Layer.succeed(
         ServerSecretStore.ServerSecretStore,
         ServerSecretStore.ServerSecretStore.of({
-          get: (name) => Effect.succeed(Option.fromNullishOr(stored.get(name))),
+          // Yields like a real file read, so concurrent callers can interleave.
+          get: (name) => Effect.yieldNow.pipe(Effect.as(Option.fromNullishOr(stored.get(name)))),
           set: (name, value) => Effect.sync(() => void stored.set(name, value)),
           create: (name, value) =>
             stored.has(name)
@@ -148,6 +149,26 @@ it.effect("a saved answer becomes a one-use ref, and the thread only learns it w
   ),
 );
 
+it.effect("two concurrent uses of one ref hand the value out once", () =>
+  withService(({ service }) =>
+    Effect.gen(function* () {
+      yield* service.answer({
+        threadId,
+        turnItemId,
+        answer: { type: "save", secret: "ghp_secret" },
+      });
+      const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId }));
+      const results = yield* Effect.all(
+        [service.consume({ ref, projectId }), service.consume({ ref, projectId })].map(
+          Effect.result,
+        ),
+        { concurrency: "unbounded" },
+      );
+      assert.deepEqual(results.map((result) => result._tag).toSorted(), ["Failure", "Success"]);
+    }),
+  ),
+);
+
 it.effect("a ref only works in the project it was entered for", () =>
   withService(({ service }) =>
     Effect.gen(function* () {

apps/server/src/secrets/SecretRequests.ts

@@ -27,6 +27,7 @@ import * as Layer from "effect/Layer";
 import * as Option from "effect/Option";
 import * as Schedule from "effect/Schedule";
 import * as Schema from "effect/Schema";
+import * as Semaphore from "effect/Semaphore";
 
 import * as ServerSecretStore from "../auth/ServerSecretStore.ts";
 import * as Metrics from "../observability/Metrics.ts";
@@ -178,8 +179,12 @@ const make = Effect.gen(function* () {
       ),
     );
 
+  // get and remove are separate store calls; one consumer at a time keeps two
+  // concurrent calls from both reading a ref before either deletes it.
+  const consumeLock = yield* Semaphore.make(1);
   const consume: SecretRequests["Service"]["consume"] = (input) =>
     consumeRef(input).pipe(
+      consumeLock.withPermits(1),
       Effect.tap(() => Metrics.increment(Metrics.secretRefsConsumedTotal, { result: "used" })),
       Effect.tapError(() =>
         Metrics.increment(Metrics.secretRefsConsumedTotal, { result: "rejected" }),

packages/client-runtime/src/state/server.ts

@@ -1306,7 +1306,8 @@ export function createServerEnvironmentAtoms<R, E>(
       tag: WS_METHODS.secretsAnswerRequest,
       concurrency: {
         mode: "singleFlight",
-        key: ({ environmentId, input }) => `${environmentId}:${input.threadId}:${input.turnItemId}`,
+        key: ({ environmentId, input }) =>
+          JSON.stringify([environmentId, input.threadId, input.turnItemId]),
       },
     }),
     refreshUsageRates: createEnvironmentRpcCommand(runtime, {