fix(redis): never credit the shared user counter from a buffer delete

An owner id is not proof of who wrote the bytes, so crediting the user counter
on clear let anyone able to name a stream decrement a ceiling they never
charged. That is the one direction that must not be possible: a counter driven
down grants writes rather than denying them.

The clear now drops the owner counter only. The user counter's fixed window
settles it instead — it already tolerates accruing bytes Redis has dropped, and
this is the same over-count bounded by the same window. The scope threading that
existed only to credit it is removed with it.

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

Author: waleedlatif1

apps/sim/lib/copilot/request/lifecycle/start.ts

@@ -206,10 +206,7 @@ export function createSSEStream(params: StreamingOrchestrationParams): ReadableS
               }
             | undefined
 
-          await Promise.all([
-            resetBuffer(streamId, { streamId, ...(userId ? { userId } : {}) }),
-            clearFilePreviewSessions(streamId),
-          ])
+          await Promise.all([resetBuffer(streamId), clearFilePreviewSessions(streamId)])
 
           if (chatId) {
             createRunSegment({

apps/sim/lib/copilot/request/session/buffer.test.ts

@@ -412,24 +412,24 @@ describe('mothership-stream-outbox', () => {
     expect(mockRedis.eval).not.toHaveBeenCalled()
   })
 
-  it('releases the owner counter and credits the user when the buffer is cleared', async () => {
+  it('drops the owner counter together with the buffer it accounts for', async () => {
     // The buffer keys are deleted rather than expired, so a counter left behind would refuse a
-    // retry that reuses the same streamId against bytes that no longer exist anywhere.
-    await clearBuffer('stream-1', 'clear_outbox', { streamId: 'stream-1', userId: 'user-1' })
+    // retry that reuses the same streamId against bytes that no longer exist anywhere. One
+    // script, so a concurrent append cannot land between the delete and the release and keep
+    // its events stored with its reservation already erased.
+    await clearBuffer('stream-1')
 
-    // One script, so a concurrent append cannot land between the delete and the release and
-    // keep its events stored with its reservation already erased.
     const evalCall = mockRedis.eval.mock.calls.at(-1)
-    expect(evalCall?.[1]).toBe(5)
+    expect(evalCall?.[1]).toBe(4)
     expect(evalCall?.[5]).toBe('execution:redis-budget:copilot_stream:stream-1')
-    expect(evalCall?.[6]).toBe('execution:redis-budget:user:user-1')
   })
 
-  it('releases only the owner counter when no user is in scope', async () => {
+  it('never touches the shared user counter when clearing a buffer', async () => {
+    // An owner id is not proof of who wrote the bytes, so crediting the user counter here would
+    // let anyone who can name a stream decrement a ceiling they never charged.
     await clearBuffer('stream-1')
 
-    const evalCall = mockRedis.eval.mock.calls.at(-1)
-    expect(evalCall?.[1]).toBe(4)
-    expect(evalCall?.[5]).toBe('execution:redis-budget:copilot_stream:stream-1')
+    const keys = mockRedis.eval.mock.calls.at(-1)?.slice(2, 6) as string[]
+    expect(keys.some((key) => key.includes('redis-budget:user:'))).toBe(false)
   })
 })

apps/sim/lib/copilot/request/session/buffer.ts

@@ -110,35 +110,29 @@ ${renderRedisBudgetReleaseLua(3)}
 return 1
 `
 
-export async function resetBuffer(streamId: string, scope?: StreamBudgetScope): Promise<void> {
-  await clearBuffer(streamId, 'reset_outbox', scope)
+export async function resetBuffer(streamId: string): Promise<void> {
+  await clearBuffer(streamId, 'reset_outbox')
 }
 
-export async function clearBuffer(
-  streamId: string,
-  operation = 'clear_outbox',
-  scope?: StreamBudgetScope
-): Promise<void> {
+export async function clearBuffer(streamId: string, operation = 'clear_outbox'): Promise<void> {
   /*
-    Delete and release in ONE script. The counter outlives the data it accounts for
-    unless it is released here — these keys are deleted rather than expired, so a retry
-    reusing the same streamId would be refused against bytes that no longer exist. Doing
-    it in a second round trip would be its own hole: a concurrent append landing between
-    the two would keep its events stored with its reservation already erased.
+    Delete and release in ONE script. The counter outlives the data it accounts for unless
+    it is dropped here — these keys are deleted rather than expired, so a retry reusing the
+    same streamId would be refused against bytes that no longer exist. Doing it in a second
+    round trip would be its own hole: a concurrent append landing between the two would keep
+    its events stored with its reservation already erased.
+
+    Only the owner counter, never the shared user counter — see the release fragment.
   */
-  const budgetKeys = getRedisBudgetKeys({
-    kind: 'copilot_stream',
-    id: streamId,
-    ...(scope?.userId ? { userId: scope.userId } : {}),
-  })
+  const [ownerBudgetKey] = getRedisBudgetKeys({ kind: 'copilot_stream', id: streamId })
   await withRedisRetry({ operation, streamId }, async (redis) => {
     await redis.eval(
       CLEAR_BUFFER_SCRIPT,
-      3 + budgetKeys.length,
+      4,
       getEventsKey(streamId),
       getSeqKey(streamId),
       getAbortKey(streamId),
-      ...budgetKeys
+      ownerBudgetKey
     )
   })
 }

apps/sim/lib/core/redis/byte-budget.server.ts

@@ -202,30 +202,27 @@ end
 }
 
 /**
- * Lua that releases an owner's whole reservation, for data that is deleted rather than
- * left to expire.
+ * Lua that drops an owner's counter, for data that is deleted rather than left to expire.
  *
  * Rendered into the caller's own script, the same way {@link renderRedisBudgetLua} is, so
  * the release commits together with the delete it accounts for. Releasing in a second
  * round trip would let a concurrent write land in between and keep its bytes stored with
  * its reservation already erased.
  *
- * Contract: budget keys are the **last** one or two entries of `KEYS`, in the order
- * {@link getRedisBudgetKeys} returns them, and `baseKeyCount` is how many precede them.
+ * The shared user counter is deliberately NOT credited here. An owner id is not proof of
+ * who wrote the bytes — anyone who can name an owner could otherwise decrement a counter
+ * they never charged, which is the one direction that must never be possible, since a
+ * counter driven down grants writes rather than denying them. The user counter's fixed
+ * window is what settles it instead: it already tolerates accruing bytes Redis has dropped
+ * (see {@link REDIS_BUDGET_TTL_SECONDS}), and this is the same over-count, bounded by the
+ * same window.
+ *
+ * Contract: the owner key is the **last** entry of `KEYS`, and `baseKeyCount` is how many
+ * precede it.
  */
 export function renderRedisBudgetReleaseLua(baseKeyCount: number): string {
-  const ownerKey = `KEYS[${baseKeyCount + 1}]`
-  const userKey = `KEYS[${baseKeyCount + 2}]`
-
   return `
-local owner_bytes = tonumber(redis.call('GET', ${ownerKey}) or '0')
-redis.call('DEL', ${ownerKey})
-if #KEYS >= ${baseKeyCount + 2} and owner_bytes > 0 then
-  local user_next = redis.call('DECRBY', ${userKey}, owner_bytes)
-  if user_next <= 0 then
-    redis.call('DEL', ${userKey})
-  end
-end
+redis.call('DEL', KEYS[${baseKeyCount + 1}])
 `
 }