fix(cursor): intercept fabricated Questions skipped AskQuestion result in headless mode (#784) (#801)

* fix(cursor): intercept fabricated 'Questions skipped' AskQuestion result in headless mode (#784)

When cursor-agent runs under `--print --output-format stream-json` (HAPI's
current Cursor remote launcher), the CLI returns a synthetic
`Questions skipped by the user, continue with the information you already have`
response for the `AskQuestion` tool in ~zero seconds with no error flag,
because there is no IDE surface to render the question. The underlying
model can interpret this as legitimate user consent and act on it.

This patch intercepts the synthetic result in
`cli/src/cursor/utils/cursorEventConverter.ts` and rewrites the
`tool_call`/completed event to a structured `no_input_surface` failure
(`status: 'failed'`, which downstream becomes `is_error: true`).

Detection has two strategies:

1. String match - any `tool_call`/completed payload whose serialized form
   contains the synthetic-skip marker is rewritten. This is robust to
   wherever cursor-agent stuffs the marker inside the `tool_call` object.
2. Timing + name heuristic (defense in depth) - any completion that arrives
   within 500 ms of its 'started' event with a trivial result, for a tool
   call named `AskQuestion`, `askQuestion`, `ask_question`, or the
   converter's `unknown` fallback, is also rewritten. This catches the case
   where cursor-agent changes the synthetic-string text in a future release.

The converter tracks per-call timestamps in a bounded `Map` (`<= 1024`
entries, oldest evicted on overflow) and clears entries when the
corresponding 'completed' event arrives. A small test-only reset hook
isolates state between Vitest cases.

This is a transitional safety patch. It auto-deletes when #781's ACP
launcher replaces the stream-json launcher and `cursor/ask_question`
becomes a proper bidirectional ACP method where fabrication is
structurally impossible.

Scope is intentionally tiny: only `cli/src/cursor/utils/cursorEventConverter.ts`,
its colocated Vitest file, and a section in `docs/guide/cursor.md`. No
changes to `cursorRemoteLauncher.ts`, ACP code, web normalizer, or
permission UI.

Refs: tiann/hapi#781 (long-term resolution via ACP migration)
Closes: tiann/hapi#784

* fix(cursor): gate AskQuestion intercept on tool name (#784 PR #801 review)

Address regression flagged by the HAPI auto-review bot on #801:

`containsSyntheticSkipMarker` previously stringified the entire `tool_call`
payload and matched the literal marker substring. Because this PR also adds
that exact marker to `docs/guide/cursor.md` (to document the intercept), a
Cursor `read_file` of that documentation page would surface the marker
inside `readToolCall.result.content` and be rewritten as a
`no_input_surface` failure, corrupting an unrelated, legitimate result.

The intercept is now gated on the tool name resolving to an
AskQuestion-shaped call (`AskQuestion`, `askQuestion`, `ask_question`, or
the converter's `unknown` fallback for unnamed function-shaped tools).
`read_file` / `write_file` tool calls - which have explicit `read_file`
and `write_file` names from `extractToolName` - no longer fall under the
intercept, regardless of what their payload contains.

The marker check itself now walks values recursively (string / array /
object), guarded by a `WeakSet` against cycles, instead of relying on
`JSON.stringify`. Slightly tidier; behaviour is otherwise unchanged for
the AskQuestion path.

Regression tests added:

- `read_file` result whose `content` contains the marker -> passes
  through with `status: 'completed'` and no `no_input_surface`.
- `write_file` whose serialized `args` contain the marker -> same.
- A non-AskQuestion function tool (`MyCustomTool`) whose result quotes
  the marker -> same.

All 846 cli tests pass (17 in this file). `bun run typecheck` exits 0.

* fix(cursor): scope synthetic-skip check to extracted result (#784 PR #801 review-2)

Address second Major finding from the HAPI auto-review bot on #801:

After the previous fix gated the intercept on the tool name, the marker
check still recursed into the entire `tool_call` object - which includes
`function.arguments`, the agent's own prompt text. A legitimate
AskQuestion whose prompt quotes the synthetic-skip marker (e.g. an agent
debugging this exact bug, or any prompt that pastes the marker verbatim)
would have been rewritten as `no_input_surface` even when the operator
actually answered.

Changes:

1. `extractToolResult` now extracts the cursor-side response from
   function-shaped tool calls. Previously it returned `{}` for anything
   that wasn't `readToolCall` or `writeToolCall`. It now returns
   `function.result` when present, otherwise every field of `function`
   except `name` and `arguments`. This excludes the agent's input from
   what downstream sees as the tool result, and as a side effect surfaces
   the actual cursor response for function-shaped tools (which was
   previously lost - see the #784 incident note about HAPI storing
   `output: {}` for AskQuestion in the message DB).

2. `shouldRewriteAsNoInputSurface` now searches only the extracted
   `result`, not the whole `tool_call`. The bot's exact recommendation.

3. Test added: an AskQuestion whose `arguments` quote the marker but
   whose `result` is a real user answer, with elapsed time past the
   500 ms threshold so the timing heuristic does not apply. Asserts the
   tool_result passes through with `status: 'completed'` and the
   operator's actual answer.

All 847 cli tests pass (18 in `cursorEventConverter.test.ts`).
`bun run typecheck` exits 0.

The widened `extractToolResult` scope is necessary for the marker check
to actually find the synthetic string (it lives inside `function.result`
or a sibling field), and is the bot's explicit recommendation. It also
removes the long-standing data-loss bug where AskQuestion responses were
surfaced to the message DB as opaque `{}` - regardless of fabrication.
This commit is contained in:
HeavyGee
2026-06-05 21:44:37 +08:00
committed by GitHub
parent a812a51dd7
commit dc0d21e05b
3 changed files with 504 additions and 1 deletions
@@ -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' });
});
});
});
@@ -84,9 +84,168 @@ function extractToolResult(toolCall: Record<string, unknown>): unknown {
const w = toolCall.writeToolCall as Record<string, unknown>;
return w.result ?? w;
}
if (toolCall.function && typeof toolCall.function === 'object') {
const fn = toolCall.function as Record<string, unknown>;
// 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<string, unknown> = {};
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<string, number>();
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<object> = 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<string, unknown>).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<string, unknown>).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,