mirror of
https://github.com/tiennm99/ccs.git
synced 2026-10-18 12:13:58 +00:00
fix(cursor): address review feedback and translator edge cases
This commit is contained in:
1 parent
d7b907ed9f
commit
27f6a675be
8 files changed
+338
-38
No files matched your search
@@ -2,7 +2,7 @@
|
||||
* Unit tests for Cursor daemon module
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import { describe, it, expect, beforeEach, afterEach, spyOn } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
@@ -273,6 +273,33 @@ describe('stopDaemon', () => {
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('refuses to stop when daemon ownership cannot be verified', async () => {
|
||||
const killSpy = spyOn(process, 'kill').mockImplementation(
|
||||
((pid: number, signal?: NodeJS.Signals | number) => {
|
||||
if (pid === process.pid && signal === 0) {
|
||||
const err = new Error('EPERM') as NodeJS.ErrnoException;
|
||||
err.code = 'EPERM';
|
||||
throw err;
|
||||
}
|
||||
|
||||
return true;
|
||||
}) as typeof process.kill
|
||||
);
|
||||
|
||||
writePidToFile(process.pid);
|
||||
|
||||
try {
|
||||
const result = await stopDaemon();
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toContain('unable to verify daemon ownership');
|
||||
expect(fs.existsSync(path.join(getTestCursorDir(), 'daemon.pid'))).toBe(true);
|
||||
} finally {
|
||||
killSpy.mockRestore();
|
||||
removePidFile();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('handleCursorCommand', () => {
|
||||
@@ -366,9 +393,9 @@ describe('renderCursorStatus', () => {
|
||||
expect(
|
||||
logs.some((line) => line.includes('Anthropic base: http://127.0.0.1:20129'))
|
||||
).toBe(true);
|
||||
expect(logs.some((line) => line.includes('Raw settings: ~/.ccs/cursor.settings.json'))).toBe(
|
||||
true
|
||||
);
|
||||
expect(
|
||||
logs.some((line) => line.includes(`Raw settings: ${getCcsDir()}/cursor.settings.json`))
|
||||
).toBe(true);
|
||||
} finally {
|
||||
console.log = originalLog;
|
||||
}
|
||||
@@ -403,6 +430,46 @@ describe('renderCursorStatus', () => {
|
||||
console.log = originalLog;
|
||||
}
|
||||
});
|
||||
|
||||
it('falls back to ~/.ccs in status output when no CCS override is active', () => {
|
||||
const originalLog = console.log;
|
||||
const originalCcsHomeValue = process.env.CCS_HOME;
|
||||
const logs: string[] = [];
|
||||
|
||||
delete process.env.CCS_HOME;
|
||||
console.log = (...args: unknown[]) => {
|
||||
logs.push(args.map((arg) => String(arg)).join(' '));
|
||||
};
|
||||
|
||||
try {
|
||||
renderCursorStatus(
|
||||
{ ...DEFAULT_CURSOR_CONFIG, enabled: true, port: 20129 },
|
||||
{
|
||||
authenticated: true,
|
||||
expired: false,
|
||||
tokenAge: 0,
|
||||
credentials: {
|
||||
accessToken: 'a'.repeat(60),
|
||||
machineId: '1234567890abcdef1234567890abcdef',
|
||||
authMethod: 'manual',
|
||||
importedAt: new Date().toISOString(),
|
||||
},
|
||||
},
|
||||
{ running: true, port: 20129, pid: 1234 }
|
||||
);
|
||||
|
||||
expect(logs.some((line) => line.includes('Raw settings: ~/.ccs/cursor.settings.json'))).toBe(
|
||||
true
|
||||
);
|
||||
} finally {
|
||||
if (originalCcsHomeValue !== undefined) {
|
||||
process.env.CCS_HOME = originalCcsHomeValue;
|
||||
} else {
|
||||
delete process.env.CCS_HOME;
|
||||
}
|
||||
console.log = originalLog;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('renderCursorHelp', () => {
|
||||
|
||||
@@ -223,7 +223,7 @@ describe('Message Translation', () => {
|
||||
expect(result.messages[0].tool_calls![0].function.name).toBe('get_weather');
|
||||
});
|
||||
|
||||
it('should accumulate tool results', () => {
|
||||
it('should flatten tool results into user tool_result blocks', () => {
|
||||
const result = buildCursorRequest(
|
||||
'gpt-4',
|
||||
{
|
||||
@@ -260,6 +260,56 @@ describe('Message Translation', () => {
|
||||
expect(result.messages[2]).toEqual({ role: 'user', content: 'What is the weather?' });
|
||||
});
|
||||
|
||||
it('should preserve consecutive tool messages as separate user turns', () => {
|
||||
const result = buildCursorRequest(
|
||||
'gpt-4',
|
||||
{
|
||||
messages: [
|
||||
{
|
||||
role: 'assistant',
|
||||
content: '',
|
||||
tool_calls: [
|
||||
{
|
||||
id: 'call_one',
|
||||
type: 'function',
|
||||
function: { name: 'search_docs', arguments: '{"q":"cursor"}' },
|
||||
},
|
||||
{
|
||||
id: 'call_two',
|
||||
type: 'function',
|
||||
function: { name: 'read_file', arguments: '{"path":"README.md"}' },
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
role: 'tool',
|
||||
content: 'first result',
|
||||
tool_call_id: 'call_one',
|
||||
},
|
||||
{
|
||||
role: 'tool',
|
||||
content: 'second result',
|
||||
tool_call_id: 'call_two',
|
||||
},
|
||||
{ role: 'user', content: 'Summarize both results.' },
|
||||
],
|
||||
},
|
||||
false,
|
||||
{}
|
||||
);
|
||||
|
||||
expect(result.messages).toHaveLength(4);
|
||||
expect(result.messages[1]).toEqual({
|
||||
role: 'user',
|
||||
content: expect.stringContaining('<tool_call_id>call_one</tool_call_id>'),
|
||||
});
|
||||
expect(result.messages[2]).toEqual({
|
||||
role: 'user',
|
||||
content: expect.stringContaining('<tool_call_id>call_two</tool_call_id>'),
|
||||
});
|
||||
expect(result.messages[3]).toEqual({ role: 'user', content: 'Summarize both results.' });
|
||||
});
|
||||
|
||||
it('should recover tool names for tool results without a name field', () => {
|
||||
const result = buildCursorRequest(
|
||||
'gpt-4',
|
||||
@@ -332,6 +382,45 @@ describe('Message Translation', () => {
|
||||
expect(result.messages[1].content).toContain('{"answer":"Cursor integration ready"}');
|
||||
});
|
||||
|
||||
it('should preserve line breaks for multipart tool_result text content', () => {
|
||||
const result = buildCursorRequest(
|
||||
'gpt-4',
|
||||
{
|
||||
messages: [
|
||||
{
|
||||
role: 'assistant',
|
||||
content: '',
|
||||
tool_calls: [
|
||||
{
|
||||
id: 'call_lines',
|
||||
type: 'function',
|
||||
function: { name: 'read_file', arguments: '{"path":"notes.txt"}' },
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
role: 'user',
|
||||
content: [
|
||||
{
|
||||
type: 'tool_result',
|
||||
tool_use_id: 'call_lines',
|
||||
content: [
|
||||
{ type: 'text', text: 'first line' },
|
||||
{ type: 'text', text: 'second line' },
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
false,
|
||||
{}
|
||||
);
|
||||
|
||||
expect(result.messages).toHaveLength(2);
|
||||
expect(result.messages[1].content).toContain('<result>first line\nsecond line</result>');
|
||||
});
|
||||
|
||||
it('should convert assistant tool_use content blocks into tool_calls', () => {
|
||||
const result = buildCursorRequest(
|
||||
'gpt-4',
|
||||
@@ -393,6 +482,38 @@ describe('Message Translation', () => {
|
||||
expect(result.messages[0].tool_calls![0].id).toBe('toolu_cursor_fallback_0_0');
|
||||
});
|
||||
|
||||
it('should normalize invalid assistant tool call ids before emitting tool results', () => {
|
||||
const result = buildCursorRequest(
|
||||
'gpt-4',
|
||||
{
|
||||
messages: [
|
||||
{
|
||||
role: 'assistant',
|
||||
content: '',
|
||||
tool_calls: [
|
||||
{
|
||||
id: 'call_bad\nline two',
|
||||
type: 'function',
|
||||
function: { name: 'search_docs', arguments: '{"q":"cursor"}' },
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
role: 'tool',
|
||||
content: 'done',
|
||||
tool_call_id: 'call_bad\nline two',
|
||||
},
|
||||
],
|
||||
},
|
||||
false,
|
||||
{}
|
||||
);
|
||||
|
||||
expect(result.messages).toHaveLength(2);
|
||||
expect(result.messages[0].tool_calls![0].id).toBe('call_bad');
|
||||
expect(result.messages[1].content).toContain('<tool_call_id>call_bad</tool_call_id>');
|
||||
});
|
||||
|
||||
it('should dedupe tool calls when assistant messages contain both tool_calls and tool_use blocks', () => {
|
||||
const result = buildCursorRequest(
|
||||
'gpt-4',
|
||||
@@ -463,6 +584,10 @@ describe('Message Translation', () => {
|
||||
expect(result.messages).toHaveLength(2);
|
||||
expect(result.messages[1].content).toContain('[truncated ');
|
||||
expect(result.messages[1].content.length).toBeLessThan(12_250);
|
||||
|
||||
const resultMatch = result.messages[1].content.match(/<result>([\s\S]*)<\/result>/);
|
||||
expect(resultMatch).not.toBeNull();
|
||||
expect(resultMatch?.[1].length).toBeLessThanOrEqual(12_000);
|
||||
});
|
||||
|
||||
it('should mark unserializable structured tool results explicitly', () => {
|
||||
@@ -504,6 +629,47 @@ describe('Message Translation', () => {
|
||||
expect(result.messages[1].content).toContain('[unserializable content]');
|
||||
});
|
||||
|
||||
it('should reject tool_result blocks without a valid tool_use_id', () => {
|
||||
expect(() =>
|
||||
buildCursorRequest(
|
||||
'gpt-4',
|
||||
{
|
||||
messages: [
|
||||
{
|
||||
role: 'user',
|
||||
content: [
|
||||
{
|
||||
type: 'tool_result',
|
||||
content: 'done',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
false,
|
||||
{}
|
||||
)
|
||||
).toThrow('messages[0].content[0] must include a valid tool result id');
|
||||
});
|
||||
|
||||
it('should reject tool role messages without a valid tool_call_id', () => {
|
||||
expect(() =>
|
||||
buildCursorRequest(
|
||||
'gpt-4',
|
||||
{
|
||||
messages: [
|
||||
{
|
||||
role: 'tool',
|
||||
content: 'done',
|
||||
},
|
||||
],
|
||||
},
|
||||
false,
|
||||
{}
|
||||
)
|
||||
).toThrow('messages[0].tool_call_id must include a valid tool result id');
|
||||
});
|
||||
|
||||
it('should handle array content format', () => {
|
||||
const result = buildCursorRequest(
|
||||
'gpt-4',
|
||||
|
||||
Reference in new issue
Block a user