diff --git a/cli/src/cursor/utils/cursorEventConverter.test.ts b/cli/src/cursor/utils/cursorEventConverter.test.ts index a9f6438a..595f7f09 100644 --- a/cli/src/cursor/utils/cursorEventConverter.test.ts +++ b/cli/src/cursor/utils/cursorEventConverter.test.ts @@ -1,11 +1,16 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, beforeEach } from 'vitest'; import { parseCursorEvent, convertCursorEventToAgentMessage, + __resetCursorEventConverterStateForTests, type CursorStreamEvent } from './cursorEventConverter'; describe('cursorEventConverter', () => { + beforeEach(() => { + __resetCursorEventConverterStateForTests(); + }); + describe('parseCursorEvent', () => { it('parses system init event', () => { const line = @@ -58,5 +63,324 @@ describe('cursorEventConverter', () => { const msg = convertCursorEventToAgentMessage(event); expect(msg).toEqual({ type: 'turn_complete', stopReason: 'success' }); }); + + it('passes through a normal tool result unchanged (read_file)', () => { + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'r1', + session_id: 's1', + tool_call: { + readToolCall: { + args: { path: '/tmp/x' }, + result: { content: 'hello' } + } + } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).toEqual({ + type: 'tool_result', + id: 'r1', + output: { content: 'hello' }, + status: 'completed' + }); + }); + }); + + describe('#784 transitional safety: AskQuestion synthetic-skip intercept', () => { + it('rewrites a tool_call result containing the synthetic skip string to a no_input_surface failure', () => { + const startedEvent = { + type: 'tool_call', + subtype: 'started', + call_id: 'q1', + session_id: 's1', + tool_call: { function: { name: 'AskQuestion', arguments: '{"q":"..."}' } } + } as CursorStreamEvent; + convertCursorEventToAgentMessage(startedEvent); + + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'q1', + session_id: 's1', + tool_call: { + function: { + name: 'AskQuestion', + arguments: '{"q":"..."}', + result: 'Questions skipped by the user, continue with the information you already have' + } + } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).not.toBeNull(); + expect(msg).toMatchObject({ + type: 'tool_result', + id: 'q1', + status: 'failed' + }); + const output = (msg as { output: { kind: string; message: string } }).output; + expect(output.kind).toBe('no_input_surface'); + expect(output.message).toMatch(/cursor-agent fabricated a skip response/); + expect(output.message).toMatch(/Re-prompt in plain text/); + }); + + it('matches the synthetic-skip string even when nested deep inside the tool_call payload', () => { + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'q2', + session_id: 's1', + tool_call: { + function: { + name: 'AskQuestion', + outcome: { + response: { + text: 'Questions skipped by the user, continue with the information you already have' + } + } + } + } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).toMatchObject({ type: 'tool_result', id: 'q2', status: 'failed' }); + }); + + it('rewrites a sub-500ms AskQuestion completion with a trivial result even without the synthetic string', () => { + const startedEvent = { + type: 'tool_call', + subtype: 'started', + call_id: 'q3', + session_id: 's1', + tool_call: { function: { name: 'AskQuestion', arguments: '{}' } } + } as CursorStreamEvent; + convertCursorEventToAgentMessage(startedEvent); + + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'q3', + session_id: 's1', + tool_call: { function: { name: 'AskQuestion', arguments: '{}' } } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).toMatchObject({ type: 'tool_result', id: 'q3', status: 'failed' }); + expect((msg as { output: { kind: string } }).output.kind).toBe('no_input_surface'); + }); + + it('rewrites a sub-500ms completion when the converter falls back to name=unknown', () => { + const startedEvent = { + type: 'tool_call', + subtype: 'started', + call_id: 'q4', + session_id: 's1', + tool_call: { function: {} } + } as CursorStreamEvent; + convertCursorEventToAgentMessage(startedEvent); + + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'q4', + session_id: 's1', + tool_call: { function: {} } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).toMatchObject({ type: 'tool_result', id: 'q4', status: 'failed' }); + }); + + it('does NOT rewrite a normal function-tool completion with a real result', () => { + const startedEvent = { + type: 'tool_call', + subtype: 'started', + call_id: 'r2', + session_id: 's1', + tool_call: { function: { name: 'MyCustomTool', arguments: '{}' } } + } as CursorStreamEvent; + convertCursorEventToAgentMessage(startedEvent); + + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'r2', + session_id: 's1', + tool_call: { + function: { name: 'MyCustomTool', arguments: '{}', result: { ok: true } } + } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).toMatchObject({ type: 'tool_result', id: 'r2', status: 'completed' }); + expect((msg as { output: unknown }).output).not.toMatchObject({ kind: 'no_input_surface' }); + }); + + it('does NOT rewrite read_file/write_file results even with empty payloads', () => { + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'rw1', + session_id: 's1', + tool_call: { readToolCall: { args: { path: '/tmp/x' }, result: {} } } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).toMatchObject({ type: 'tool_result', id: 'rw1', status: 'completed' }); + }); + + // Regression test for the false positive flagged on PR #801 by the + // HAPI auto-review bot: this PR adds the literal synthetic-skip + // marker to docs/guide/cursor.md, so a Cursor read_file of that + // file would surface the marker inside readToolCall.result.content. + // The intercept must NOT rewrite that as a no_input_surface failure. + it('does NOT rewrite a read_file result whose content contains the synthetic marker', () => { + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'doc1', + session_id: 's1', + tool_call: { + readToolCall: { + args: { path: 'docs/guide/cursor.md' }, + result: { + content: + 'Lorem ipsum ... Questions skipped by the user, continue with the information you already have ... etc.' + } + } + } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).toMatchObject({ + type: 'tool_result', + id: 'doc1', + status: 'completed' + }); + expect((msg as { output: unknown }).output).not.toMatchObject({ + kind: 'no_input_surface' + }); + }); + + it('does NOT rewrite a write_file result whose payload contains the synthetic marker', () => { + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'doc2', + session_id: 's1', + tool_call: { + writeToolCall: { + args: { + path: 'docs/guide/cursor.md', + content: + 'Documenting: "Questions skipped by the user, continue with the information you already have"' + }, + result: { bytesWritten: 256 } + } + } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).toMatchObject({ + type: 'tool_result', + id: 'doc2', + status: 'completed' + }); + expect((msg as { output: unknown }).output).not.toMatchObject({ + kind: 'no_input_surface' + }); + }); + + // Regression test for the second Major finding from the HAPI bot on + // PR #801: a legitimate AskQuestion whose prompt text (carried in + // `function.arguments`) quotes the synthetic-skip marker - e.g. an + // agent debugging this exact bug - must NOT be rewritten when the + // user has actually answered. The marker check must look only at the + // extracted result, never the agent's input arguments. + it('does NOT rewrite an AskQuestion whose arguments quote the marker but whose result is a real answer', async () => { + const startedEvent = { + type: 'tool_call', + subtype: 'started', + call_id: 'meta1', + session_id: 's1', + tool_call: { + function: { + name: 'AskQuestion', + arguments: + '{"prompt":"Do you want to handle the case where cursor-agent returns: Questions skipped by the user, continue with the information you already have"}' + } + } + } as CursorStreamEvent; + convertCursorEventToAgentMessage(startedEvent); + + // Wait past the synthetic latency threshold so the timing + // heuristic does not apply - this is a real user answer, not a + // zero-latency fabrication. + await new Promise((resolve) => setTimeout(resolve, 550)); + + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'meta1', + session_id: 's1', + tool_call: { + function: { + name: 'AskQuestion', + arguments: + '{"prompt":"Do you want to handle the case where cursor-agent returns: Questions skipped by the user, continue with the information you already have"}', + result: 'yes, please add the intercept' + } + } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).toMatchObject({ + type: 'tool_result', + id: 'meta1', + status: 'completed' + }); + expect((msg as { output: unknown }).output).toBe('yes, please add the intercept'); + expect((msg as { output: unknown }).output).not.toMatchObject({ + kind: 'no_input_surface' + }); + }); + + it('does NOT rewrite a non-AskQuestion function tool whose result happens to contain the marker text', () => { + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'fn1', + session_id: 's1', + tool_call: { + function: { + name: 'MyCustomTool', + arguments: '{}', + result: { + note: 'Quoting: Questions skipped by the user, continue with the information you already have - end quote.' + } + } + } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).toMatchObject({ type: 'tool_result', id: 'fn1', status: 'completed' }); + expect((msg as { output: unknown }).output).not.toMatchObject({ + kind: 'no_input_surface' + }); + }); + + it('does NOT rewrite an AskQuestion completion that took longer than the synthetic threshold', async () => { + const startedEvent = { + type: 'tool_call', + subtype: 'started', + call_id: 'q5', + session_id: 's1', + tool_call: { function: { name: 'AskQuestion', arguments: '{}' } } + } as CursorStreamEvent; + convertCursorEventToAgentMessage(startedEvent); + + await new Promise((resolve) => setTimeout(resolve, 550)); + + const completedEvent = { + type: 'tool_call', + subtype: 'completed', + call_id: 'q5', + session_id: 's1', + tool_call: { function: { name: 'AskQuestion', arguments: '{}' } } + } as CursorStreamEvent; + const msg = convertCursorEventToAgentMessage(completedEvent); + expect(msg).toMatchObject({ type: 'tool_result', id: 'q5', status: 'completed' }); + }); }); }); diff --git a/cli/src/cursor/utils/cursorEventConverter.ts b/cli/src/cursor/utils/cursorEventConverter.ts index 45fc752b..3876ebeb 100644 --- a/cli/src/cursor/utils/cursorEventConverter.ts +++ b/cli/src/cursor/utils/cursorEventConverter.ts @@ -84,9 +84,168 @@ function extractToolResult(toolCall: Record): unknown { const w = toolCall.writeToolCall as Record; return w.result ?? w; } + if (toolCall.function && typeof toolCall.function === 'object') { + const fn = toolCall.function as Record; + // Cursor's stream-json function-shaped tool calls put the agent's + // input in `arguments` and the cursor-side response in other fields. + // Surface the response (preferring `result` when present, otherwise + // everything except `name` / `arguments`) so downstream callers don't + // lose the cursor-side payload to a `{}` fallback. Excluding + // `arguments` matters for the #784 intercept: the agent's own prompt + // text must not be searched for the synthetic-skip marker. + if (fn.result !== undefined) return fn.result; + const rest: Record = {}; + for (const [k, v] of Object.entries(fn)) { + if (k === 'name' || k === 'arguments') continue; + rest[k] = v; + } + return Object.keys(rest).length > 0 ? rest : {}; + } return {}; } +/** + * Transitional safety patch for tiann/hapi#784. + * + * cursor-agent in headless `--print --output-format stream-json` mode fabricates + * the following literal string as the AskQuestion tool result, with no error + * flag and in ~zero seconds, because there is no IDE surface to render the + * question. The agent's model then treats this as legitimate user consent. + * + * HAPI intercepts the synthetic string at the converter layer and rewrites the + * tool result to a structured `no_input_surface` failure, so downstream agents + * see an explicit error instead of fabricated consent. + * + * This patch is intentionally scoped to the stream-json launcher and + * auto-deletes when tiann/hapi#781 (ACP migration) replaces stream-json with + * the proper bidirectional `cursor/ask_question` ACP method. + */ +const SYNTHETIC_SKIP_MARKER = + 'Questions skipped by the user, continue with the information you already have'; + +const NO_INPUT_SURFACE_OUTPUT = { + kind: 'no_input_surface', + message: + 'cursor-agent fabricated a skip response in headless mode. The operator did not respond. Re-prompt in plain text and wait for a real user message before proceeding.' +} as const; + +/** + * Names cursor-agent has been observed to use (or fall back to) for the + * AskQuestion tool when its result reaches the stream-json converter. + */ +const ASK_QUESTION_TOOL_NAMES = new Set(['AskQuestion', 'askQuestion', 'ask_question', 'unknown']); + +/** + * Defense-in-depth latency threshold. A real interactive answer cannot arrive + * faster than this; cursor-agent's fabricated skip arrives in ~0 ms. + */ +const SYNTHETIC_LATENCY_THRESHOLD_MS = 500; + +/** + * Tracks when each tool_call 'started' event arrived, keyed by call_id. + * Used to detect zero-latency fabricated AskQuestion completions even if the + * synthetic-string text changes in a future cursor-agent release. + * + * Bounded to prevent unbounded growth if 'completed' events are ever missed. + */ +const TOOL_CALL_STARTED_MAX = 1024; +const toolCallStartedAt = new Map(); + +function rememberToolCallStart(callId: string, now: number = Date.now()): void { + if (toolCallStartedAt.size >= TOOL_CALL_STARTED_MAX) { + const oldest = toolCallStartedAt.keys().next().value; + if (typeof oldest === 'string') { + toolCallStartedAt.delete(oldest); + } + } + toolCallStartedAt.set(callId, now); +} + +function takeToolCallElapsedMs(callId: string, now: number = Date.now()): number | null { + const started = toolCallStartedAt.get(callId); + if (started === undefined) return null; + toolCallStartedAt.delete(callId); + return now - started; +} + +/** + * Recursively checks every string-typed value reachable from `value` for the + * synthetic-skip marker. Used instead of `JSON.stringify(...).includes(...)` so + * the intercept does not false-positive on legitimate tool results that happen + * to quote the marker text (notably a `read_file` of `docs/guide/cursor.md`, + * which documents this exact intercept). + * + * Guards against cycles via a visited-set; the cycle case in practice is + * vanishingly rare on stream-json payloads (parsed from JSON.parse) but the + * guard is cheap and removes any tail risk if a future caller hands us a + * non-tree object graph. + */ +function containsSyntheticSkipMarker(value: unknown, seen: WeakSet = new WeakSet()): boolean { + if (typeof value === 'string') { + return value.includes(SYNTHETIC_SKIP_MARKER); + } + if (value && typeof value === 'object') { + if (seen.has(value as object)) return false; + seen.add(value as object); + if (Array.isArray(value)) { + return value.some((entry) => containsSyntheticSkipMarker(entry, seen)); + } + return Object.values(value as Record).some((entry) => + containsSyntheticSkipMarker(entry, seen) + ); + } + return false; +} + +function isTrivialResult(result: unknown): boolean { + if (result === null || result === undefined) return true; + if (typeof result === 'string') return result.trim().length === 0; + if (typeof result === 'object') { + return Object.keys(result as Record).length === 0; + } + return false; +} + +function shouldRewriteAsNoInputSurface(opts: { + name: string; + result: unknown; + elapsedMs: number | null; +}): boolean { + // Gate on the tool name resolving to an AskQuestion-shaped call (or the + // converter's `unknown` fallback for function-shaped tools without a + // name). Prevents legitimate `read_file` / `write_file` results that + // contain the literal marker string (e.g. reading this repo's + // `docs/guide/cursor.md`, which documents the intercept) from being + // rewritten as `no_input_surface` failures. + if (!ASK_QUESTION_TOOL_NAMES.has(opts.name)) { + return false; + } + // Search only the extracted result, not the whole tool_call payload. The + // `arguments` field carries the agent's own prompt text, which can quote + // the marker without that being a fabricated skip; matching there would + // false-positive on legitimate AskQuestion calls that ask about this + // exact bug or paste the marker verbatim into their prompt. + if (containsSyntheticSkipMarker(opts.result)) { + return true; + } + if ( + opts.elapsedMs !== null && + opts.elapsedMs < SYNTHETIC_LATENCY_THRESHOLD_MS && + isTrivialResult(opts.result) + ) { + return true; + } + return false; +} + +/** + * Test-only hook to reset the timing tracker between test cases. Not exported + * from the package surface; consumed by the colocated test file. + */ +export function __resetCursorEventConverterStateForTests(): void { + toolCallStartedAt.clear(); +} + export function convertCursorEventToAgentMessage(event: CursorStreamEvent): AgentMessage | null { switch (event.type) { case 'assistant': { @@ -102,6 +261,7 @@ export function convertCursorEventToAgentMessage(event: CursorStreamEvent): Agen const name = extractToolName(toolCall); const input = extractToolInput(toolCall); if (event.subtype === 'started') { + rememberToolCallStart(event.call_id); return { type: 'tool_call', id: event.call_id, @@ -111,6 +271,15 @@ export function convertCursorEventToAgentMessage(event: CursorStreamEvent): Agen }; } const result = extractToolResult(toolCall); + const elapsedMs = takeToolCallElapsedMs(event.call_id); + if (shouldRewriteAsNoInputSurface({ name, result, elapsedMs })) { + return { + type: 'tool_result', + id: event.call_id, + output: { ...NO_INPUT_SURFACE_OUTPUT }, + status: 'failed' + }; + } return { type: 'tool_result', id: event.call_id, diff --git a/docs/guide/cursor.md b/docs/guide/cursor.md index b69c67ea..434214d0 100644 --- a/docs/guide/cursor.md +++ b/docs/guide/cursor.md @@ -48,6 +48,16 @@ Set mode via `--mode` flag or change from the web UI during a session. - **Tool approval** - In remote mode, `--trust` is used; tools run without per-request approval. Use `--yolo` for full bypass. - **Session resume** - Pass `--resume ` or `--continue` to resume. Use `agent ls` to list previous chats and get chat IDs. +### Headless safety: AskQuestion behavior + +When running cursor-agent under `--print --output-format stream-json` (HAPI's current remote mode), the cursor-agent CLI returns a synthetic `Questions skipped by the user, continue with the information you already have` response for the `AskQuestion` tool because there is no IDE surface to render the question. The agent's underlying model can interpret this as legitimate user consent and act on it. + +HAPI intercepts this synthetic response in the stream-json event converter and rewrites it to an explicit `no_input_surface` error (`is_error: true`), so agents do not act on fabricated user consent. Defense-in-depth: any `AskQuestion` (or `name=unknown`) tool completion that arrives within ~500 ms of its start event with a trivial payload is treated the same way, in case cursor-agent changes the synthetic-string text in a future release. + +Agents running under HAPI's Cursor remote mode should fall back to plain-text prompting (markdown options + waiting for a regular user message) until the [ACP migration (tiann/hapi#781)](https://github.com/tiann/hapi/issues/781) lands and `cursor/ask_question` becomes available as a proper bidirectional ACP method. At that point this intercept becomes unnecessary and is removed. + +Tracking issue: [tiann/hapi#784](https://github.com/tiann/hapi/issues/784). + ## Integration Once running, your Cursor session appears in the HAPI web app and Telegram Mini App. You can: