Skip to content

Commit 74d9d14

Browse files
author
unraid
committed
test: keep Codecov coverage on real agent communication paths
PR #369 was merged before the final Codecov coverage fix landed, so this follow-up carries only the incremental real-path tests needed on top of main. The tests exercise AgentSummary lifecycle branches, mailbox fail-closed behavior, UDS client connection failure through a real capability file, and UDS response-reader framing without mock.module, warning suppression, feature fallback, or production-code churn. Constraint: PR #369 is already merged; this branch must contain only the incremental Codecov repair on top of latest main Rejected: Reopen or keep pushing the merged PR branch | merged PR refs do not update and would leave Codecov stale Rejected: Mock bun:bundle or hide warnings | would reintroduce cross-test pollution and pseudo coverage Rejected: Keep unrelated SendMessageTool production diff | it created avoidable patch-coverage debt without improving the runtime path Confidence: high Scope-risk: narrow Directive: Keep these coverage tests on real paths; do not replace them with output suppression or feature-flag mocks Tested: bunx tsc --noEmit --pretty false Tested: bun run lint Tested: bun test src\services\AgentSummary\__tests__\agentSummary.test.ts src\services\AgentSummary\__tests__\summaryContext.test.ts src\utils\__tests__\teammateMailbox.test.ts src\utils\__tests__\udsMessaging.test.ts src\utils\__tests__\udsResponseReader.test.ts packages\builtin-tools\src\tools\SendMessageTool\__tests__\udsRecipientSanitization.test.ts Tested: bun run test:all Tested: bun test --coverage --coverage-reporter lcov --coverage-dir coverage Tested: bun run build Tested: bun run build:vite Tested: bun audit Tested: git diff --check Tested: Claude simplify review GO (.omx/artifacts/claude-simplify-codecov-20260427-144906.md) Tested: Claude security review GO (.omx/artifacts/claude-security-codecov-20260427-145007.md) Not-tested: GitHub-hosted Codecov upload after this amended commit until PR checks rerun
1 parent 52b61c2 commit 74d9d14

5 files changed

Lines changed: 205 additions & 46 deletions

File tree

src/services/AgentSummary/__tests__/agentSummary.test.ts

Lines changed: 106 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ import type {
55
CacheSafeParams,
66
ForkedAgentResult,
77
} from '../../../utils/forkedAgent.js'
8-
import { startAgentSummarization } from '../agentSummary.js'
8+
import {
9+
type AgentSummaryDependencies,
10+
startAgentSummarization,
11+
} from '../agentSummary.js'
912

1013
const transcriptMessages = [
1114
{ type: 'user', message: { content: 'start' }, uuid: 'u1' },
@@ -27,17 +30,14 @@ describe('startAgentSummarization', () => {
2730
let forkCalls: ForkCall[]
2831
let updateCalls: Array<{ taskId: string; summary: string }>
2932
let transcriptMessagesForTest: Message[]
33+
let debugLogs: string[]
34+
let loggedErrors: Error[]
35+
let clearedHandles: unknown[]
3036

31-
beforeEach(() => {
32-
forkCalls = []
33-
updateCalls = []
34-
scheduled = undefined
35-
handle = undefined
36-
transcriptMessagesForTest = transcriptMessages
37-
})
38-
39-
test('summarizes bounded transcript once and skips unchanged fingerprints', async () => {
40-
handle = startAgentSummarization(
37+
function startTestSummarization(
38+
dependencies: AgentSummaryDependencies = {},
39+
): { stop: () => void } {
40+
return startAgentSummarization(
4141
'task-1',
4242
asAgentId('a0000000000000000'),
4343
{
@@ -48,14 +48,22 @@ describe('startAgentSummarization', () => {
4848
} as unknown as CacheSafeParams,
4949
() => undefined,
5050
{
51-
clearTimeout: () => undefined,
51+
clearTimeout: ((timeoutId: unknown) => {
52+
clearedHandles.push(timeoutId)
53+
}) as typeof clearTimeout,
5254
getAgentTranscript: async () => ({
5355
messages: transcriptMessagesForTest,
5456
contentReplacements: [],
5557
}),
5658
isPoorModeActive: () => false,
57-
logError: () => undefined,
58-
logForDebugging: () => undefined,
59+
logError: error => {
60+
loggedErrors.push(
61+
error instanceof Error ? error : new Error(String(error)),
62+
)
63+
},
64+
logForDebugging: message => {
65+
debugLogs.push(message)
66+
},
5967
runForkedAgent: async (args: ForkCall) => {
6068
forkCalls.push(args)
6169
return {
@@ -79,8 +87,24 @@ describe('startAgentSummarization', () => {
7987
updateAgentSummary: (taskId: string, summary: string) => {
8088
updateCalls.push({ taskId, summary })
8189
},
90+
...dependencies,
8291
},
8392
)
93+
}
94+
95+
beforeEach(() => {
96+
forkCalls = []
97+
updateCalls = []
98+
scheduled = undefined
99+
handle = undefined
100+
transcriptMessagesForTest = transcriptMessages
101+
debugLogs = []
102+
loggedErrors = []
103+
clearedHandles = []
104+
})
105+
106+
test('summarizes bounded transcript once and skips unchanged fingerprints', async () => {
107+
handle = startTestSummarization()
84108

85109
expect(typeof scheduled).toBe('function')
86110
await scheduled!()
@@ -106,47 +130,83 @@ describe('startAgentSummarization', () => {
106130
expect(updateCalls).toHaveLength(1)
107131
})
108132

109-
test('skips summarization when bounded context is too small', async () => {
110-
transcriptMessagesForTest = transcriptMessages.slice(0, 2)
111-
112-
handle = startAgentSummarization(
113-
'task-1',
114-
asAgentId('a0000000000000000'),
133+
test('skips summarization when filtering leaves too little bounded context', async () => {
134+
transcriptMessagesForTest = [
135+
{ type: 'user', message: { content: 'start' }, uuid: 'u1' },
115136
{
116-
forkContextMessages: transcriptMessages,
117-
model: 'claude-test',
118-
} as unknown as CacheSafeParams,
119-
() => undefined,
120-
{
121-
clearTimeout: () => undefined,
122-
getAgentTranscript: async () => ({
123-
messages: transcriptMessagesForTest,
124-
contentReplacements: [],
125-
}),
126-
isPoorModeActive: () => false,
127-
logError: () => undefined,
128-
logForDebugging: () => undefined,
129-
runForkedAgent: async (args: ForkCall) => {
130-
forkCalls.push(args)
131-
return { messages: [] } as unknown as ForkedAgentResult
132-
},
133-
setTimeout: ((callback: TimerHandler) => {
134-
if (typeof callback !== 'function') {
135-
throw new Error('Expected timer callback')
136-
}
137-
scheduled = callback as () => void | Promise<void>
138-
return 1 as unknown as ReturnType<typeof setTimeout>
139-
}) as unknown as typeof setTimeout,
140-
updateAgentSummary: (taskId: string, summary: string) => {
141-
updateCalls.push({ taskId, summary })
137+
type: 'assistant',
138+
uuid: 'a1',
139+
message: {
140+
content: [{ type: 'tool_use', id: 'missing', name: 'Read' }],
142141
},
143142
},
143+
{ type: 'user', message: { content: 'continue' }, uuid: 'u2' },
144+
] as unknown as Message[]
145+
146+
handle = startTestSummarization()
147+
148+
expect(typeof scheduled).toBe('function')
149+
await scheduled!()
150+
151+
expect(forkCalls).toEqual([])
152+
expect(updateCalls).toEqual([])
153+
expect(debugLogs).toContain(
154+
'[AgentSummary] Skipping summary for task-1: no bounded context available',
144155
)
156+
})
157+
158+
test('skips summarization before building context when transcript is too short', async () => {
159+
transcriptMessagesForTest = transcriptMessages.slice(0, 2)
160+
handle = startTestSummarization()
145161

146162
expect(typeof scheduled).toBe('function')
147163
await scheduled!()
148164

149165
expect(forkCalls).toEqual([])
150166
expect(updateCalls).toEqual([])
167+
expect(debugLogs).toContain(
168+
'[AgentSummary] Skipping summary for task-1: not enough messages (2)',
169+
)
170+
})
171+
172+
test('skips and reschedules while poor mode is active', async () => {
173+
handle = startTestSummarization({
174+
isPoorModeActive: () => true,
175+
})
176+
177+
expect(typeof scheduled).toBe('function')
178+
await scheduled!()
179+
180+
expect(forkCalls).toEqual([])
181+
expect(updateCalls).toEqual([])
182+
expect(debugLogs).toContain(
183+
'[AgentSummary] Skipping summary — poor mode active',
184+
)
185+
})
186+
187+
test('logs summary errors and keeps the next timer owned by the summarizer', async () => {
188+
const error = new Error('fork failed')
189+
handle = startTestSummarization({
190+
runForkedAgent: async () => {
191+
throw error
192+
},
193+
})
194+
195+
expect(typeof scheduled).toBe('function')
196+
await scheduled!()
197+
198+
expect(loggedErrors).toEqual([error])
199+
expect(updateCalls).toEqual([])
200+
})
201+
202+
test('stop clears the pending summary timer', () => {
203+
handle = startTestSummarization()
204+
205+
handle.stop()
206+
207+
expect(debugLogs).toContain(
208+
'[AgentSummary] Stopping summarization for task-1',
209+
)
210+
expect(clearedHandles).toEqual([1])
151211
})
152212
})

src/services/AgentSummary/__tests__/summaryContext.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,13 @@ describe('getSummaryContextFingerprint', () => {
141141
expect(estimateMessageChars(message)).toBeGreaterThan(0)
142142
})
143143

144+
test('treats unsupported top-level primitives as zero-size estimates', () => {
145+
expect(
146+
estimateMessageChars((() => undefined) as unknown as Message),
147+
).toBe(0)
148+
expect(estimateMessageChars(1n as unknown as Message)).toBe(0)
149+
})
150+
144151
test('returns null for an empty transcript', () => {
145152
expect(getSummaryContextFingerprint([])).toBeNull()
146153
})

src/utils/__tests__/teammateMailbox.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,17 @@ describe('compactMailboxMessages', () => {
171171

172172
expect(compacted).toEqual([])
173173
})
174+
175+
test('returns an empty mailbox when all retention lanes are disabled', () => {
176+
const compacted = compactMailboxMessages([message('unread', false)], {
177+
maxMessages: 0,
178+
maxReadMessages: 0,
179+
maxUnreadProtocolMessages: 0,
180+
maxRetainedBytes: 1_000,
181+
})
182+
183+
expect(compacted).toEqual([])
184+
})
174185
})
175186

176187
describe('teammate mailbox retention', () => {
@@ -331,6 +342,23 @@ describe('teammate mailbox retention', () => {
331342
expect(await readFile(inboxPath, 'utf-8')).toBe('{not-json')
332343
})
333344

345+
test('writeToMailbox rejects when the inbox path is already a directory', async () => {
346+
const inboxPath = getInboxPath('worker', 'alpha')
347+
await mkdir(inboxPath, { recursive: true })
348+
349+
await expect(
350+
writeToMailbox(
351+
'worker',
352+
{
353+
from: 'team-lead',
354+
text: 'new',
355+
timestamp: new Date(5).toISOString(),
356+
},
357+
'alpha',
358+
),
359+
).rejects.toThrow()
360+
})
361+
334362
test('readMailbox fails closed on corrupt mailbox content', async () => {
335363
const inboxPath = getInboxPath('worker', 'alpha')
336364
await mkdir(dirname(inboxPath), { recursive: true })

src/utils/__tests__/udsMessaging.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,23 @@ describe('UDS inbox retention', () => {
217217
)
218218
})
219219

220+
test('udsClient send reports connection failures without leaking token state', async () => {
221+
const path = socketPath('uds-client-connect-error')
222+
const capabilityDir = join(tempConfigDir, 'messaging-capabilities')
223+
const capabilityName = `${createHash('sha256').update(path).digest('hex')}.json`
224+
await mkdir(capabilityDir, { recursive: true, mode: 0o700 })
225+
await writeFile(
226+
join(capabilityDir, capabilityName),
227+
JSON.stringify({ socketPath: path, authToken: 'test-token' }),
228+
'utf-8',
229+
)
230+
const { sendToUdsSocket } = await import('../udsClient.js')
231+
232+
await expect(sendToUdsSocket(path, 'hello')).rejects.toThrow(
233+
'Failed to connect to peer',
234+
)
235+
})
236+
220237
test('sendUdsMessage fails closed before connecting without an auth token', async () => {
221238
await expect(
222239
sendUdsMessage(socketPath('no-auth-token'), { type: 'text', data: 'x' }),

src/utils/__tests__/udsResponseReader.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,28 @@ describe('attachUdsResponseReader', () => {
9797
expect(socket.ended).toBe(true)
9898
})
9999

100+
test('continues scanning when blank and valid frames share one chunk', () => {
101+
const socket = new FakeSocket()
102+
let settled = false
103+
let settledError: Error | undefined
104+
105+
attachUdsResponseReader(asSocket(socket), {
106+
maxFrameBytes: 128,
107+
onSettled: error => {
108+
settled = true
109+
settledError = error
110+
},
111+
})
112+
113+
socket.emitData(
114+
Buffer.from(`\n${JSON.stringify({ type: 'response' })}\n`),
115+
)
116+
117+
expect(settled).toBe(true)
118+
expect(settledError).toBeUndefined()
119+
expect(socket.ended).toBe(true)
120+
})
121+
100122
test('rejects receiver error frames', () => {
101123
const socket = new FakeSocket()
102124
let settledError: Error | undefined
@@ -116,6 +138,31 @@ describe('attachUdsResponseReader', () => {
116138
expect(socket.destroyed).toBe(true)
117139
})
118140

141+
test('ignores unrelated receiver frames until a terminal response arrives', () => {
142+
const socket = new FakeSocket()
143+
let settled = false
144+
let settledError: Error | undefined
145+
146+
attachUdsResponseReader(asSocket(socket), {
147+
maxFrameBytes: 128,
148+
onSettled: error => {
149+
settled = true
150+
settledError = error
151+
},
152+
})
153+
154+
socket.emitData(
155+
Buffer.from(
156+
`${JSON.stringify({ type: 'notification', data: 'queued' })}\n`,
157+
),
158+
)
159+
expect(settled).toBe(false)
160+
161+
socket.emitData(Buffer.from(`${JSON.stringify({ type: 'response' })}\n`))
162+
expect(settled).toBe(true)
163+
expect(settledError).toBeUndefined()
164+
})
165+
119166
test('uses custom socket error formatting', () => {
120167
const socket = new FakeSocket()
121168
let settledError: Error | undefined

0 commit comments

Comments
 (0)