From 2c7131cea63eacafe6dd89341f51c4ccc8a2f5b4 Mon Sep 17 00:00:00 2001
From: unraid <local@unraid.local>
Date: Mon, 27 Apr 2026 16:02:49 +0800
Subject: [PATCH] test: remove brittle review follow-up assumptions

CodeRabbit's second pass found two valid brittleness issues and one suggested callback-reference assertion that would not match production behavior. This keeps the production behavior unchanged: timers still schedule the summarizer closure, tests now assert timer-handle identity, and UDS connection errors use native Error.cause instead of shadowing it.

Constraint: Do not manufacture behavior just to satisfy a review hint; assertions must match the real AgentSummary scheduling contract.

Rejected: Assert a fresh scheduled callback function | scheduleNext intentionally passes the same runSummary closure each time.

Rejected: Store a custom cause field on UdsPeerConnectionError | native Error.cause is available under ESNext/Bun.

Confidence: high

Scope-risk: narrow

Directive: Timer tests should assert returned handle identity for ownership, not incidental numeric values.

Tested: bun test src/services/AgentSummary/__tests__/agentSummary.test.ts src/utils/__tests__/udsMessaging.test.ts

Tested: bunx tsc --noEmit --pretty false

Tested: bun run lint

Tested: bun run test:all

Tested: bun test --coverage --coverage-reporter lcov --coverage-dir coverage

Tested: bun run build

Tested: bun run build:vite

Not-tested: GitHub-hosted CodeRabbit refresh until pushed.
---
 .../AgentSummary/__tests__/agentSummary.test.ts    | 14 +++++++++++---
 src/utils/udsClient.ts                             |  7 ++++---
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/services/AgentSummary/__tests__/agentSummary.test.ts b/src/services/AgentSummary/__tests__/agentSummary.test.ts
index 39fff199c..cc3c87d36 100644
--- a/src/services/AgentSummary/__tests__/agentSummary.test.ts
+++ b/src/services/AgentSummary/__tests__/agentSummary.test.ts
@@ -34,6 +34,7 @@ describe('startAgentSummarization', () => {
   let loggedErrors: Error[]
   let clearedHandles: unknown[]
   let scheduledCount: number
+  let lastTimerHandle: unknown
 
   function startTestSummarization(
     dependencies: AgentSummaryDependencies = {},
@@ -84,7 +85,8 @@ describe('startAgentSummarization', () => {
           }
           scheduledCount += 1
           scheduled = callback as () => void | Promise<void>
-          return scheduledCount as unknown as ReturnType<typeof setTimeout>
+          lastTimerHandle = { id: scheduledCount }
+          return lastTimerHandle as ReturnType<typeof setTimeout>
         }) as unknown as typeof setTimeout,
         updateAgentSummary: (taskId: string, summary: string) => {
           updateCalls.push({ taskId, summary })
@@ -104,6 +106,7 @@ describe('startAgentSummarization', () => {
     loggedErrors = []
     clearedHandles = []
     scheduledCount = 0
+    lastTimerHandle = undefined
   })
 
   test('summarizes bounded transcript once and skips unchanged fingerprints', async () => {
@@ -179,6 +182,7 @@ describe('startAgentSummarization', () => {
 
     expect(typeof scheduled).toBe('function')
     const initialScheduledCount = scheduledCount
+    const initialTimerHandle = lastTimerHandle
     await scheduled!()
 
     expect(forkCalls).toEqual([])
@@ -187,9 +191,10 @@ describe('startAgentSummarization', () => {
       '[AgentSummary] Skipping summary — poor mode active',
     )
     expect(scheduledCount).toBe(initialScheduledCount + 1)
+    expect(lastTimerHandle).not.toBe(initialTimerHandle)
   })
 
-  test('logs summary errors and keeps the next timer owned by the summarizer', async () => {
+  test('logs summary errors and schedules the next timer', async () => {
     const error = new Error('fork failed')
     handle = startTestSummarization({
       runForkedAgent: async () => {
@@ -199,21 +204,24 @@ describe('startAgentSummarization', () => {
 
     expect(typeof scheduled).toBe('function')
     const initialScheduledCount = scheduledCount
+    const initialTimerHandle = lastTimerHandle
     await scheduled!()
 
     expect(loggedErrors).toEqual([error])
     expect(updateCalls).toEqual([])
     expect(scheduledCount).toBe(initialScheduledCount + 1)
+    expect(lastTimerHandle).not.toBe(initialTimerHandle)
   })
 
   test('stop clears the pending summary timer', () => {
     handle = startTestSummarization()
+    const pendingHandle = lastTimerHandle
 
     handle.stop()
 
     expect(debugLogs).toContain(
       '[AgentSummary] Stopping summarization for task-1',
     )
-    expect(clearedHandles).toEqual([1])
+    expect(clearedHandles).toEqual([pendingHandle])
   })
 })
diff --git a/src/utils/udsClient.ts b/src/utils/udsClient.ts
index 63792ef72..1e06756bb 100644
--- a/src/utils/udsClient.ts
+++ b/src/utils/udsClient.ts
@@ -38,13 +38,14 @@ export type PeerSession = {
 
 export class UdsPeerConnectionError extends Error {
   readonly socketPath: string
-  readonly cause: unknown
 
   constructor(socketPath: string, cause: unknown) {
-    super(`Failed to connect to peer at ${socketPath}: ${errorMessage(cause)}`)
+    super(
+      `Failed to connect to peer at ${socketPath}: ${errorMessage(cause)}`,
+      { cause },
+    )
     this.name = 'UdsPeerConnectionError'
     this.socketPath = socketPath
-    this.cause = cause
   }
 }