Skip to content

Commit ca72251

Browse files
committed
fix(desktop): linear path trim, prompt Stop between keystrokes, clear a closed session's input marker
1 parent 8760ca7 commit ca72251

11 files changed

Lines changed: 127 additions & 23 deletions

File tree

‎apps/desktop/src/main/browser-agent/session.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,6 +1071,22 @@ describe('browser-agent session', () => {
10711071
expect(session.listTabs()).toHaveLength(1)
10721072
})
10731073

1074+
it('forgets the user input marker when the session closes, so a reused tab id starts clean', () => {
1075+
const agent = session.ensureAutomationTab()
1076+
const keyDown = (agent.view as unknown as MockView).webContents.on.mock.calls.find(
1077+
([eventName]) => eventName === 'before-input-event'
1078+
)?.[1]
1079+
if (typeof keyDown !== 'function') throw new Error('no before-input-event listener bound')
1080+
keyDown({ preventDefault: vi.fn() }, { type: 'keyDown', isAutoRepeat: false })
1081+
expect(session.msSinceUserIntervention()).not.toBeNull()
1082+
1083+
session.closeSession()
1084+
const reopened = session.ensureAutomationTab()
1085+
1086+
expect(reopened.id).toBe(agent.id)
1087+
expect(session.msSinceUserIntervention()).toBeNull()
1088+
})
1089+
10741090
it('opens, switches, and closes tabs with stable ids', () => {
10751091
const first = session.ensureTab()
10761092
const second = session.addTab()

‎apps/desktop/src/main/browser-agent/session.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3646,6 +3646,7 @@ function closeLiveTabs(): void {
36463646
currentScope.automationActive = false
36473647
currentScope.automationNeedsAttention = false
36483648
currentScope.visibleTabUserSelected = false
3649+
currentScope.userIntervention = null
36493650
clearFocusedBrowserTab()
36503651
}
36513652

‎apps/desktop/src/main/local-filesystem.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,27 @@ describe('LocalFilesystemService', () => {
107107
})
108108
})
109109

110+
it('trims long runs of slashes in grep and glob paths in linear time', async () => {
111+
const granted = await mount(service)
112+
const slashes = `${'/'.repeat(200_000)}x`
113+
const startedAt = performance.now()
114+
115+
expect(
116+
service.isAuthorizedClientToolRequest(
117+
{ operation: 'grep', uri: granted.uri, pattern: 'TODO', requestId: 'grep-tool' },
118+
{ toolName: 'grep', args: { path: slashes, pattern: 'TODO' } }
119+
)
120+
).toBe(false)
121+
await service.handle({
122+
operation: 'glob',
123+
uri: granted.uri,
124+
pattern: '**/*.ts',
125+
pathPrefix: slashes,
126+
})
127+
128+
expect(performance.now() - startedAt).toBeLessThan(1000)
129+
})
130+
110131
it('binds privileged client reads and searches to server-persisted tool args', async () => {
111132
const granted = await mount(service)
112133
const vfsRoot = `user-local/${encodeURIComponent(granted.name)}--${granted.id}`

‎apps/desktop/src/main/local-filesystem.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
} from '@sim/desktop-bridge/local-filesystem-limits'
2020
import { generateId } from '@sim/utils/id'
2121
import { isRecordLike } from '@sim/utils/object'
22-
import { escapeRegExp, truncate } from '@sim/utils/string'
22+
import { escapeRegExp, stripTrailingSlashes, truncate } from '@sim/utils/string'
2323
import { app, dialog, shell } from 'electron'
2424
import micromatch from 'micromatch'
2525
import safeRegex from 'safe-regex2'
@@ -556,7 +556,7 @@ export class LocalFilesystemService {
556556
// request carrying them is the renderer searching for something the
557557
// model did not ask for, or hiding results it believes are complete.
558558
if (request.query !== undefined || request.include !== undefined) return false
559-
const rawPath = typeof args.path === 'string' ? args.path.replace(/\/+$/, '') : ''
559+
const rawPath = typeof args.path === 'string' ? stripTrailingSlashes(args.path) : ''
560560
const uriAllowed =
561561
rawPath === 'user-local'
562562
? [...this.mounts.values()].some((mount) => request.uri === mount.uri)
@@ -1008,7 +1008,7 @@ export class LocalFilesystemService {
10081008
if (rawPathPrefix !== undefined && typeof rawPathPrefix !== 'string') {
10091009
throw new LocalFilesystemError('INVALID_REQUEST', 'pathPrefix must be a string.')
10101010
}
1011-
const pathPrefix = typeof rawPathPrefix === 'string' ? rawPathPrefix.replace(/\/+$/, '') : ''
1011+
const pathPrefix = typeof rawPathPrefix === 'string' ? stripTrailingSlashes(rawPathPrefix) : ''
10121012
const matcher = compileGlob(pattern)
10131013
const resolvedPath = await this.resolveUri(uri)
10141014
const baseStat = await stat(resolvedPath.realPath)

‎apps/desktop/src/main/terminal/session.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,4 +188,36 @@ describe('TerminalSession command lifecycle', () => {
188188
session.dispose()
189189
}
190190
})
191+
192+
it('ends the wait between keystrokes on Stop while the program keeps redrawing', async () => {
193+
vi.useFakeTimers()
194+
const session = TerminalSession.create({
195+
terminalId: 'terminal-redraw',
196+
cwd: '/tmp',
197+
cols: 80,
198+
rows: 24,
199+
callbacks: { onData: () => {}, onState: () => {}, onCommand: () => {}, onExit: () => {} },
200+
})
201+
const redraw = setInterval(() => ptyStub.dataHandler?.('frame'), 20)
202+
try {
203+
const writesBefore = ptyStub.writes.length
204+
const stop = new AbortController()
205+
let settled = false
206+
const typing = session.type('first\nsecond', stop.signal).then(() => {
207+
settled = true
208+
})
209+
await vi.advanceTimersByTimeAsync(300)
210+
expect(settled).toBe(false)
211+
212+
stop.abort()
213+
await vi.advanceTimersByTimeAsync(1)
214+
expect(settled).toBe(true)
215+
await typing
216+
217+
expect(ptyStub.writes.slice(writesBefore)).toEqual(['first'])
218+
} finally {
219+
clearInterval(redraw)
220+
session.dispose()
221+
}
222+
})
191223
})

‎apps/desktop/src/main/terminal/session.ts‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
type TerminalRunResult,
2424
type TerminalTabState,
2525
} from '@sim/terminal-protocol'
26-
import { sleep } from '@sim/utils/helpers'
26+
import { interruptibleSleep } from '@sim/utils/helpers'
2727
import {
2828
Terminal as HeadlessTerminal,
2929
type IBuffer,
@@ -527,7 +527,7 @@ export class TerminalSession {
527527
async pressKeys(keys: TerminalControlKey[], signal?: AbortSignal): Promise<void> {
528528
for (let index = 0; index < keys.length; index += 1) {
529529
if (this.disposed || signal?.aborted) return
530-
if (index > 0) await this.settleBetweenKeystrokes()
530+
if (index > 0) await this.settleBetweenKeystrokes(signal)
531531
if (signal?.aborted) return
532532
this.sendKey(keys[index])
533533
}
@@ -543,21 +543,24 @@ export class TerminalSession {
543543
const chunks = toInputChunks(text)
544544
for (let index = 0; index < chunks.length; index += 1) {
545545
if (this.disposed || signal?.aborted) return
546-
if (index > 0) await this.settleBetweenKeystrokes()
546+
if (index > 0) await this.settleBetweenKeystrokes(signal)
547547
if (signal?.aborted) return
548548
this.write(chunks[index])
549549
}
550550
}
551551

552-
/** Holds a gap, then lets any resulting redraw finish before the next write. */
553-
private async settleBetweenKeystrokes(): Promise<void> {
554-
await sleep(KEYSTROKE_GAP_MS)
552+
/**
553+
* Holds a gap, then lets any resulting redraw finish before the next write.
554+
* Ends early on Stop, so a program that redraws constantly cannot hold it.
555+
*/
556+
private async settleBetweenKeystrokes(signal?: AbortSignal): Promise<void> {
557+
await interruptibleSleep(KEYSTROKE_GAP_MS, signal)
555558
const deadline = Date.now() + KEYSTROKE_SETTLE_MAX_MS
556-
while (!this.disposed) {
559+
while (!this.disposed && !signal?.aborted) {
557560
const quietFor = Date.now() - this.lastOutputAt
558561
const remaining = Math.min(KEYSTROKE_GAP_MS - quietFor, deadline - Date.now())
559562
if (remaining <= 0) return
560-
await sleep(remaining)
563+
await interruptibleSleep(remaining, signal)
561564
}
562565
}
563566

‎packages/desktop-bridge/src/local-filesystem-tools.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,24 @@ describe('user-local grep', () => {
7878
runUserLocalFilesystemTool('call-1', 'grep', args, context(false))
7979
).resolves.not.toHaveProperty('truncated')
8080
})
81+
82+
it('treats a trailing slash on the path as the same folder', async () => {
83+
await expect(
84+
runUserLocalFilesystemTool(
85+
'call-1',
86+
'grep',
87+
{ ...args, path: 'user-local//' },
88+
context(false)
89+
)
90+
).resolves.toEqual(await runUserLocalFilesystemTool('call-1', 'grep', args, context(false)))
91+
})
92+
93+
it('trims a long run of slashes that stops short of the end in linear time', async () => {
94+
const path = `${'/'.repeat(200_000)}x`
95+
const startedAt = performance.now()
96+
await expect(
97+
runUserLocalFilesystemTool('call-1', 'grep', { ...args, path }, context(false))
98+
).rejects.toThrow()
99+
expect(performance.now() - startedAt).toBeLessThan(1000)
100+
})
81101
})

‎packages/desktop-bridge/src/local-filesystem-tools.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* local filesystem service. The chat view runs them through the preload bridge; the desktop's
44
* background executor runs them in-process. Both get the same paths and result shapes.
55
*/
6+
import { stripTrailingSlashes } from '@sim/utils/string'
67
import micromatch from 'micromatch'
78
import type {
89
LocalFilesystemData,
@@ -217,7 +218,7 @@ async function grep(
217218
args: Record<string, unknown>
218219
): Promise<Record<string, unknown>> {
219220
const pattern = requiredString(args, 'pattern')
220-
const path = requiredString(args, 'path').replace(/\/+$/, '')
221+
const path = stripTrailingSlashes(requiredString(args, 'path'))
221222
const outputMode =
222223
args.output_mode === 'files_with_matches' || args.output_mode === 'count'
223224
? args.output_mode

‎packages/sim-cli/src/config/profile.ts‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
writeFileSync,
1111
} from 'node:fs'
1212
import { dirname } from 'node:path'
13+
import { stripTrailingSlashes } from '@sim/utils/string'
1314
import { lock } from 'proper-lockfile'
1415
import { embeddedProfile } from '../embed-context'
1516
import {
@@ -583,17 +584,6 @@ export function deleteProfile(profile: string): { config: boolean; credentials:
583584
return { config, credentials }
584585
}
585586

586-
/**
587-
* Removes every trailing `/`. A backward scan rather than `/\/+$/`: that regex
588-
* restarts at each `/` in a long run that does not reach the end, so it is
589-
* quadratic in the run length.
590-
*/
591-
function stripTrailingSlashes(value: string): string {
592-
let end = value.length
593-
while (end > 0 && value.charCodeAt(end - 1) === 0x2f) end--
594-
return value.slice(0, end)
595-
}
596-
597587
/**
598588
* Validates an endpoint and strips its trailing slashes.
599589
*

‎packages/utils/src/string.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
projectEscapedMarkdownForSearch,
77
sanitizeForJsonb,
88
sanitizeValueForJsonb,
9+
stripTrailingSlashes,
910
truncateAtCodePoint,
1011
} from './string.js'
1112

@@ -85,6 +86,14 @@ describe('escapeRegExp', () => {
8586
})
8687
})
8788

89+
describe('stripTrailingSlashes', () => {
90+
it('removes only the trailing run', () => {
91+
expect(stripTrailingSlashes('/a//b///')).toBe('/a//b')
92+
expect(stripTrailingSlashes('///')).toBe('')
93+
expect(stripTrailingSlashes('a')).toBe('a')
94+
})
95+
})
96+
8897
describe('compareStrings', () => {
8998
it('sorts uppercase before lowercase, unlike localeCompare', () => {
9099
expect(compareStrings('Z', 'a')).toBe(-1)

0 commit comments

Comments
 (0)