fix(cursor): pair interrupted calls and gate advisor bridge approval
Two independent bugs found in review. A stream that dies mid-turn takes the terminal-error path: `settleH2` rejects when the transport closes without `turnEnded`, so the flush on the success path never runs. `connect_scm` and native todo blocks are stamped `kCursorExecResolved` at start, so `agent-loop.ts` synthesizes no placeholder and only their completion frame pairs a result - the call was left unpaired and its card animating, and `buildSessionContext` strips a dangling call from every rebuilt transcript. The catch path now closes open blocks and pairs those server-owned calls with an interrupted result. Exec-settled MCP blocks are excluded: the dispatch that ran them owns their result and `drainInFlightDispatches` awaits it, so pairing here would duplicate against the same id. Separately, the advisor bridge supplied no `getToolContext`. `ExtensionToolWrapper` reads the approval mode, per-tool policies and `autoApprove` only from that execute-time context, so every wrapped advisor bridge tool resolved as `yolo` with empty policies - a configured `ask` or `deny` on `edit`/`grep` did not apply to native frames. Advisors now get the same `ToolContextStore` as the primary bridge. Both are covered against the real paths: the interrupted call through the HTTP/2 fixture server (a helper-level test passes even with the catch-path flush removed), and approval through real deny policies. (cherry picked from commit 5ace682578af96708caf88db2d44b9077a1c0e74)
This commit is contained in:
committed by
can1357
parent
5779c762de
commit
a363f95009
@@ -82,6 +82,7 @@
|
||||
- Fixed interleaved Cursor tool calls corrupting each other. The stream decoder tracked a single "current" block and settled it on any `toolCallCompleted`, ignoring the envelope's `call_id`: a completion for one call closed whichever block happened to be open and paired it with the wrong result, and `start A, start B` orphaned A entirely so its own completion settled B while A was never paired — which strips the whole interaction from every rebuilt transcript. Open blocks are now retained per envelope `call_id`, and end-of-stream closes all of them rather than only the last.
|
||||
- Fixed a Cursor `search_conversations` call leaving no transcript block. The frame is answered from a fixed verdict, so nothing downstream pairs a result for it, and an unpaired call takes its whole interaction out of every rebuilt transcript.
|
||||
- Fixed the Cursor stream's end-of-transport cleanup erasing the arguments of every block still open. Blocks whose args arrive whole (todo, connect-SCM, MCP) never feed the streamed partial-JSON buffer, and reparsing an absent buffer yields `{}`, so a truncated or disconnected turn rebuilt those calls with no arguments at all. Only blocks that actually streamed their args are reparsed now.
|
||||
- Fixed a Cursor stream dying mid-turn stranding the call it left open. `connect_scm` and native todo blocks are stamped resolved the moment they open, so the agent loop synthesizes no placeholder and only their completion frame pairs a result — a transport that closed first left the card animating and the call unpaired, which takes the whole interaction out of every rebuilt transcript. The terminal-error path now closes open blocks and pairs those server-owned calls with an interrupted result; the flush ran only on clean completion before, which is not the path a dying stream takes. Exec-settled MCP blocks are left alone, since the dispatch that ran them owns their result.
|
||||
- Fixed the Pi exec frames displaying a different operation than the one they run. The provider synthesized its transcript block from a second, hand-rolled translation of the frame args, so `pi_read`'s `offset`/`limit` were shown as a whole-file read, `pi_grep`'s `literal` pattern as an unescaped regex, and `pi_find`'s path/glob join differed from the executed one. Both sides now share a single translation.
|
||||
- Fixed the streamed `pi_*_tool_call` announcements that modern builds send alongside each exec frame being unrecognized. The exec channel already synthesizes those blocks when it runs the tool; the duplicate was avoided only because the decoder recognized none of the variants, which would have started double-rendering as soon as any one was added.
|
||||
- Fixed `pi_bash` results reaching Cursor clipped with no truncation notice. Two truncation records exist locally: `read`/`grep` set `details.truncation`, which carries an explicit `truncated` flag, while `bash` sets `details.meta.truncation`, whose record has no such flag — its presence is the signal. `piTruncation` read only the first shape and required the flag, so every real Bash truncation was dropped and the server was told the clipped output was complete. Both shapes now translate, and an explicit `truncated: false` still suppresses the field.
|
||||
|
||||
@@ -478,6 +478,10 @@ export const streamCursor: StreamFunction<"cursor-agent"> = (
|
||||
let h2Settled = false;
|
||||
let sawTurnEnded = false;
|
||||
let endStreamError: Error | null = null;
|
||||
// Reachable from the catch: a stream that dies mid-turn must still close
|
||||
// and pair the blocks it left open, and `state` itself is scoped to the
|
||||
// try below.
|
||||
let openBlockState: BlockState | undefined;
|
||||
const settleH2 = (error?: unknown): void => {
|
||||
if (h2Settled) return;
|
||||
h2Settled = true;
|
||||
@@ -597,6 +601,7 @@ export const streamCursor: StreamFunction<"cursor-agent"> = (
|
||||
onTodoSnapshot: options?.execHandlers?.todoSync?.bind(options.execHandlers),
|
||||
onToolResult: options?.onToolResult,
|
||||
};
|
||||
openBlockState = state;
|
||||
|
||||
const onConversationCheckpoint = (checkpoint: ConversationStateStructure) => {
|
||||
conversationStateCache.set(conversationId, checkpoint);
|
||||
@@ -756,6 +761,19 @@ export const streamCursor: StreamFunction<"cursor-agent"> = (
|
||||
// (handlers have no cancellation contract and must not delay the
|
||||
// terminal error the user asked for).
|
||||
await drainInFlightDispatches();
|
||||
// A stream that dies mid-turn leaves blocks open, and this is the path
|
||||
// it takes: `settleH2` rejects when the transport closes without
|
||||
// `turnEnded`, so the success-path flush above never runs. Closing
|
||||
// them here settles their live cards and pairs the server-owned calls
|
||||
// (`connect_scm`, native todo) that nothing else answers — an
|
||||
// unpaired call is stripped from every rebuilt transcript.
|
||||
// Undefined only when the failure predates the state's construction,
|
||||
// in which case no block was ever opened.
|
||||
if (openBlockState) {
|
||||
endCurrentTextBlock(output, stream, openBlockState);
|
||||
endCurrentThinkingBlock(output, stream, openBlockState);
|
||||
flushOpenToolCalls(output, stream, openBlockState);
|
||||
}
|
||||
const result = await AIError.finalize(error, { api: model.api, signal: options?.signal });
|
||||
output.stopReason = result.stopReason;
|
||||
output.errorStatus = result.status;
|
||||
@@ -2823,6 +2841,18 @@ function isExecOwnedToolCall(toolCall: { tool?: { case?: string } } | undefined)
|
||||
* never set the partial buffer; `parseStreamingJson(undefined)` returns `{}`,
|
||||
* so reparsing unconditionally would erase the arguments of every such block
|
||||
* caught open by a truncated stream.
|
||||
*
|
||||
* Server-owned blocks are also paired here. `connect-scm` and `todo` are
|
||||
* stamped {@link kCursorExecResolved} the moment they open, so `agent-loop.ts`
|
||||
* synthesizes no placeholder for them and only their `toolCallCompleted` frame
|
||||
* pairs a result. A transport that closes before that frame would leave the
|
||||
* call unpaired, and `buildSessionContext` strips a dangling call from every
|
||||
* rebuilt transcript — the interaction disappears. An interrupted result is
|
||||
* emitted instead.
|
||||
*
|
||||
* MCP blocks are excluded even when resolved: the exec dispatch that marked
|
||||
* them owns their result, and `drainInFlightDispatches` awaits it before this
|
||||
* runs, so pairing here would duplicate one against the same `toolCallId`.
|
||||
*/
|
||||
export function flushOpenToolCalls(
|
||||
output: AssistantMessage,
|
||||
@@ -2838,6 +2868,17 @@ export function flushOpenToolCalls(
|
||||
block.arguments = parseStreamingJson(partialJson);
|
||||
clearStreamingPartialJson(block);
|
||||
}
|
||||
const kind = block[kStreamingBlockKind];
|
||||
if (kind === "connect-scm" || kind === "todo") {
|
||||
state.onToolResult?.({
|
||||
role: "toolResult",
|
||||
toolCallId: block.id,
|
||||
toolName: block.name,
|
||||
content: [{ type: "text", text: "The connection to Cursor closed before this call completed." }],
|
||||
isError: true,
|
||||
timestamp: Date.now(),
|
||||
});
|
||||
}
|
||||
stream.push({ type: "toolcall_end", contentIndex: idx, toolCall: block, partial: output });
|
||||
}
|
||||
state.openToolCalls.clear();
|
||||
|
||||
@@ -139,7 +139,7 @@ function cursorAssistantMessage(): AssistantMessage {
|
||||
};
|
||||
}
|
||||
|
||||
function newBlockState(): BlockState {
|
||||
function newBlockState(overrides: Partial<BlockState> = {}): BlockState {
|
||||
let textBlock: BlockState["currentTextBlock"] = null;
|
||||
let thinkingBlock: BlockState["currentThinkingBlock"] = null;
|
||||
let toolCall: ToolCallState | null = null;
|
||||
@@ -166,6 +166,7 @@ function newBlockState(): BlockState {
|
||||
toolCall = t;
|
||||
},
|
||||
setFirstTokenTime: () => {},
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -271,6 +272,75 @@ describe("Cursor stream teardown", () => {
|
||||
|
||||
expect(block.arguments).toEqual({ path: "/repo/a.ts" });
|
||||
});
|
||||
|
||||
it("pairs a server-owned call the transport cut short", async () => {
|
||||
// `connect_scm` and native todo blocks are stamped `kCursorExecResolved`
|
||||
// at start, so `agent-loop.ts` synthesizes no placeholder for them and
|
||||
// only their completion frame pairs a result. A transport that dies
|
||||
// first left the call unpaired, and `buildSessionContext` strips a
|
||||
// dangling call — the whole interaction vanished from replay.
|
||||
const output = cursorAssistantMessage();
|
||||
const stream = new AssistantMessageEventStream();
|
||||
const paired: ToolResultMessage[] = [];
|
||||
const state = newBlockState({ onToolResult: result => void paired.push(result) });
|
||||
|
||||
processInteractionUpdate(
|
||||
{
|
||||
message: {
|
||||
case: "toolCallStarted",
|
||||
value: {
|
||||
callId: "envelope-todo",
|
||||
toolCall: { tool: { case: "updateTodosToolCall", value: { args: { todos: [] } } } },
|
||||
},
|
||||
},
|
||||
},
|
||||
output,
|
||||
stream,
|
||||
state,
|
||||
{ sawTokenDelta: false },
|
||||
);
|
||||
|
||||
const block = output.content.find((b): b is ToolCallState => b.type === "toolCall");
|
||||
if (!block) throw new Error("expected an open tool-call block");
|
||||
expect(paired).toHaveLength(0);
|
||||
|
||||
flushOpenToolCalls(output, stream, state);
|
||||
|
||||
expect(paired).toHaveLength(1);
|
||||
expect(paired[0].toolCallId).toBe(block.id);
|
||||
expect(paired[0].isError).toBe(true);
|
||||
});
|
||||
|
||||
it("leaves an exec-settled MCP call to the dispatch that owns its result", async () => {
|
||||
// MCP blocks are also marked resolved, but by the exec dispatch, which
|
||||
// already emitted their result and is awaited before teardown. Pairing
|
||||
// one here too would file a duplicate against the same `toolCallId`.
|
||||
const output = cursorAssistantMessage();
|
||||
const stream = new AssistantMessageEventStream();
|
||||
const paired: ToolResultMessage[] = [];
|
||||
const state = newBlockState({ onToolResult: result => void paired.push(result) });
|
||||
state.resolvedMcpToolCallIds.add("mcp-1");
|
||||
|
||||
processInteractionUpdate(
|
||||
{
|
||||
message: {
|
||||
case: "toolCallStarted",
|
||||
value: {
|
||||
callId: "envelope-mcp",
|
||||
toolCall: { tool: { case: "mcpToolCall", value: { args: { toolCallId: "mcp-1" } } } },
|
||||
},
|
||||
},
|
||||
},
|
||||
output,
|
||||
stream,
|
||||
state,
|
||||
{ sawTokenDelta: false },
|
||||
);
|
||||
|
||||
flushOpenToolCalls(output, stream, state);
|
||||
|
||||
expect(paired).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Cursor modern exec frames: failure channel", () => {
|
||||
|
||||
@@ -2,7 +2,7 @@ import { afterEach, describe, expect, it } from "bun:test";
|
||||
import * as http2 from "node:http2";
|
||||
import { create, toBinary } from "@bufbuild/protobuf";
|
||||
import { streamCursor } from "@oh-my-pi/pi-ai/providers/cursor";
|
||||
import type { Context, Model } from "@oh-my-pi/pi-ai/types";
|
||||
import type { Context, CursorToolResultHandler, Model, ToolResultMessage } from "@oh-my-pi/pi-ai/types";
|
||||
import { buildModel } from "@oh-my-pi/pi-catalog/build";
|
||||
import {
|
||||
AgentServerMessageSchema,
|
||||
@@ -10,7 +10,11 @@ import {
|
||||
InteractionUpdateSchema,
|
||||
ReadArgsSchema,
|
||||
TextDeltaUpdateSchema,
|
||||
ToolCallSchema,
|
||||
ToolCallStartedUpdateSchema,
|
||||
TurnEndedUpdateSchema,
|
||||
UpdateTodosArgsSchema,
|
||||
UpdateTodosToolCallSchema,
|
||||
} from "@oh-my-pi/pi-catalog/discovery/cursor-gen/agent_pb";
|
||||
|
||||
const CONNECT_END_STREAM_FLAG = 0b00000010;
|
||||
@@ -23,7 +27,8 @@ type Scenario =
|
||||
| { kind: "hang-after-turn" }
|
||||
| { kind: "exec-in-final-chunk"; responseFinished: PromiseWithResolvers<void> }
|
||||
| { kind: "exec-then-transport-error"; responseFinished: PromiseWithResolvers<void> }
|
||||
| { kind: "exec-then-hang" };
|
||||
| { kind: "exec-then-hang" }
|
||||
| { kind: "todo-start-then-death" };
|
||||
|
||||
let server: http2.Http2Server | undefined;
|
||||
const sessions = new Set<http2.Http2Session>();
|
||||
@@ -104,6 +109,36 @@ function execAndTurnEndedFrame(): Buffer {
|
||||
return Buffer.concat([execRequestFrame(), turnEndedFrame()]);
|
||||
}
|
||||
|
||||
/**
|
||||
* A native `update_todos` call announcement. Cursor runs these server-side, so
|
||||
* the block is stamped resolved at start and only its `toolCallCompleted`
|
||||
* frame pairs a result — nothing downstream synthesizes one.
|
||||
*/
|
||||
function todoStartFrame(): Buffer {
|
||||
const message = create(AgentServerMessageSchema, {
|
||||
message: {
|
||||
case: "interactionUpdate",
|
||||
value: create(InteractionUpdateSchema, {
|
||||
message: {
|
||||
case: "toolCallStarted",
|
||||
value: create(ToolCallStartedUpdateSchema, {
|
||||
callId: "todo-envelope",
|
||||
toolCall: create(ToolCallSchema, {
|
||||
tool: {
|
||||
case: "updateTodosToolCall",
|
||||
value: create(UpdateTodosToolCallSchema, {
|
||||
args: create(UpdateTodosArgsSchema, { todos: [] }),
|
||||
}),
|
||||
},
|
||||
}),
|
||||
}),
|
||||
},
|
||||
}),
|
||||
},
|
||||
});
|
||||
return frameConnectMessage(toBinary(AgentServerMessageSchema, message));
|
||||
}
|
||||
|
||||
async function startServer(): Promise<string> {
|
||||
server = http2.createServer();
|
||||
server.on("session", session => {
|
||||
@@ -150,6 +185,16 @@ async function startServer(): Promise<string> {
|
||||
return;
|
||||
}
|
||||
|
||||
if (scenario.kind === "todo-start-then-death") {
|
||||
// The server announces a native todo call, then the stream dies
|
||||
// without `turnEnded` and without the call's completion frame. This
|
||||
// is the real interrupted-call shape: `settleH2` rejects, so the
|
||||
// success-path flush never runs.
|
||||
stream.write(todoStartFrame());
|
||||
stream.end();
|
||||
return;
|
||||
}
|
||||
|
||||
if (scenario.kind === "exec-in-final-chunk") {
|
||||
const { responseFinished } = scenario;
|
||||
// Resolves once the server has flushed the whole response, so the test
|
||||
@@ -226,8 +271,15 @@ const context: Context = {
|
||||
messages: [{ role: "user", content: "terminal lifecycle", timestamp: 1 }],
|
||||
};
|
||||
|
||||
async function collectStream(model: Model<"cursor-agent">, options?: { signal?: AbortSignal }) {
|
||||
const stream = streamCursor(model, context, { apiKey: "test-token", signal: options?.signal });
|
||||
async function collectStream(
|
||||
model: Model<"cursor-agent">,
|
||||
options?: { signal?: AbortSignal; onToolResult?: CursorToolResultHandler },
|
||||
) {
|
||||
const stream = streamCursor(model, context, {
|
||||
apiKey: "test-token",
|
||||
signal: options?.signal,
|
||||
onToolResult: options?.onToolResult,
|
||||
});
|
||||
const eventTypes: string[] = [];
|
||||
for await (const event of stream) {
|
||||
eventTypes.push(event.type);
|
||||
@@ -303,6 +355,37 @@ describe("Cursor terminal lifecycle after turnEnded", () => {
|
||||
expect(result.errorMessage).toContain("Cursor stream ended before turnEnded");
|
||||
});
|
||||
|
||||
it("pairs and closes a server-owned call the dying stream left open", async () => {
|
||||
// The failure this guards: a native todo block is stamped resolved at
|
||||
// start, so `agent-loop.ts` synthesizes no placeholder for it and only
|
||||
// its completion frame pairs a result. When the transport dies first the
|
||||
// call went unpaired and its card stayed animating — and
|
||||
// `buildSessionContext` strips a dangling call, so the interaction
|
||||
// vanished from every rebuilt transcript.
|
||||
//
|
||||
// This must run against the real terminal-error path: `settleH2` rejects
|
||||
// on a stream that ends before `turnEnded`, so the success path's flush
|
||||
// is never reached.
|
||||
scenario = { kind: "todo-start-then-death" };
|
||||
const baseUrl = await startServer();
|
||||
const paired: ToolResultMessage[] = [];
|
||||
const { eventTypes, result } = await collectStream(makeModel(baseUrl), {
|
||||
onToolResult: toolResult => void paired.push(toolResult),
|
||||
});
|
||||
|
||||
expect(eventTypes.at(-1)).toBe("error");
|
||||
expect(result.stopReason).toBe("error");
|
||||
|
||||
const call = result.content.find(block => block.type === "toolCall");
|
||||
if (!call) throw new Error("expected the announced todo call in the output");
|
||||
// Closed, so no live card is left animating.
|
||||
expect(eventTypes).toContain("toolcall_end");
|
||||
// Paired, so replay keeps the interaction.
|
||||
expect(paired).toHaveLength(1);
|
||||
expect(paired[0].toolCallId).toBe(call.id);
|
||||
expect(paired[0].isError).toBe(true);
|
||||
});
|
||||
|
||||
it("aborts without emitting done when the signal fires", async () => {
|
||||
scenario = { kind: "hang-after-turn" };
|
||||
const baseUrl = await startServer();
|
||||
|
||||
@@ -142,6 +142,7 @@
|
||||
- Fixed Cursor advisors failing every `pi_edit`. The advisor roster handed the bridge the `edit` instance built for the advisor's own loop, which follows the configured `edit.mode` (`hashline` by default) and rejects the frame's `old_text`/`new_text` pairs — the same mode mismatch the primary bridge already fixed, on the path it missed. The exec map now substitutes a `replace`-mode instance, gated on the advisor actually having been granted `edit`, while the advisor's own loop keeps the tool it was given.
|
||||
- Fixed `pi_bash` killing commands that explicitly asked for no deadline. `timeout` is `optional int32` and `bash` documents `0` as "disables the command deadline", but a truthiness check folded a supplied `0` into unset, applying the 300s default instead. A present `0` now passes through; negatives, which have no local meaning and would otherwise clamp to the 1s floor, still fall back to the default.
|
||||
- Fixed the Cursor exec bridge granting `edit` and `grep` to sessions that withheld them. Both bridge-only tools are constructed rather than looked up, and `executeTool` prefers a constructed override over the registry, so a restricted tool set (`toolNames` without them, or `restrictToolNames`) still got a working `pi_edit`/`pi_grep` — native frames arrive regardless of the advertised catalog. Both are now gated on the session having actually granted the tool, matching the `delete` frame's existing check (issue #5680).
|
||||
- Fixed Cursor advisor bridge tools bypassing approval settings. The advisor's `pi_edit`/`pi_grep` instances are approval-wrapped, but the wrapper reads `tools.approvalMode`, per-tool `tools.approval.<tool>` policies and `autoApprove` only from the execute-time tool context — which the advisor bridge never supplied, so every native advisor frame resolved as `yolo` with empty policies and ran past a configured `ask` or `deny`. Advisors now receive the same context store as the primary bridge.
|
||||
|
||||
## [17.1.5] - 2026-07-27
|
||||
|
||||
|
||||
@@ -3275,6 +3275,10 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
|
||||
// Same `replace`-mode requirement as the primary bridge; the advisor
|
||||
// path gates it on the advisor's own `edit` grant.
|
||||
advisorCreateEditTool: () => createBridgeEditTool(advisorToolSession, extensionRunner),
|
||||
// The advisor's bridge tools are wrapped for approval, but the wrapper
|
||||
// reads the mode and per-tool policies only from the execute-time
|
||||
// context — the primary bridge passes the same store.
|
||||
advisorGetToolContext: () => toolContextStore.getContext(),
|
||||
titleSystemPrompt: options.titleSystemPrompt,
|
||||
});
|
||||
hasSession = true;
|
||||
|
||||
@@ -1,4 +1,11 @@
|
||||
import type { Agent, AgentMessage, AgentTool, StreamFn, ThinkingLevel } from "@oh-my-pi/pi-agent-core";
|
||||
import type {
|
||||
Agent,
|
||||
AgentMessage,
|
||||
AgentTool,
|
||||
AgentToolContext,
|
||||
StreamFn,
|
||||
ThinkingLevel,
|
||||
} from "@oh-my-pi/pi-agent-core";
|
||||
import type {
|
||||
Context,
|
||||
Effort,
|
||||
@@ -221,6 +228,15 @@ export interface AgentSessionConfig {
|
||||
* configured `edit.mode` and rejects the frame's `old_text`/`new_text` pairs.
|
||||
*/
|
||||
advisorCreateEditTool?(): AgentTool | undefined;
|
||||
/**
|
||||
* The execute-time context the advisor's bridge tools resolve approval from.
|
||||
*
|
||||
* `ExtensionToolWrapper` reads `tools.approvalMode`, per-tool
|
||||
* `tools.approval.<tool>` policies and `autoApprove` only from this context;
|
||||
* with none it defaults to `yolo` with empty policies, so a bridge tool would
|
||||
* run a native frame the user configured `ask` or `deny` for.
|
||||
*/
|
||||
advisorGetToolContext?: () => AgentToolContext | undefined;
|
||||
/** Preloaded watchdog prompt content for the advisor. */
|
||||
advisorWatchdogPrompt?: string;
|
||||
/** Shared advisor instructions loaded from WATCHDOG.yml. */
|
||||
|
||||
@@ -1292,6 +1292,7 @@ export class AgentSession {
|
||||
tools: config.advisorTools,
|
||||
createGrepTool: config.advisorCreateGrepTool,
|
||||
createEditTool: config.advisorCreateEditTool,
|
||||
getToolContext: config.advisorGetToolContext,
|
||||
watchdogPrompt: config.advisorWatchdogPrompt,
|
||||
sharedInstructions: config.advisorSharedInstructions,
|
||||
contextPrompt: config.advisorContextPrompt,
|
||||
|
||||
@@ -2,6 +2,7 @@ import {
|
||||
Agent,
|
||||
type AgentMessage,
|
||||
type AgentTool,
|
||||
type AgentToolContext,
|
||||
AppendOnlyContextManager,
|
||||
type CompactionSummaryMessage,
|
||||
countTokens,
|
||||
@@ -192,6 +193,14 @@ export interface SessionAdvisorsOptions {
|
||||
* match, so without this every native advisor edit fails validation.
|
||||
*/
|
||||
createEditTool?(): AgentTool | undefined;
|
||||
/**
|
||||
* The execute-time context the bridge's tools resolve approval from.
|
||||
*
|
||||
* `ExtensionToolWrapper` reads the approval mode, per-tool policies and
|
||||
* `autoApprove` only from here; with none it falls back to `yolo` and empty
|
||||
* policies, so a native frame would run past a configured `ask` or `deny`.
|
||||
*/
|
||||
getToolContext?: () => AgentToolContext | undefined;
|
||||
watchdogPrompt?: string;
|
||||
sharedInstructions?: string;
|
||||
contextPrompt?: string;
|
||||
@@ -267,6 +276,7 @@ export class SessionAdvisors {
|
||||
#advisorTools: AgentTool[] | undefined;
|
||||
#advisorCreateGrepTool: SessionAdvisorsOptions["createGrepTool"];
|
||||
#advisorCreateEditTool: SessionAdvisorsOptions["createEditTool"];
|
||||
#advisorGetToolContext: SessionAdvisorsOptions["getToolContext"];
|
||||
#advisorWatchdogPrompt: string | undefined;
|
||||
#advisorSharedInstructions: string | undefined;
|
||||
#advisorContextPrompt: string | undefined;
|
||||
@@ -291,6 +301,7 @@ export class SessionAdvisors {
|
||||
this.#advisorTools = options.tools;
|
||||
this.#advisorCreateGrepTool = options.createGrepTool;
|
||||
this.#advisorCreateEditTool = options.createEditTool;
|
||||
this.#advisorGetToolContext = options.getToolContext;
|
||||
this.#advisorWatchdogPrompt = options.watchdogPrompt;
|
||||
this.#advisorSharedInstructions = options.sharedInstructions;
|
||||
this.#advisorContextPrompt = options.contextPrompt;
|
||||
@@ -747,6 +758,9 @@ export class SessionAdvisors {
|
||||
cwd: this.#host.sessionManager.getCwd(),
|
||||
getCwd: () => this.#host.sessionManager.getCwd(),
|
||||
tools: bridgeToolMap(advisorToolMap, this.#advisorCreateEditTool),
|
||||
// Approval mode, per-tool policies and `autoApprove` live only on
|
||||
// this context; without it every bridge tool resolves as `yolo`.
|
||||
getToolContext: this.#advisorGetToolContext,
|
||||
allowNativeDelete: advisorCanMutateFiles,
|
||||
// Gated on the advisor's own grant: the factory builds a fresh
|
||||
// tool, so handing it over unconditionally would give a roster
|
||||
|
||||
@@ -344,6 +344,55 @@ describe("bridge tool resolution beyond the model-facing registry", () => {
|
||||
expect(result.content.map(c => (c.type === "text" ? c.text : "")).join("")).toContain("not available");
|
||||
});
|
||||
|
||||
it("denies a native pi_edit frame the user's policy blocks", async () => {
|
||||
// The bridge's `edit` is wrapped, but `ExtensionToolWrapper` reads the
|
||||
// approval mode and per-tool policies only from the execute-time
|
||||
// context — with none it resolves as `yolo` with empty policies and the
|
||||
// frame edits the file regardless of what the user configured.
|
||||
const target = path.join(cwd, "denied.txt");
|
||||
await Bun.write(target, "alpha\nbeta\n");
|
||||
const settings = Settings.isolated({ "tools.approval": { edit: "deny" } });
|
||||
const session = createTestSession(cwd, { settings });
|
||||
const handlers = new CursorExecHandlers({
|
||||
cwd,
|
||||
tools: bridgeToolMap(new Map<string, Tool>([["edit", new EditTool(session)]]), () =>
|
||||
createBridgeEditTool(session, passthroughRunner()),
|
||||
),
|
||||
getToolContext: () => ({ settings }) as AgentToolContext,
|
||||
});
|
||||
|
||||
const result = await handlers.piEdit({
|
||||
toolCallId: "e5",
|
||||
args: { path: target, edits: [{ oldText: "beta", newText: "gamma" }] },
|
||||
} as never);
|
||||
|
||||
expect(result.isError).toBe(true);
|
||||
expect(await Bun.file(target).text()).toBe("alpha\nbeta\n");
|
||||
});
|
||||
|
||||
it("denies a scoped pi_grep frame the user's policy blocks", async () => {
|
||||
// Same gate on the other bridge-only tool: the per-call `grep` the
|
||||
// factory builds for a frame carrying `context`/`limit` must answer to
|
||||
// `tools.approval.grep` like every registry call.
|
||||
await Bun.write(path.join(cwd, "hit.txt"), "needle\n");
|
||||
const settings = Settings.isolated({ "tools.approval": { grep: "deny" } });
|
||||
const session = createTestSession(cwd, { settings });
|
||||
const handlers = new CursorExecHandlers({
|
||||
cwd,
|
||||
tools: new Map<string, Tool>(),
|
||||
createGrepTool: createBridgeGrepFactory(session, passthroughRunner()),
|
||||
getToolContext: () => ({ settings }) as AgentToolContext,
|
||||
});
|
||||
|
||||
const result = await handlers.piGrep({
|
||||
toolCallId: "g2",
|
||||
args: { pattern: "needle", path: cwd, context: 1, limit: 5 },
|
||||
} as never);
|
||||
|
||||
expect(result.isError).toBe(true);
|
||||
expect(result.content.map(c => (c.type === "text" ? c.text : "")).join("")).toContain("blocked by user policy");
|
||||
});
|
||||
|
||||
it("wraps the per-call grep the real bridge factory builds", async () => {
|
||||
// The reviewed bypass was in the factory the session hands the bridge,
|
||||
// not in the bridge: a raw `new GrepTool(...)` there skips the approval
|
||||
|
||||
Reference in New Issue
Block a user