From e12e8bafef8f35c116ba7ec59dd4027ff5b1f43b Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Fri, 23 Oct 2026 19:56:27 -0700 Subject: [PATCH] test: strengthen hook lifecycle and transcript boundaries (#33015) Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- ...-hook-listener-session-replacement.test.ts | 5 ++- .../agent-hook-listener-transport.test.ts | 42 +++++++++++-------- ...claude-settled-permission-requests.test.ts | 23 +++++++++- .../reverse-file-region-scan.test.ts | 29 +++++++++---- .../agent-hook-request-body-memory.test.ts | 32 -------------- 5 files changed, 69 insertions(+), 62 deletions(-) delete mode 100644 src/shared/agent-hook-request-body-memory.test.ts diff --git a/src/shared/agent-hook-listener-session-replacement.test.ts b/src/shared/agent-hook-listener-session-replacement.test.ts index da179e68414..747aa67908b 100644 --- a/src/shared/agent-hook-listener-session-replacement.test.ts +++ b/src/shared/agent-hook-listener-session-replacement.test.ts @@ -141,7 +141,8 @@ describe('Claude session replacement voids the replaced session claims', () => { const paneKey = makePaneKey('same-session', LEAF_ID) stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) - const same = stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(true) + const same = stop(state, paneKey, SESSION_A) expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(true) expect(same?.payload.state).toBe('working') @@ -193,7 +194,7 @@ describe('Claude session replacement voids the replaced session claims', () => { expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(false) }) - it('leaves the lead-state record to the incoming fold rather than deleting it', () => { + it('folds the replacement session prompt into the lead working state', () => { const state = createHookListenerState() const paneKey = makePaneKey('lead-untouched', LEAF_ID) diff --git a/src/shared/agent-hook-listener-transport.test.ts b/src/shared/agent-hook-listener-transport.test.ts index 50a4df2860b..831203a0e40 100644 --- a/src/shared/agent-hook-listener-transport.test.ts +++ b/src/shared/agent-hook-listener-transport.test.ts @@ -1,7 +1,7 @@ import { EventEmitter } from 'node:events' import type { IncomingHttpHeaders, IncomingMessage } from 'node:http' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { mkdtempSync, readFileSync, rmSync, statSync } from 'node:fs' +import { mkdtempSync, readFileSync, readdirSync, rmSync, statSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { @@ -68,7 +68,7 @@ describe('shared agent-hook-listener', () => { expectRequestParserListenersReleased(req) }) - it('normalizes a raw JSON hook body with metadata headers', async () => { + it('normalizes a routed raw JSON hook body with metadata headers', async () => { const req = createReadableRequest({ 'content-type': 'application/json', 'x-orca-pane-key': paneKey, @@ -80,13 +80,17 @@ describe('shared agent-hook-listener', () => { req.emit('data', Buffer.from('{"hook_event_name":"UserPromptSubmit","prompt":"hello"}')) req.emit('end') + const source = resolveHookSource('/hook/claude') + if (source === null) { + throw new Error('Expected the Claude hook route to resolve') + } const merged = mergeAgentHookRequestHeaders(await body, req.headers) expect( - normalizeHookPayload(createHookListenerState(), 'claude', merged, 'production') + normalizeHookPayload(createHookListenerState(), source, merged, 'production') ).toMatchObject({ paneKey, worktreeId: 'repo::/tmp/work', - payload: { state: 'working', prompt: 'hello' } + payload: { state: 'working', prompt: 'hello', agentType: 'claude' } }) }) @@ -261,18 +265,7 @@ describe('shared agent-hook-listener', () => { expectRequestParserListenersReleased(req) }) - it('routes pathnames to a known source or null', () => { - expect(resolveHookSource('/hook/claude')).toBe('claude') - expect(resolveHookSource('/hook/cursor')).toBe('cursor') - expect(resolveHookSource('/hook/antigravity')).toBe('antigravity') - expect(resolveHookSource('/hook/grok')).toBe('grok') - expect(resolveHookSource('/hook/hermes')).toBe('hermes') - expect(resolveHookSource('/hook/pi')).toBe('pi') - expect(resolveHookSource('/hook/omp')).toBe('omp') - expect(resolveHookSource('/hook/prime-agent')).toBe('prime-agent') - expect(resolveHookSource('/hook/command-code')).toBe('command-code') - expect(resolveHookSource('/hook/opencode2')).toBe('opencode2') - expect(resolveHookSource('/hook/mimo-code')).toBe('mimo-code') + it('rejects root and unknown hook routes', () => { expect(resolveHookSource('/hook/unknown')).toBeNull() expect(resolveHookSource('/')).toBeNull() }) @@ -295,7 +288,7 @@ describe('shared agent-hook-listener', () => { rmSync(dir, { recursive: true, force: true }) }) - it('writes the endpoint file atomically with the right contents and mode', () => { + it('writes endpoint fields with owner-only POSIX permissions', () => { const finalPath = join(dir, getEndpointFileName()) const ok = writeEndpointFile(dir, finalPath, { port: 12345, @@ -326,6 +319,21 @@ describe('shared agent-hook-listener', () => { version: '1' }) expect(ok).toBe(false) + expect(readdirSync(dir)).toEqual([]) }) }) + + it('preserves a large JSON body across repeated buffer growth from one-byte chunks', async () => { + const req = createReadableRequest({ 'content-type': 'application/json' }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The request harness supplies the headers and stream events consumed by the reader. + const reading = readRequestBody(req as unknown as IncomingMessage) + const value = 'x'.repeat(100_000) + const bytes = Buffer.from(JSON.stringify({ value })) + for (let index = 0; index < bytes.length; index += 1) { + req.emit('data', bytes.subarray(index, index + 1)) + } + req.emit('end') + await expect(reading).resolves.toEqual({ value }) + expectRequestParserListenersReleased(req) + }) }) diff --git a/src/shared/agent-hook-listener/claude-settled-permission-requests.test.ts b/src/shared/agent-hook-listener/claude-settled-permission-requests.test.ts index fa4e09e90aa..73bcbf2b60a 100644 --- a/src/shared/agent-hook-listener/claude-settled-permission-requests.test.ts +++ b/src/shared/agent-hook-listener/claude-settled-permission-requests.test.ts @@ -63,7 +63,11 @@ describe('retained Claude request settlements', () => { retained = retainClaudeSettledRequests(retained, base, ended(`tool-${index}`)) } expect(retained).toHaveLength(MAX_CLAUDE_SETTLED_REQUESTS) - expect(retained?.at(-1)).toEqual({ toolUseId: 'tool-39' }) + expect(retained).toEqual( + Array.from({ length: MAX_CLAUDE_SETTLED_REQUESTS }, (_, index) => ({ + toolUseId: `tool-${40 - MAX_CLAUDE_SETTLED_REQUESTS + index}` + })) + ) }) it('rejects a malformed or oversized wire list whole', () => { @@ -86,13 +90,16 @@ describe('retained Claude request settlements', () => { hookEventName: 'PermissionRequest', toolUseId: 'tool-1', toolAgentId: 'child-1', + toolAgentType: 'Explore', payload: { ...base.payload, state: 'waiting' } } const replay: AgentHookEventPayload = { ...base, connectionId: 'conn-1', isReplay: true, - claudeSettledRequests: [{ toolUseId: 'tool-1', toolAgentId: 'child-1' }] + claudeSettledRequests: [ + { toolUseId: 'tool-1', toolAgentId: 'child-1', toolAgentType: 'Explore' } + ] } expect(isClaudePermissionSettledByReplay(waiting, replay)).toBe(true) expect(isClaudePermissionSettledByReplay(waiting, { ...replay, isReplay: undefined })).toBe( @@ -107,5 +114,17 @@ describe('retained Claude request settlements', () => { expect(isClaudePermissionSettledByReplay(waiting, { ...replay, launchToken: 'launch-2' })).toBe( false ) + expect(isClaudePermissionSettledByReplay({ ...waiting, toolUseId: 'tool-2' }, replay)).toBe( + false + ) + expect(isClaudePermissionSettledByReplay({ ...waiting, toolAgentType: 'Review' }, replay)).toBe( + false + ) + expect( + isClaudePermissionSettledByReplay(waiting, { + ...replay, + providerSession: { key: 'session_id', id: 'session-2' } + }) + ).toBe(false) }) }) diff --git a/src/shared/agent-hook-listener/reverse-file-region-scan.test.ts b/src/shared/agent-hook-listener/reverse-file-region-scan.test.ts index 8e929a83e76..3f2ba264d79 100644 --- a/src/shared/agent-hook-listener/reverse-file-region-scan.test.ts +++ b/src/shared/agent-hook-listener/reverse-file-region-scan.test.ts @@ -34,8 +34,12 @@ describe('scanFileRegionsBackward', () => { const lines = Array.from({ length: 40 }, (_, i) => `line-${i}`) const file = writeFixture(`${lines.join('\n')}\n`) for (const chunkBytes of [8, 16, 64, 4096]) { - // Why sorted: the scan walks backwards, so the caller sees later lines first. - expect([...collectLines(file, chunkBytes)].sort()).toEqual([...lines].sort()) + const regions: string[][] = [] + scanFileRegionsBackward(file, { chunkBytes, maxScanBytes: 1024 * 1024 }, (region) => { + regions.push(region.toString('utf8').split('\n').filter(Boolean)) + return undefined + }) + expect(regions.toReversed().flat()).toEqual(lines) } }) @@ -55,13 +59,20 @@ describe('scanFileRegionsBackward', () => { }) it('reports the byte offset each region starts at', () => { - const file = writeFixture('alpha\nbravo\ncharlie\n') - const offsets = scanFileRegionsBackward( + const file = writeFixture('😀alpha\nbravo\ncharlie\n') + const offsets: { text: string; regionPosition: number }[] = [] + scanFileRegionsBackward( file, - { chunkBytes: 4096, maxScanBytes: 4096 }, - (region, regionPosition) => ({ text: region.toString('utf8'), regionPosition }) + { chunkBytes: 8, maxScanBytes: 4096 }, + (region, regionPosition) => { + offsets.push({ text: region.toString('utf8'), regionPosition }) + return undefined + } ) - expect(offsets).toEqual({ text: 'alpha\nbravo\ncharlie\n', regionPosition: 0 }) + expect(offsets).toEqual([ + { text: 'bravo\ncharlie', regionPosition: 10 }, + { text: '😀alpha', regionPosition: 0 } + ]) }) it('drops the leading partial line when it stops on the byte budget', () => { @@ -72,10 +83,10 @@ describe('scanFileRegionsBackward', () => { }) it('reads a file with no trailing newline', () => { - expect(collectLines(writeFixture('alpha\nbravo'), 4)).toContain('bravo') + expect(collectLines(writeFixture('alpha\nbravo'), 4)).toEqual(['bravo', 'alpha']) }) - it('treats an empty, missing, or unreadable file as nothing found', () => { + it('treats an empty or missing file as nothing found', () => { expect(collectLines(writeFixture(''), 64)).toEqual([]) expect( scanFileRegionsBackward( diff --git a/src/shared/agent-hook-request-body-memory.test.ts b/src/shared/agent-hook-request-body-memory.test.ts deleted file mode 100644 index ce0c8029dd0..00000000000 --- a/src/shared/agent-hook-request-body-memory.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { EventEmitter } from 'node:events' -import type { IncomingHttpHeaders, IncomingMessage } from 'node:http' -import { describe, expect, it, vi } from 'vitest' -import { readRequestBody } from './agent-hook-listener/request-body' - -type FakeIncomingMessage = EventEmitter & { - headers: IncomingHttpHeaders - destroy: ReturnType -} - -function createReadableRequest(): FakeIncomingMessage { - const request = new EventEmitter() as FakeIncomingMessage - request.headers = { 'content-type': 'application/json' } - request.destroy = vi.fn(() => request.emit('close')) - return request -} - -describe('agent hook request body retention', () => { - it('accepts adversarial one-byte events without per-event retained buffers', async () => { - const request = createReadableRequest() - const reading = readRequestBody(request as unknown as IncomingMessage) - const value = 'x'.repeat(100_000) - const body = Buffer.from(JSON.stringify({ value })) - - for (let index = 0; index < body.length; index += 1) { - request.emit('data', body.subarray(index, index + 1)) - } - request.emit('end') - - await expect(reading).resolves.toEqual({ value }) - }) -})