Fix cross-worktree chat leak: key native-chat transcript cache by resolved file path (#7341)
This commit is contained in:
parent
7c10abc582
commit
533cafdfa9
|
|
@ -17,6 +17,7 @@ vi.mock('./transcript-reader', async (importOriginal) => {
|
|||
}
|
||||
})
|
||||
|
||||
import { isTextBlock } from '../../shared/native-chat-types'
|
||||
import {
|
||||
clearNativeChatTranscriptCache,
|
||||
readNativeChatTranscriptCached
|
||||
|
|
@ -89,4 +90,62 @@ describe('readNativeChatTranscriptCached', () => {
|
|||
const result = await readNativeChatTranscriptCached('claude', 'absent')
|
||||
expect('error' in result && result.error).toBeTruthy()
|
||||
})
|
||||
|
||||
// Why: two worktrees can present the SAME (agent, sessionId) via different
|
||||
// transcript files — e.g. the same session resumed into a second worktree,
|
||||
// which writes a new transcript file. Keying the cache by sessionId let one
|
||||
// worktree's cached parse be served to the other whenever their file mtimes
|
||||
// coincided, leaking A's chat transcript into C's panel (#7326).
|
||||
it('never serves one file’s parse for a different file that shares a sessionId', async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), 'orca-native-chat-cache-xwt-'))
|
||||
tempRoots.push(root)
|
||||
const fileA = join(root, 'worktree-a.jsonl')
|
||||
const fileC = join(root, 'worktree-c.jsonl')
|
||||
await writeFile(
|
||||
fileA,
|
||||
jsonLines([
|
||||
{
|
||||
type: 'user',
|
||||
uuid: 'a0',
|
||||
timestamp: '2026-06-01T10:00:00.000Z',
|
||||
message: { role: 'user', content: 'from-worktree-A' }
|
||||
}
|
||||
])
|
||||
)
|
||||
await writeFile(
|
||||
fileC,
|
||||
jsonLines([
|
||||
{
|
||||
type: 'user',
|
||||
uuid: 'c0',
|
||||
timestamp: '2026-06-01T10:00:00.000Z',
|
||||
message: { role: 'user', content: 'from-worktree-C' }
|
||||
}
|
||||
])
|
||||
)
|
||||
// Force IDENTICAL mtimes so a sessionId-only key's mtime guard cannot rescue
|
||||
// the collision — this is the intermittent, activity-driven case.
|
||||
const when = new Date('2026-06-01T10:00:00.000Z')
|
||||
await utimes(fileA, when, when)
|
||||
await utimes(fileC, when, when)
|
||||
|
||||
const readText = (result: Awaited<ReturnType<typeof readNativeChatTranscriptCached>>): string =>
|
||||
'messages' in result
|
||||
? result.messages
|
||||
.flatMap((message) => message.blocks)
|
||||
.filter(isTextBlock)
|
||||
.map((block) => block.text)
|
||||
.join(' ')
|
||||
: ''
|
||||
|
||||
// Same sessionId, different transcript files (worktree A resumed into C).
|
||||
const a = await readNativeChatTranscriptCached('claude', 'shared-session', fileA)
|
||||
const c = await readNativeChatTranscriptCached('claude', 'shared-session', fileC)
|
||||
|
||||
expect(readText(a)).toContain('from-worktree-A')
|
||||
expect(readText(c)).toContain('from-worktree-C')
|
||||
expect(readText(c)).not.toContain('from-worktree-A')
|
||||
// Distinct files must not share a cached parse object.
|
||||
expect(c).not.toBe(a)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -4,13 +4,18 @@ import { resolveSessionFilePath } from './session-file-resolver'
|
|||
import { readNativeChatTranscript, type ReadTranscriptResult } from './transcript-reader'
|
||||
|
||||
// Why: both the desktop IPC handler and the runtime RPC handler read the same
|
||||
// host-filesystem transcript, so a single process-global cache keyed by
|
||||
// agent:sessionId maximizes the hit rate across desktop + every paired
|
||||
// web/mobile client. Keying by connection instead would defeat the multi-client
|
||||
// case this feature targets and multiply memory by the connection count.
|
||||
// The cache stores ONE canonical, unwindowed parse; windowing and per-surface
|
||||
// truncation stay in the callers so the same parse is reused across all `limit`
|
||||
// values and every client kind.
|
||||
// host-filesystem transcript, so a single process-global cache keyed by the
|
||||
// RESOLVED transcript file path maximizes the hit rate across desktop + every
|
||||
// paired web/mobile client (all clients of one session resolve the same path
|
||||
// against this runtime's home). Keying by connection instead would defeat the
|
||||
// multi-client case this feature targets and multiply memory by the connection
|
||||
// count. The key is the resolved file path, NOT `agent:sessionId`: two panes can
|
||||
// share one sessionId yet resolve to DIFFERENT files (the same session resumed
|
||||
// into a second worktree, which writes a new transcript file), and a
|
||||
// sessionId-only key let one worktree's cached parse be served to another when
|
||||
// their file mtimes momentarily coincided (#7326). The cache stores ONE
|
||||
// canonical, unwindowed parse; windowing and per-surface truncation stay in the
|
||||
// callers so the same parse is reused across all `limit` values and every client kind.
|
||||
|
||||
type CachedTranscript = {
|
||||
result: ReadTranscriptResult
|
||||
|
|
@ -40,8 +45,8 @@ function setCached(key: string, value: CachedTranscript): void {
|
|||
}
|
||||
}
|
||||
|
||||
function cacheKey(agent: AgentType, sessionId: string): string {
|
||||
return `${agent}:${sessionId}`
|
||||
function cacheKey(agent: AgentType, filePath: string): string {
|
||||
return `${agent}:${filePath}`
|
||||
}
|
||||
|
||||
async function fileMtimeMs(filePath: string): Promise<number> {
|
||||
|
|
@ -68,7 +73,7 @@ export async function readNativeChatTranscriptCached(
|
|||
return { error: `No transcript found for ${agent} session ${sessionId}` }
|
||||
}
|
||||
|
||||
const key = cacheKey(agent, sessionId)
|
||||
const key = cacheKey(agent, filePath)
|
||||
const mtimeMs = await fileMtimeMs(filePath)
|
||||
const cached = cache.get(key)
|
||||
if (cached && Number.isFinite(mtimeMs) && cached.mtimeMs === mtimeMs) {
|
||||
|
|
@ -84,7 +89,7 @@ export async function readNativeChatTranscriptCached(
|
|||
return result
|
||||
}
|
||||
|
||||
/** Test-only: drop the per-session transcript cache between runs. */
|
||||
/** Test-only: drop the transcript parse cache between runs. */
|
||||
export function clearNativeChatTranscriptCache(): void {
|
||||
cache.clear()
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue