From cf5df0630a2360c0c145b926dc94e1775ed8d7b0 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sun, 12 Apr 2026 02:14:26 -0400 Subject: [PATCH] fix(cursor): resolve PR review findings --- src/cursor/constants.ts | 1 + src/cursor/cursor-executor.ts | 33 ++++++-- src/cursor/cursor-runtime-probe.ts | 8 +- src/cursor/cursor-stream-parser.ts | 53 ++++++++++-- src/web-server/routes/cursor-routes.ts | 15 +++- tests/npm/cli.test.js | 14 +++ tests/unit/cursor/cursor-protobuf.test.ts | 94 +++++++++++++++++++-- tests/unit/web-server/cursor-routes.test.ts | 1 + ui/src/hooks/use-cursor.ts | 2 + ui/src/lib/i18n.ts | 8 ++ ui/src/pages/cursor.tsx | 29 +++++-- ui/tests/unit/ui/pages/cursor-page.test.tsx | 27 ++++++ 12 files changed, 256 insertions(+), 29 deletions(-) diff --git a/src/cursor/constants.ts b/src/cursor/constants.ts index 8a971469..b9c0d18d 100644 --- a/src/cursor/constants.ts +++ b/src/cursor/constants.ts @@ -1,6 +1,7 @@ export const CURSOR_SUBCOMMANDS = [ 'auth', 'status', + 'probe', 'models', 'start', 'stop', diff --git a/src/cursor/cursor-executor.ts b/src/cursor/cursor-executor.ts index a8ae63de..d7344be0 100644 --- a/src/cursor/cursor-executor.ts +++ b/src/cursor/cursor-executor.ts @@ -18,6 +18,7 @@ import { type FrameResult, StreamingFrameParser, decompressPayload, + mapCursorConnectError, } from './cursor-stream-parser.js'; /** Executor parameters */ @@ -88,12 +89,12 @@ function toCursorErrorPayloadFromJson(jsonError: { jsonError?.error?.message || 'API Error'; - const isRateLimit = jsonError?.error?.code === 'resource_exhausted'; + const mappedError = mapCursorConnectError(jsonError?.error?.code); return { message: errorMsg, - status: isRateLimit ? 429 : 400, - errorType: isRateLimit ? 'rate_limit_error' : 'api_error', + status: mappedError.status, + errorType: mappedError.errorType, code: jsonError?.error?.details?.[0]?.debug?.error || 'unknown', }; } @@ -650,6 +651,12 @@ export class CursorExecutor { req.on('end', () => { if (streamClosed) return; + for (const frame of parser.finish()) { + if (frame.type === 'error') { + handleFrameError(frame); + return; + } + } resolveStreamingResponse(); if (chunkCount === 0 && toolCallCount === 0) { emitSSE(buildChunk({ role: 'assistant', content: '' }, null)); @@ -757,14 +764,14 @@ export class CursorExecutor { json?.error?.message; if (msg) { - const isRateLimit = json?.error?.code === 'resource_exhausted'; + const mappedError = mapCursorConnectError(json?.error?.code); yield { type: 'error', error: { message: msg, - status: isRateLimit ? 429 : 400, - errorType: isRateLimit ? 'rate_limit_error' : 'api_error', - code: isRateLimit ? 'rate_limited' : 'cursor_error', + status: mappedError.status, + errorType: mappedError.errorType, + code: json?.error?.code || 'cursor_error', }, }; return; @@ -821,6 +828,18 @@ export class CursorExecutor { yield { type: 'thinking', text: result.thinking }; } } + + if (offset !== buffer.length) { + yield { + type: 'error', + error: { + message: 'Truncated Cursor ConnectRPC frame.', + status: 502, + errorType: 'server_error', + code: 'cursor_protocol_error', + }, + }; + } } transformProtobufToJSON(buffer: Buffer, model: string, _body: ExecutorParams['body']): Response { diff --git a/src/cursor/cursor-runtime-probe.ts b/src/cursor/cursor-runtime-probe.ts index 3ac4ca3c..3dadfa36 100644 --- a/src/cursor/cursor-runtime-probe.ts +++ b/src/cursor/cursor-runtime-probe.ts @@ -144,6 +144,12 @@ export async function probeCursorRuntime(config: CursorConfig): Promise 0; } + + finish(): FrameResult[] { + if (this.buffer.length === 0) { + return []; + } + + this.buffer = Buffer.alloc(0); + return [toFrameErrorResult(createTruncatedFrameError())]; + } } diff --git a/src/web-server/routes/cursor-routes.ts b/src/web-server/routes/cursor-routes.ts index 73575352..5309c581 100644 --- a/src/web-server/routes/cursor-routes.ts +++ b/src/web-server/routes/cursor-routes.ts @@ -31,6 +31,19 @@ interface DaemonStartPreconditionError { error: string; } +function getPublicAutoDetectError(result: ReturnType): string { + switch (result.reason) { + case 'db_not_found': + return 'Cursor state database not found on this host.'; + case 'sqlite_unavailable': + return 'Cursor state database was found, but sqlite3 is not available in PATH.'; + case 'db_query_failed': + return 'Cursor state database was found, but CCS could not query it.'; + default: + return result.error ?? 'Token not found'; + } +} + export function getDaemonStartPreconditionError( input: DaemonStartPreconditionInput ): DaemonStartPreconditionError | null { @@ -132,7 +145,7 @@ router.post('/auth/auto-detect', async (_req: Request, res: Response): Promise { assert(!output.includes("Profile '--verbose' not found"), 'Should not treat flags as profiles'); } }); + + it('routes cursor probe through the cursor command handler', function() { + try { + execSync(`bun "${srcCcsPath}" cursor probe`, { + stdio: 'pipe', + timeout: 3000, + env: { ...process.env, CCS_HOME: testCcsHome } + }); + } catch (e) { + const output = e.stderr?.toString() || e.stdout?.toString() || ''; + assert(!output.includes("Profile 'cursor' not found"), 'Should not fall through to profile lookup'); + assert(output.includes('Cursor Live Probe'), 'Should render the cursor probe command output'); + } + }); }); describe('Profile handling', () => { diff --git a/tests/unit/cursor/cursor-protobuf.test.ts b/tests/unit/cursor/cursor-protobuf.test.ts index 337ae7ee..8b3df7b3 100644 --- a/tests/unit/cursor/cursor-protobuf.test.ts +++ b/tests/unit/cursor/cursor-protobuf.test.ts @@ -846,7 +846,7 @@ describe('Request Encoding', () => { }); describe('Edge cases', () => { - it('should handle malformed frame gracefully', () => { + it('should reject malformed frame headers', async () => { const executor = new CursorExecutor(); // Incomplete frame header (only 3 bytes instead of 5) @@ -856,11 +856,13 @@ describe('Request Encoding', () => { messages: [], }); - // Should return valid response even with malformed input - expect(result.status).toBe(200); + expect(result.status).toBe(502); + const body = JSON.parse(await result.text()); + expect(body.error.type).toBe('server_error'); + expect(body.error.message).toContain('Truncated Cursor ConnectRPC frame'); }); - it('should handle truncated payload', () => { + it('should reject truncated payloads', async () => { const executor = new CursorExecutor(); // Frame header says payload is 100 bytes but only 5 bytes follow @@ -872,8 +874,10 @@ describe('Request Encoding', () => { messages: [], }); - // Should handle gracefully - expect(result.status).toBe(200); + expect(result.status).toBe(502); + const body = JSON.parse(await result.text()); + expect(body.error.type).toBe('server_error'); + expect(body.error.message).toContain('Truncated Cursor ConnectRPC frame'); }); it('should handle multi-frame buffer', () => { @@ -1057,6 +1061,29 @@ describe('CursorExecutor', () => { expect(body.error.type).toBe('rate_limit_error'); }); + it('should map unavailable end-stream errors to 503', async () => { + const unavailableFrame = buildFrame( + new TextEncoder().encode( + JSON.stringify({ + error: { + code: 'unavailable', + message: 'upstream down', + }, + }) + ), + 0x02 + ); + + const result = executor.transformProtobufToJSON(unavailableFrame, 'gpt-4', { + messages: [], + }); + + expect(result.status).toBe(503); + const body = JSON.parse(await result.text()); + expect(body.error.type).toBe('api_error'); + expect(body.error.message).toContain('upstream down'); + }); + it('should surface reasoning_content when thinking payload is present', async () => { const textContent = 'Final answer'; const thinkingContent = 'Internal reasoning trail'; @@ -1281,6 +1308,23 @@ describe('CursorExecutor', () => { expect(body).not.toContain('data: [DONE]'); }); + it('emits an SSE error event when trailing bytes leave a truncated frame', async () => { + const executor = new CursorExecutor(); + const combined = Buffer.concat([buildTextFrame('Partial success'), Buffer.from([0x00, 0x00, 0x00])]); + + const result = executor.transformProtobufToSSE(combined, 'test-model', { + messages: [], + stream: true, + }); + + expect(result.status).toBe(200); + const body = await result.text(); + expect(body).toContain('Partial success'); + expect(body).toContain('event: error'); + expect(body).toContain('Truncated Cursor ConnectRPC frame'); + expect(body).not.toContain('data: [DONE]'); + }); + it('returns an explicit error for unknown ConnectRPC frame flags', async () => { const executor = new CursorExecutor(); const result = executor.transformProtobufToJSON(buildFrame(new Uint8Array([0x01]), 0x04), 'gpt-4', { @@ -1510,6 +1554,30 @@ describe('StreamingFrameParser', () => { } }); + it('should map unavailable end-stream errors to 503 in the parser', () => { + const parser = new StreamingFrameParser(); + const endStreamError = buildFrame( + new TextEncoder().encode( + JSON.stringify({ + error: { + code: 'unavailable', + message: 'upstream down', + }, + }) + ), + 0x02 + ); + + const results = parser.push(endStreamError); + + expect(results.length).toBe(1); + expect(results[0].type).toBe('error'); + if (results[0].type === 'error') { + expect(results[0].status).toBe(503); + expect(results[0].errorType).toBe('api_error'); + } + }); + it('should parse thinking frames', () => { const parser = new StreamingFrameParser(); const frame = buildThinkingFrame('Think step by step'); @@ -1601,6 +1669,20 @@ describe('StreamingFrameParser', () => { parser2.push(emptyFrame); expect(parser2.hasPartial()).toBe(false); }); + + it('should surface truncated trailing bytes when the stream finishes', () => { + const parser = new StreamingFrameParser(); + parser.push(Buffer.from([0x00, 0x00, 0x00])); + + const results = parser.finish(); + + expect(results.length).toBe(1); + expect(results[0].type).toBe('error'); + if (results[0].type === 'error') { + expect(results[0].status).toBe(502); + expect(results[0].message).toContain('Truncated Cursor ConnectRPC frame'); + } + }); }); describe('decompressPayload', () => { diff --git a/tests/unit/web-server/cursor-routes.test.ts b/tests/unit/web-server/cursor-routes.test.ts index 65945b83..4334d750 100644 --- a/tests/unit/web-server/cursor-routes.test.ts +++ b/tests/unit/web-server/cursor-routes.test.ts @@ -284,6 +284,7 @@ describe('Cursor Routes Logic', () => { expect(typeof json.error).toBe('string'); expect(json.error?.length).toBeGreaterThan(0); expect(json.reason).toBe('db_not_found'); + expect(json.error).not.toContain(isolatedHome); } finally { if (originalHome !== undefined) { process.env.HOME = originalHome; diff --git a/ui/src/hooks/use-cursor.ts b/ui/src/hooks/use-cursor.ts index 747db82e..0b67bf3c 100644 --- a/ui/src/hooks/use-cursor.ts +++ b/ui/src/hooks/use-cursor.ts @@ -269,6 +269,7 @@ export function useCursor() { config: configQuery.data, configLoading: configQuery.isLoading, + refetchConfig: configQuery.refetch, models: modelsQuery.data?.models ?? [], currentModel: modelsQuery.data?.current ?? null, @@ -317,6 +318,7 @@ export function useCursor() { statusQuery.refetch, configQuery.data, configQuery.isLoading, + configQuery.refetch, modelsQuery.data, modelsQuery.isLoading, rawSettingsQuery.data, diff --git a/ui/src/lib/i18n.ts b/ui/src/lib/i18n.ts index faef6c61..67e00c03 100644 --- a/ui/src/lib/i18n.ts +++ b/ui/src/lib/i18n.ts @@ -1340,6 +1340,8 @@ const resources = { probeSucceeded: 'Probe succeeded', probeFailed: 'Probe failed', probeSaveFirst: 'Save or discard Cursor changes before running the live probe', + refreshStatus: 'Refresh Cursor status', + refreshConfiguration: 'Refresh Cursor configuration', }, droidPage: { loadingDiagnostics: 'Loading Droid diagnostics...', @@ -2688,6 +2690,8 @@ const resources = { probeSucceeded: '探测成功', probeFailed: '探测失败', probeSaveFirst: '请先保存或放弃 Cursor 更改，再运行实时探测', + refreshStatus: '刷新 Cursor 状态', + refreshConfiguration: '刷新 Cursor 配置', }, droidPage: { loadingDiagnostics: '加载 Droid 诊断中...', @@ -4130,6 +4134,8 @@ const resources = { probeSucceeded: 'Kiểm tra thành công', probeFailed: 'Kiểm tra thất bại', probeSaveFirst: 'Hãy lưu hoặc bỏ các thay đổi Cursor trước khi chạy kiểm tra trực tiếp', + refreshStatus: 'Làm mới trạng thái Cursor', + refreshConfiguration: 'Làm mới cấu hình Cursor', }, droidPage: { loadingDiagnostics: 'Đang tải chẩn đoán Droid...', @@ -5580,6 +5586,8 @@ const resources = { probeSucceeded: 'プローブ成功', probeFailed: 'プローブ失敗', probeSaveFirst: 'ライブプローブを実行する前に Cursor の変更を保存するか破棄してください', + refreshStatus: 'Cursor の状態を更新', + refreshConfiguration: 'Cursor の設定を更新', }, droidPage: { loadingDiagnostics: 'Droid診断を読み込み中...', diff --git a/ui/src/pages/cursor.tsx b/ui/src/pages/cursor.tsx index d5bbec3f..54d9ae86 100644 --- a/ui/src/pages/cursor.tsx +++ b/ui/src/pages/cursor.tsx @@ -292,6 +292,7 @@ export function CursorPage() { statusLoading, refetchStatus, config, + refetchConfig, updateConfigAsync, isUpdatingConfig, models, @@ -395,6 +396,11 @@ export function CursorPage() { setProbeSnapshotKey(null); }; + const resetConfigDraft = (nextConfig = config) => { + setConfigDraft(buildConfigDraft(nextConfig)); + setConfigDirty(false); + }; + const canStart = Boolean(status?.enabled && status?.authenticated && !status?.token_expired); const integrationBadge = useMemo( () => @@ -699,11 +705,15 @@ export function CursorPage() { } }; - const handleHeaderRefresh = () => { + const handleHeaderRefresh = async () => { setRawConfigDirty(false); clearProbeState(); - refetchStatus(); - refetchRawSettings(); + const [, refreshedConfig] = await Promise.all([ + refetchStatus(), + refetchConfig(), + refetchRawSettings(), + ]); + resetConfigDraft(refreshedConfig.data ?? config); }; return ( @@ -733,6 +743,8 @@ export function CursorPage() { className="h-8 w-8" onClick={() => refetchStatus()} disabled={statusLoading} + aria-label={t('cursorPage.refreshStatus')} + title={t('cursorPage.refreshStatus')} > @@ -794,11 +806,12 @@ export function CursorPage() { @@ -993,6 +1006,8 @@ export function CursorPage() { size="sm" onClick={handleHeaderRefresh} disabled={statusLoading || rawSettingsLoading} + aria-label={t('cursorPage.refreshConfiguration')} + title={t('cursorPage.refreshConfiguration')} > ({ runProbeAsync: vi.fn(), resetProbe: vi.fn(), refetchStatus: vi.fn(), + refetchConfig: vi.fn(), refetchRawSettings: vi.fn(), updateConfigAsync: vi.fn(), saveRawSettingsAsync: vi.fn(), @@ -75,6 +76,7 @@ function buildUseCursorResult(overrides: Record = {}) { sonnet_model: 'gpt-5.3-codex', haiku_model: 'gpt-5-mini', }, + refetchConfig: mocks.refetchConfig, updateConfigAsync: mocks.updateConfigAsync, isUpdatingConfig: false, models: [{ id: 'gpt-5.3-codex', name: 'GPT-5.3 Codex', provider: 'openai' }], @@ -110,6 +112,13 @@ describe('CursorPage', () => { beforeEach(() => { mocks.runProbeAsync.mockReset(); mocks.resetProbe.mockReset(); + mocks.refetchConfig.mockReset(); + mocks.refetchConfig.mockResolvedValue({ + status: 'success', + isError: false, + error: null, + data: buildUseCursorResult().config, + }); mocks.runProbeAsync.mockResolvedValue(probeFailure); mocks.useCursor.mockReturnValue(buildUseCursorResult()); toastMocks.error.mockReset(); @@ -154,4 +163,22 @@ describe('CursorPage', () => { 'Save or discard Cursor changes before running the live probe' ); }); + + it('refreshes config state with accessible refresh controls', async () => { + render(); + + expect(screen.getByRole('button', { name: 'Refresh Cursor status' })).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Refresh Cursor configuration' }) + ).toBeInTheDocument(); + + await userEvent.click(screen.getByRole('tab', { name: 'Settings' })); + await userEvent.clear(screen.getByLabelText('Port')); + await userEvent.type(screen.getByLabelText('Port'), '20130'); + + await userEvent.click(screen.getByRole('button', { name: 'Refresh Cursor configuration' })); + + await waitFor(() => expect(mocks.refetchConfig).toHaveBeenCalledTimes(1)); + expect(screen.getByLabelText('Port')).toHaveValue(20129); + }); });