Skip to content

Commit 1ac7ac2

Browse files
authored
fix(desktop): correct history, file paths and Bun runtime regressions (#1570)
* fix(desktop): address history and file review QA regressions * fix(desktop): preserve decoded absolute paths in output cards * test(team): wait for persisted member output before restart * fix(search): wait for worker termination before resuming commits * fix(runtime): pin Bun 1.4.3 for SQLite worker reliability * fix(runtime): resolve Bun 1.4 lifecycle and fixture regressions * test(team): persist the first turn before owner-loss recovery
1 parent 2447370 commit 1ac7ac2

45 files changed

Lines changed: 1208 additions & 156 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎desktop/electron/services/systemProxyBridge.test.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import http from 'node:http'
22
import net from 'node:net'
3+
import { EventEmitter } from 'node:events'
34
import { PassThrough, type Duplex } from 'node:stream'
45
import { afterEach, describe, expect, it, vi } from 'vitest'
56
import {
@@ -327,6 +328,60 @@ describe('SystemProxyBridge', () => {
327328
expect(new URL(restartedUrl).hostname).toBe('127.0.0.1')
328329
})
329330

331+
it('waits for the listen callback before closing a pending listener', async () => {
332+
const pending = createPendingProxyServer(41001)
333+
const createServer = vi.spyOn(http, 'createServer').mockReturnValueOnce(pending.server)
334+
const bridge = new SystemProxyBridge(async () => 'DIRECT')
335+
bridges.push(bridge)
336+
const startup = bridge.start()
337+
const startupResult = startup.catch(error => error)
338+
const stopping = bridge.stop()
339+
340+
try {
341+
expect(pending.server.listening).toBe(true)
342+
expect(pending.server.closeAllConnections).not.toHaveBeenCalled()
343+
expect(pending.server.close).not.toHaveBeenCalled()
344+
pending.finishListening()
345+
await stopping
346+
expect(await startupResult).toEqual(new Error('System proxy bridge startup was stopped'))
347+
expect(pending.server.close).toHaveBeenCalledTimes(1)
348+
} finally {
349+
pending.finishListening()
350+
await stopping
351+
createServer.mockRestore()
352+
}
353+
})
354+
355+
it('keeps a newer listener active when an older pending startup is stopped', async () => {
356+
const first = createPendingProxyServer(41001)
357+
const second = createPendingProxyServer(41002)
358+
const createServer = vi.spyOn(http, 'createServer')
359+
.mockReturnValueOnce(first.server)
360+
.mockReturnValueOnce(second.server)
361+
const bridge = new SystemProxyBridge(async () => 'DIRECT')
362+
bridges.push(bridge)
363+
const firstStartup = bridge.start().catch(error => error)
364+
const stopping = bridge.stop()
365+
const secondStartup = bridge.start()
366+
367+
try {
368+
second.finishListening()
369+
expect(await secondStartup).toBe('http://127.0.0.1:41002')
370+
first.finishListening()
371+
await stopping
372+
expect(await firstStartup).toEqual(new Error('System proxy bridge startup was stopped'))
373+
expect(second.server.listening).toBe(true)
374+
expect(second.server.closeAllConnections).not.toHaveBeenCalled()
375+
expect(second.server.close).not.toHaveBeenCalled()
376+
expect(await bridge.start()).toBe('http://127.0.0.1:41002')
377+
} finally {
378+
first.finishListening()
379+
second.finishListening()
380+
await stopping
381+
createServer.mockRestore()
382+
}
383+
})
384+
330385
it('closes active CONNECT clients and outbound routes during stop', async () => {
331386
let acceptTarget!: () => void
332387
const targetAccepted = new Promise<void>(resolve => { acceptTarget = resolve })
@@ -357,6 +412,33 @@ describe('SystemProxyBridge', () => {
357412
})
358413
})
359414

415+
function createPendingProxyServer(port: number) {
416+
const server = new EventEmitter() as http.Server
417+
let listening = false
418+
let listenCallback: (() => void) | undefined
419+
Object.defineProperty(server, 'listening', { get: () => listening })
420+
server.listen = vi.fn((...args: unknown[]) => {
421+
listening = true
422+
listenCallback = args.at(-1) as () => void
423+
return server
424+
}) as typeof server.listen
425+
server.close = vi.fn((callback?: () => void) => {
426+
listening = false
427+
callback?.()
428+
return server
429+
}) as typeof server.close
430+
server.closeAllConnections = vi.fn()
431+
server.address = vi.fn(() => ({ address: '127.0.0.1', family: 'IPv4', port }))
432+
return {
433+
server,
434+
finishListening() {
435+
const callback = listenCallback
436+
listenCallback = undefined
437+
callback?.()
438+
},
439+
}
440+
}
441+
360442
async function startBridge(
361443
resolver: (url: string) => Promise<string>,
362444
): Promise<string> {

‎desktop/electron/services/systemProxyBridge.ts‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,10 @@ export class SystemProxyBridge implements SystemProxyBridgeLike {
8181
this.server = null
8282
for (const socket of this.clientSockets) socket.destroy()
8383
for (const socket of this.outboundSockets) socket.destroy()
84-
// Bun needs the HTTP connection registry drained before close() will
85-
// finish after an upgraded CONNECT socket. Calling this before listen
86-
// completes breaks the separate startup/stop race, so guard on listening.
87-
if (server?.listening) server.closeAllConnections()
88-
const closing = server?.listening
89-
? new Promise<void>(resolve => server.close(() => resolve()))
90-
: Promise.resolve()
91-
await closing
84+
// Bun can report listening before its listen callback runs. Closing then
85+
// suppresses that callback, so let startup observe cancellation first.
9286
await startPromise?.catch(() => {})
87+
if (server) await closeServer(server)
9388
}
9489

9590
private async startOnce(generation: number): Promise<string> {
@@ -456,6 +451,9 @@ class SingleSocketAgent extends http.Agent {
456451

457452
function closeServer(server: http.Server): Promise<void> {
458453
if (!server.listening) return Promise.resolve()
454+
// Drain Bun's HTTP connection registry after an upgraded CONNECT socket.
455+
// Callers wait for the listen callback before closing the server.
456+
server.closeAllConnections()
459457
return new Promise(resolve => server.close(() => resolve()))
460458
}
461459

‎desktop/src/__tests__/memorySettings.test.tsx‎

Lines changed: 35 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { useMemoryStore } from '../stores/memoryStore'
77
import { useSessionStore } from '../stores/sessionStore'
88
import { useSettingsStore } from '../stores/settingsStore'
99
import { useUIStore } from '../stores/uiStore'
10+
import type { PreviewLinkTarget } from '@/lib/previewLinkRouter'
1011

1112
const { memoryApiMock } = vi.hoisted(() => ({
1213
memoryApiMock: {
@@ -21,30 +22,34 @@ vi.mock('../api/memory', () => ({
2122
memoryApi: memoryApiMock,
2223
}))
2324

24-
vi.mock('../components/markdown/MarkdownRenderer', () => ({
25-
MarkdownRenderer: ({ content, onLinkClick }: { content: string; onLinkClick?: (href: string) => boolean | void }) => {
26-
const match = content.match(/\[([^\]]+)\]\(([^)]+)\)/)
27-
const linkText = match?.[1]
28-
const linkHref = match?.[2]
29-
return (
30-
<div data-testid="markdown-preview">
31-
{content}
32-
{linkText && linkHref ? (
33-
<a
34-
href={linkHref}
35-
onClick={(event) => {
36-
if (onLinkClick?.(linkHref)) {
37-
event.preventDefault()
38-
}
39-
}}
40-
>
41-
{linkText}
42-
</a>
43-
) : null}
44-
</div>
45-
)
46-
},
47-
}))
25+
vi.mock('../components/markdown/MarkdownRenderer', async () => {
26+
const { classifyPreviewLink } = await import('@/lib/previewLinkRouter')
27+
return {
28+
MarkdownRenderer: ({ content, onLinkClick }: { content: string; onLinkClick?: (href: PreviewLinkTarget) => boolean | void }) => {
29+
const match = content.match(/\[([^\]]+)\]\(([^)]+)\)/)
30+
const linkText = match?.[1]
31+
const linkHref = match?.[2]
32+
return (
33+
<div data-testid="markdown-preview">
34+
{content}
35+
{linkText && linkHref ? (
36+
<a
37+
href={linkHref}
38+
onClick={(event) => {
39+
const parsed = classifyPreviewLink(linkHref)
40+
if (onLinkClick?.(parsed.path ? { path: parsed.path } : linkHref)) {
41+
event.preventDefault()
42+
}
43+
}}
44+
>
45+
{linkText}
46+
</a>
47+
) : null}
48+
</div>
49+
)
50+
},
51+
}
52+
})
4853

4954
describe('MemorySettings', () => {
5055
beforeEach(() => {
@@ -487,7 +492,8 @@ describe('MemorySettings', () => {
487492
expect(screen.queryByLabelText('Editor')).not.toBeInTheDocument()
488493
})
489494

490-
it('opens linked memory markdown files from the rendered preview', async () => {
495+
it.each(['notes/manual.md', 'notes/literal%20name.md', 'notes/name#hash.md'])(
496+
'opens linked memory markdown file %s from the parsed preview', async (targetPath) => {
491497
memoryApiMock.listFiles.mockResolvedValue({
492498
files: [
493499
{
@@ -501,7 +507,7 @@ describe('MemorySettings', () => {
501507
isIndex: true,
502508
},
503509
{
504-
path: 'notes/manual.md',
510+
path: targetPath,
505511
name: 'manual.md',
506512
title: 'Manual',
507513
bytes: 24,
@@ -514,9 +520,9 @@ describe('MemorySettings', () => {
514520
memoryApiMock.readFile.mockImplementation((_projectId: string, path: string) => Promise.resolve({
515521
file: {
516522
path,
517-
content: path === 'notes/manual.md'
523+
content: path !== 'MEMORY.md'
518524
? '# Manual\n'
519-
: '# Project Memory\n\n- [Manual](notes/manual.md)\n',
525+
: `# Project Memory\n\n- [Manual](${targetPath.split('/').map(encodeURIComponent).join('/')})\n`,
520526
updatedAt: '2026-05-01T00:00:00.000Z',
521527
bytes: 48,
522528
},
@@ -527,7 +533,7 @@ describe('MemorySettings', () => {
527533
fireEvent.click(await screen.findByRole('link', { name: 'Manual' }))
528534

529535
await waitFor(() => {
530-
expect(memoryApiMock.readFile).toHaveBeenCalledWith('-workspace-demo', 'notes/manual.md')
536+
expect(memoryApiMock.readFile).toHaveBeenCalledWith('-workspace-demo', targetPath)
531537
})
532538
expect(await screen.findByTestId('markdown-preview')).toHaveTextContent('Manual')
533539
expect(screen.queryByLabelText('Editor')).not.toBeInTheDocument()

‎desktop/src/components/chat/AssistantMessage.filepaths.test.tsx‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,79 @@ afterEach(() => {
106106
})
107107

108108
describe('AssistantMessage file references', () => {
109+
it.each([
110+
['[open](/work/My%20Notes.md)', '/work/My Notes.md'],
111+
['[open](/work/literal%2520name.md)', '/work/literal%20name.md'],
112+
['输出目录:`/other/QA%20root`\n[open](My%20Notes.md)', '/other/QA%20root/My Notes.md'],
113+
])('opens the reconciled card and prose at the same parsed file for %s', async (content, path) => {
114+
// The server confirms only the actual disk path, so an encoded URI cannot
115+
// accidentally get a card just because a permissive mock says it exists.
116+
statWorkspaceFiles.mockImplementation(async (_sessionId: string, paths: string[]) => ({
117+
files: paths.map((candidate) => ({
118+
path: candidate, state: candidate === path ? 'file' : 'missing', mtimeMs: Date.now(),
119+
})),
120+
}))
121+
const view = render(<AssistantMessage
122+
sessionId="s1" content={content} turnChangedFiles={[]} turnOutputEvidence={shellTurn}
123+
/>)
124+
fireEvent.click(await screen.findByRole('button', { name: 'assistantOutputs.openAria' }))
125+
await waitFor(() => expect(openPreviewFn).toHaveBeenCalledWith('s1', path, {}))
126+
openPreviewFn.mockClear()
127+
fireEvent.click(screen.getByRole('link', { name: 'open' }))
128+
await waitFor(() => expect(openPreviewFn).toHaveBeenCalledWith('s1', path, {}))
129+
view.unmount()
130+
resetWorkspaceFileStatsForTests()
131+
resetDiskListingCacheForTests()
132+
openPreviewFn.mockClear()
133+
134+
// The exact changed-file match must keep the card when no unseen writes
135+
// can explain an unmatched output.
136+
render(<AssistantMessage
137+
sessionId="s1" content={content} turnChangedFiles={[path]} turnOutputEvidence={{ unlistedWrites: false }}
138+
/>)
139+
fireEvent.click(await screen.findByRole('button', { name: 'assistantOutputs.openAria' }))
140+
await waitFor(() => expect(openPreviewFn).toHaveBeenCalledWith('s1', path, {}))
141+
})
142+
143+
// Native QA: the renderer had decoded the Markdown destination into data-file-path,
144+
// then the click router decoded that filesystem path a second time.
145+
it.each([
146+
['[open](docs/literal%2520name.md)', 'docs/literal%20name.md', {}],
147+
['[open](/work/literal%2525name.md:7:3)', '/work/literal%25name.md', { line: 7, column: 3 }],
148+
['[open](C:/QA/literal%2523name.md#L9)', 'C:/QA/literal%23name.md', { line: 9 }],
149+
['[open](file:///work/literal%252Fname.md)', '/work/literal%2Fname.md', {}],
150+
['[open](docs/My%20Notes.md:3)', 'docs/My Notes.md', { line: 3 }],
151+
['[open](file:///work/My%20Notes.md)', '/work/My Notes.md', {}],
152+
['[open](file:///C:/QA/My%20Notes.md#L9)', 'C:/QA/My Notes.md', { line: 9 }],
153+
['[open](docs/name%23hash.md#L2)', 'docs/name#hash.md', { line: 2 }],
154+
['[open](docs/100%.md)', 'docs/100%.md', {}],
155+
])('opens the exact disk path from rendered Markdown %s', async (content, path, reveal) => {
156+
render(<AssistantMessage sessionId="s1" content={content} />)
157+
fireEvent.click(screen.getByRole('link', { name: 'open' }))
158+
await waitFor(() => expect(openPreviewFn).toHaveBeenCalledWith('s1', path, reveal))
159+
expect(statWorkspaceFiles).toHaveBeenCalledWith('s1', expect.arrayContaining([path]))
160+
})
161+
162+
it.each(['%20', '%25', '%23', '%2F'])(
163+
'opens literal %s names in bare and inline-code references under the declared root', async (percent) => {
164+
const path = `docs/literal${percent}name.md`
165+
const content = `输出目录:\`/work/QA%20root\`\n\n${path}:7\n\n\`${path}:9:3\``
166+
render(<AssistantMessage sessionId="s1" content={content} />)
167+
fireEvent.click(await screen.findByRole('link', { name: `${path}:7` }))
168+
expect(openPreviewFn).toHaveBeenCalledWith('s1', `/work/QA%20root/${path}`, { line: 7 })
169+
fireEvent.click(await screen.findByRole('link', { name: `${path}:9:3` }))
170+
expect(openPreviewFn).toHaveBeenCalledWith('s1', `/work/QA%20root/${path}`, { line: 9, column: 3 })
171+
},
172+
)
173+
174+
it('uses the same literal percent filename in a Markdown context menu', async () => {
175+
render(<AssistantMessage sessionId="s1" content={'[open](docs/literal%2520name.md:7)'} />)
176+
fireEvent.contextMenu(screen.getByRole('link', { name: 'open' }))
177+
await waitFor(() => expect(getTargetsForPath).toHaveBeenCalledWith('/work/docs/literal%20name.md'))
178+
fireEvent.click(await screen.findByRole('menuitem', { name: 'openWith.copyPath' }))
179+
expect(copyTextToClipboard).toHaveBeenCalledWith('/work/docs/literal%20name.md')
180+
})
181+
109182
it('opens the code view at the referenced line', async () => {
110183
// #1146, and the contract src/constants/prompts.ts already asks the model for.
111184
render(<AssistantMessage sessionId="s1" content={'越界在 desktop/src/lib/foo.ts:42'} isStreaming={false} />)

‎desktop/src/components/chat/AssistantMessage.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { useTranslation, type TranslationKey } from '../../i18n'
2525
import { useDiskConfirmedTargets } from '../../hooks/useDiskConfirmedTargets'
2626
import { useTurnWrittenTargets } from '../../hooks/useTurnWrittenTargets'
2727
import { createWorkspaceFileLinkVerifier } from '../../lib/workspaceFileStats'
28+
import type { PreviewLinkTarget } from '@/lib/previewLinkRouter'
2829

2930
type Props = {
3031
content: string
@@ -64,7 +65,7 @@ export const AssistantMessage = memo(function AssistantMessage({
6465
const [viewer, setViewer] = useState<{ images: Array<{ src: string; name: string; path?: string }>; index: number } | null>(null)
6566

6667
const handleLinkClick = useCallback(
67-
(href: string, event: ReactMouseEvent<HTMLDivElement>): boolean => {
68+
(href: PreviewLinkTarget, event: ReactMouseEvent<HTMLDivElement>): boolean => {
6869
if (!sessionId) return false
6970
const handled = openPreviewLink(resolveAssistantFileHref(href, content), sessionId)
7071
if (handled) event.preventDefault()

‎desktop/src/components/chat/AssistantOutputTargetCard.test.tsx‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,15 @@ afterEach(() => {
101101
})
102102

103103
describe('AssistantOutputTargetCard', () => {
104+
it.each(['%20', '%25', '%23', '%2F'])(
105+
'opens a written file with a literal %s in its filename', async (percent) => {
106+
const path = `docs/literal${percent}name.md`
107+
render(<AssistantOutputTargetCard target={{ ...markdownTarget, href: path, normalizedPath: path, source: 'changed-file' }} sessionId="s1" />)
108+
fireEvent.click(screen.getByLabelText('assistantOutputs.open'))
109+
await waitFor(() => expect(openPreviewFn).toHaveBeenCalledWith('s1', path, {}))
110+
},
111+
)
112+
104113
it('renders a markdown target title + Markdown badge', () => {
105114
render(<AssistantOutputTargetCard target={markdownTarget} sessionId="s1" />)
106115
expect(screen.getByText('readme.md')).toBeInTheDocument()

‎desktop/src/components/chat/AssistantOutputTargetCard.tsx‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { describeFileType, type OpenWithItem } from '../../lib/openWithItems'
1212
import { buildOpenWithMenuItemsForHref } from '../../lib/openWithMenuItems'
1313
import { openPreviewLink } from '../../lib/openPreviewLink'
1414
import { reportOpenFailure } from '../../lib/systemFileOpen'
15+
import type { PreviewLinkTarget } from '@/lib/previewLinkRouter'
1516

1617
type Props = {
1718
target: AssistantOutputTarget
@@ -24,6 +25,10 @@ export function AssistantOutputTargetCard({ target, sessionId, workDir }: Props)
2425
const [openWith, setOpenWith] = useState<{ items: OpenWithItem[]; anchor: DOMRect; triggerEl: HTMLElement } | null>(null)
2526

2627
const isLocalhost = target.kind === 'localhost-url'
28+
// normalizedPath is a disk path, even when the authored href was encoded.
29+
const linkTarget: PreviewLinkTarget = isLocalhost
30+
? target.href
31+
: { path: target.normalizedPath ?? target.href }
2732
const typeInfo = describeFileType(target.normalizedPath ?? target.href)
2833
const badge = isLocalhost
2934
? t('assistantOutputs.kind.localhost')
@@ -41,8 +46,8 @@ export function AssistantOutputTargetCard({ target, sessionId, workDir }: Props)
4146
event.stopPropagation()
4247
// A card is only shown for something it can open; one the router cannot
4348
// place must say so rather than leave the click looking dead (#1556).
44-
if (!openPreviewLink(target.href, sessionId)) reportOpenFailure(target.href)
45-
}, [sessionId, target.href])
49+
if (!openPreviewLink(linkTarget, sessionId)) reportOpenFailure(target.href)
50+
}, [sessionId, linkTarget, target.href])
4651

4752
const handleOpenWith = useCallback((event: ReactMouseEvent<HTMLButtonElement>) => {
4853
event.stopPropagation()
@@ -56,15 +61,15 @@ export function AssistantOutputTargetCard({ target, sessionId, workDir }: Props)
5661
const triggerEl = event.currentTarget
5762
const rect = triggerEl.getBoundingClientRect()
5863
void (async () => {
59-
const items = await buildOpenWithMenuItemsForHref(target.href, {
64+
const items = await buildOpenWithMenuItemsForHref(linkTarget, {
6065
sessionId,
6166
workDir,
6267
t: (k, v) => t(k as TranslationKey, v),
6368
})
6469
if (items.length === 0) return
6570
setOpenWith({ items, anchor: rect, triggerEl })
6671
})()
67-
}, [openWith, sessionId, t, target.href, workDir])
72+
}, [openWith, sessionId, t, linkTarget, workDir])
6873

6974
return (
7075
<section className="flex items-stretch overflow-hidden rounded-[var(--radius-lg)] border border-[var(--color-border)] bg-[var(--color-surface-container-lowest)]">

0 commit comments

Comments
 (0)