Skip to content

Commit aa2ff60

Browse files
committed
Harden proxy runs and CLI continuity
1 parent 0048aad commit aa2ff60

29 files changed

Lines changed: 814 additions & 89 deletions

src/agent/agent.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { UserQueryStore } from "../user-queries/store.js";
3333
import { type ResolvedConfig, resolveConfig } from "./config.js";
3434
import { type Effort, resolveEffort } from "./effort.js";
3535
import { buildProjectFilesAddendum } from "./project-files.js";
36+
import { streamProxySafely } from "./safe-stream.js";
3637
import { buildSystemPrompt } from "./system-prompt.js";
3738

3839
const WRITE_TOOL_NAMES: ReadonlySet<string> = new Set(["write_file", "edit_file", "multi_edit", "notebook_edit"]);
@@ -99,6 +100,8 @@ export interface CreateAgentOptions {
99100
systemPromptAddendum?: string;
100101
/** Reuse an existing task-list id while rebuilding an in-memory agent. */
101102
taskListId?: string;
103+
/** Persist settled turns to the resumable session store. Default true. */
104+
persistSession?: boolean;
102105
}
103106

104107
export interface AgentBundle {
@@ -253,6 +256,10 @@ export function createAgent(opts: CreateAgentOptions = {}): AgentBundle {
253256
"Use exit_plan_mode after presenting your plan to regain write access.",
254257
};
255258
}
259+
const preview = permissions.preview(toolName, args);
260+
if (preview.decision === "block") {
261+
return { block: true, reason: preview.reason ?? "Blocked by permission policy." };
262+
}
256263
const decision = await permissions.evaluate(toolName, args);
257264
if (decision === "block") {
258265
return { block: true, reason: "Permission denied by user." };
@@ -337,6 +344,7 @@ export function createAgent(opts: CreateAgentOptions = {}): AgentBundle {
337344
...(thinkingLevel && { thinkingLevel: thinkingLevel as Effort }),
338345
},
339346
getApiKey,
347+
...(source === "proxy" && { streamFn: streamProxySafely }),
340348
beforeToolCall: (ctx, signal) => guardToolCall(ctx.toolCall.name, ctx.args, signal),
341349
afterToolCall: async (ctx, signal) => {
342350
await dispatchPostToolHooks(
@@ -384,7 +392,8 @@ export function createAgent(opts: CreateAgentOptions = {}): AgentBundle {
384392
messages: opts.initialMessages ?? resumed?.messages ?? [],
385393
...(effort && { thinkingLevel: effort }),
386394
},
387-
getApiKey: () => apiKey,
395+
getApiKey,
396+
...(source === "proxy" && { streamFn: streamProxySafely }),
388397
transformContext: async (messages, signal) => {
389398
if (!compaction.needsCompaction(messages)) return withRelevantMemoryReminder(memory, messages);
390399

@@ -482,6 +491,7 @@ export function createAgent(opts: CreateAgentOptions = {}): AgentBundle {
482491
// Persist after every agent_end so a crash mid-session doesn't lose work.
483492
agent.subscribe((event) => {
484493
if (event.type !== "agent_end") return;
494+
if (opts.persistSession === false) return;
485495
try {
486496
const messages = event.messages.length > 0 ? event.messages : (resumed?.messages ?? []);
487497
if (messages.length === 0) return;

src/agent/router.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,17 @@ describe("routeUserInput", () => {
1818
await expect(routeUserInput(glue, "fix the build", { hasHistory: true })).resolves.toEqual({ kind: "agent" });
1919
});
2020

21-
it("returns 'plan' when intent classifies to plan", async () => {
21+
it("keeps complex actionable work with the tool-using agent", async () => {
2222
const glue = mockGlue({ intent: "plan" });
2323
await expect(
2424
routeUserInput(glue, "rewrite the worker as a state machine", { hasHistory: false }),
25+
).resolves.toEqual({ kind: "agent" });
26+
});
27+
28+
it("uses the reviewable plan flow when the user explicitly requests it", async () => {
29+
const glue = mockGlue({ intent: "plan" });
30+
await expect(
31+
routeUserInput(glue, "Make an implementation plan before coding", { hasHistory: false }),
2532
).resolves.toEqual({ kind: "plan" });
2633
});
2734

src/agent/safe-stream.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { type AssistantMessage, createAssistantMessageEventStream } from "@earendil-works/pi-ai";
2+
import { describe, expect, it } from "vitest";
3+
import {
4+
DEFAULT_PROXY_RESPONSE_TIMEOUT_MS,
5+
proxyResponseTimeoutMs,
6+
sanitizeAssistantMessage,
7+
sanitizeAssistantStream,
8+
stripDsmlProtocol,
9+
} from "./safe-stream.js";
10+
11+
function message(content: AssistantMessage["content"]): AssistantMessage {
12+
return {
13+
role: "assistant",
14+
content,
15+
api: "openai-completions",
16+
provider: "openai",
17+
model: "test",
18+
usage: {
19+
input: 0,
20+
output: 0,
21+
cacheRead: 0,
22+
cacheWrite: 0,
23+
totalTokens: 0,
24+
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
25+
},
26+
stopReason: "toolUse",
27+
timestamp: 1,
28+
};
29+
}
30+
31+
describe("stripDsmlProtocol", () => {
32+
it("removes full and partially streamed protocol markers", () => {
33+
expect(stripDsmlProtocol("Working on it.\n<|DSML|function_calls>bad")).toBe("Working on it.");
34+
expect(stripDsmlProtocol("Working on it.\n<|DSM")).toBe("Working on it.");
35+
});
36+
});
37+
38+
describe("proxyResponseTimeoutMs", () => {
39+
it("uses a bounded default and clamps explicit values", () => {
40+
expect(proxyResponseTimeoutMs(undefined)).toBe(DEFAULT_PROXY_RESPONSE_TIMEOUT_MS);
41+
expect(proxyResponseTimeoutMs("not-a-number")).toBe(DEFAULT_PROXY_RESPONSE_TIMEOUT_MS);
42+
expect(proxyResponseTimeoutMs("100")).toBe(10_000);
43+
expect(proxyResponseTimeoutMs("9999999")).toBe(600_000);
44+
expect(proxyResponseTimeoutMs("45000")).toBe(45_000);
45+
});
46+
});
47+
48+
describe("sanitizeAssistantMessage", () => {
49+
it("drops textual protocol and duplicate semantic tool calls", () => {
50+
const clean = sanitizeAssistantMessage(
51+
message([
52+
{ type: "text", text: "I will inspect it.\n<|DSML|function_calls>duplicate" },
53+
{ type: "toolCall", id: "one", name: "read_file", arguments: { path: "a.ts", line: 1 } },
54+
{ type: "toolCall", id: "two", name: "read_file", arguments: { line: 1, path: "a.ts" } },
55+
{ type: "toolCall", id: "three", name: "read_file", arguments: { path: "b.ts" } },
56+
]),
57+
);
58+
59+
expect(clean.content).toEqual([
60+
{ type: "text", text: "I will inspect it." },
61+
{ type: "toolCall", id: "one", name: "read_file", arguments: { path: "a.ts", line: 1 } },
62+
{ type: "toolCall", id: "three", name: "read_file", arguments: { path: "b.ts" } },
63+
]);
64+
});
65+
66+
it("makes the sanitized final message the stream result used by the executor", async () => {
67+
const dirty = message([
68+
{ type: "toolCall", id: "one", name: "shell", arguments: { command: "npm test" } },
69+
{ type: "toolCall", id: "two", name: "shell", arguments: { command: "npm test" } },
70+
]);
71+
const upstream = createAssistantMessageEventStream();
72+
const safe = sanitizeAssistantStream(upstream);
73+
upstream.push({ type: "start", partial: dirty });
74+
upstream.push({ type: "done", reason: "toolUse", message: dirty });
75+
for await (const _event of safe) {
76+
// Drain the wrapped stream exactly as pi-agent-core does.
77+
}
78+
expect((await safe.result()).content).toHaveLength(1);
79+
});
80+
});

src/agent/safe-stream.ts

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
import {
2+
type AssistantMessage,
3+
type AssistantMessageEvent,
4+
type AssistantMessageEventStream,
5+
createAssistantMessageEventStream,
6+
streamSimple,
7+
} from "@earendil-works/pi-ai";
8+
9+
const DSML_MARKER = "<|dsml|";
10+
export const DEFAULT_PROXY_RESPONSE_TIMEOUT_MS = 90_000;
11+
12+
/** Remove textual proxy protocol that must never be rendered or re-parsed. */
13+
export function stripDsmlProtocol(text: string): string {
14+
const normalized = text.toLowerCase().replaceAll("|", "|");
15+
const fullMarker = normalized.indexOf(DSML_MARKER);
16+
if (fullMarker >= 0) return text.slice(0, fullMarker).trimEnd();
17+
18+
// Streaming can split the marker across chunks. Hide a trailing partial
19+
// marker as soon as it starts instead of flashing protocol in the TUI.
20+
for (let i = normalized.lastIndexOf("<"); i >= 0; i = normalized.lastIndexOf("<", i - 1)) {
21+
const suffix = normalized.slice(i);
22+
if (DSML_MARKER.startsWith(suffix)) return text.slice(0, i).trimEnd();
23+
}
24+
return text;
25+
}
26+
27+
/** Normalize a proxy response before pi-agent-core can execute its tools. */
28+
export function sanitizeAssistantMessage(message: AssistantMessage): AssistantMessage {
29+
const seenToolCalls = new Set<string>();
30+
const content: AssistantMessage["content"] = [];
31+
for (const block of message.content) {
32+
if (block.type === "text") {
33+
const text = stripDsmlProtocol(block.text);
34+
if (text) content.push({ ...block, text });
35+
continue;
36+
}
37+
if (block.type === "toolCall") {
38+
const fingerprint = `${block.name}:${stableJson(block.arguments)}`;
39+
if (seenToolCalls.has(fingerprint)) continue;
40+
seenToolCalls.add(fingerprint);
41+
}
42+
content.push(block);
43+
}
44+
return { ...message, content };
45+
}
46+
47+
/**
48+
* Proxy models occasionally return both native tool calls and a textual DSML
49+
* copy, and have also repeated an identical native call in one response. This
50+
* wrapper cleans every partial snapshot plus the final message so the UI and
51+
* executor see the same safe response.
52+
*/
53+
export function streamProxySafely(...args: Parameters<typeof streamSimple>): ReturnType<typeof streamSimple> {
54+
const [model, context, options] = args;
55+
const output = createAssistantMessageEventStream();
56+
const controller = new AbortController();
57+
const parentSignal = options?.signal;
58+
let latest = emptyAssistantMessage(model);
59+
let timedOut = false;
60+
61+
const abortFromParent = () => controller.abort(parentSignal?.reason);
62+
if (parentSignal?.aborted) abortFromParent();
63+
else parentSignal?.addEventListener("abort", abortFromParent, { once: true });
64+
65+
const timeoutMs = proxyResponseTimeoutMs();
66+
const timer = setTimeout(() => {
67+
timedOut = true;
68+
controller.abort();
69+
output.push({
70+
type: "error",
71+
reason: "error",
72+
error: failedAssistantMessage(
73+
latest,
74+
"error",
75+
`Model response exceeded ${Math.round(timeoutMs / 1000)}s and was stopped. Retry with a narrower request or start a fresh session.`,
76+
),
77+
});
78+
}, timeoutMs);
79+
timer.unref?.();
80+
81+
const upstream = streamSimple(model, context, { ...options, signal: controller.signal });
82+
void (async () => {
83+
try {
84+
for await (const event of upstream) {
85+
const sanitized = sanitizeEvent(event);
86+
latest = messageFromEvent(sanitized);
87+
output.push(sanitized);
88+
}
89+
} catch (error) {
90+
if (timedOut) return;
91+
output.push({
92+
type: "error",
93+
reason: controller.signal.aborted ? "aborted" : "error",
94+
error: failedAssistantMessage(
95+
latest,
96+
controller.signal.aborted ? "aborted" : "error",
97+
error instanceof Error ? error.message : String(error),
98+
),
99+
});
100+
}
101+
})();
102+
void output.result().finally(() => {
103+
clearTimeout(timer);
104+
parentSignal?.removeEventListener("abort", abortFromParent);
105+
});
106+
return output;
107+
}
108+
109+
export function sanitizeAssistantStream(upstream: AssistantMessageEventStream): AssistantMessageEventStream {
110+
const output = createAssistantMessageEventStream();
111+
void (async () => {
112+
for await (const event of upstream) output.push(sanitizeEvent(event));
113+
})();
114+
return output;
115+
}
116+
117+
function sanitizeEvent(event: AssistantMessageEvent): AssistantMessageEvent {
118+
if (event.type === "done") return { ...event, message: sanitizeAssistantMessage(event.message) };
119+
if (event.type === "error") return { ...event, error: sanitizeAssistantMessage(event.error) };
120+
return { ...event, partial: sanitizeAssistantMessage(event.partial) };
121+
}
122+
123+
export function proxyResponseTimeoutMs(value = process.env.CODEBASE_RESPONSE_TIMEOUT_MS): number {
124+
if (value === undefined || value.trim() === "") return DEFAULT_PROXY_RESPONSE_TIMEOUT_MS;
125+
const parsed = Number(value);
126+
if (!Number.isFinite(parsed)) return DEFAULT_PROXY_RESPONSE_TIMEOUT_MS;
127+
return Math.max(10_000, Math.min(600_000, Math.round(parsed)));
128+
}
129+
130+
function messageFromEvent(event: AssistantMessageEvent): AssistantMessage {
131+
if (event.type === "done") return event.message;
132+
if (event.type === "error") return event.error;
133+
return event.partial;
134+
}
135+
136+
function emptyAssistantMessage(model: Parameters<typeof streamSimple>[0]): AssistantMessage {
137+
return {
138+
role: "assistant",
139+
content: [],
140+
api: model.api,
141+
provider: model.provider,
142+
model: model.id,
143+
usage: {
144+
input: 0,
145+
output: 0,
146+
cacheRead: 0,
147+
cacheWrite: 0,
148+
totalTokens: 0,
149+
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
150+
},
151+
stopReason: "stop",
152+
timestamp: Date.now(),
153+
};
154+
}
155+
156+
function failedAssistantMessage(
157+
message: AssistantMessage,
158+
stopReason: "aborted" | "error",
159+
errorMessage: string,
160+
): AssistantMessage {
161+
const sanitized = sanitizeAssistantMessage(message);
162+
return {
163+
...sanitized,
164+
// A partially streamed tool call was never executed. Do not persist or
165+
// render it as a completed action after timeout/abort.
166+
content: sanitized.content.filter((block) => block.type !== "toolCall"),
167+
stopReason,
168+
errorMessage,
169+
};
170+
}
171+
172+
function stableJson(value: unknown): string {
173+
if (Array.isArray(value)) return `[${value.map(stableJson).join(",")}]`;
174+
if (value && typeof value === "object") {
175+
return `{${Object.entries(value as Record<string, unknown>)
176+
.sort(([a], [b]) => a.localeCompare(b))
177+
.map(([key, child]) => `${JSON.stringify(key)}:${stableJson(child)}`)
178+
.join(",")}}`;
179+
}
180+
return JSON.stringify(value) ?? String(value);
181+
}

src/agent/system-prompt.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ export function buildSystemPrompt(opts: BuildSystemPromptOptions = {}): string {
5454
lines.push(
5555
"Issue independent tool calls together in a single response. If you need to read three files whose contents don't depend on each other, request all three reads at once instead of one per turn — sequential reads are the slow path and should only happen when a later call genuinely needs an earlier call's result. The same applies to greps, glob queries, and any read-only investigation.",
5656
);
57+
lines.push(
58+
"Before editing or overwriting an existing file, inspect that file in the current session. Never guess a path or its current contents; read the target first so the write tools have a fresh snapshot.",
59+
);
5760
lines.push(
5861
"When a task fans out cleanly — multi-file audits, security reviews, broad codebase exploration — prefer dispatching subagents via dispatch_agent so each stream runs in parallel and their context stays out of your main loop. Don't also do the same searches yourself; that wastes turns and doubles the noise.",
5962
);

src/agent/visible-messages.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import type { AgentMessage } from "@earendil-works/pi-agent-core";
2+
import { describe, expect, it } from "vitest";
3+
import { stripRuntimeMarkup, visibleMessages } from "./visible-messages.js";
4+
5+
describe("stripRuntimeMarkup", () => {
6+
it("removes system reminders and DSML framing", () => {
7+
expect(
8+
stripRuntimeMarkup("<system-reminder>internal receipt rules</system-reminder>Actual prompt<|DSML|bad"),
9+
).toBe("Actual prompt");
10+
});
11+
});
12+
13+
describe("visibleMessages", () => {
14+
it("hides thinking plus duplicate calls and their results", () => {
15+
const messages = [
16+
{
17+
role: "assistant",
18+
content: [
19+
{ type: "thinking", thinking: "private repair narration" },
20+
{ type: "toolCall", id: "a", name: "shell", arguments: { command: "npm test" } },
21+
{ type: "toolCall", id: "b", name: "shell", arguments: { command: "npm test" } },
22+
],
23+
},
24+
{ role: "toolResult", toolCallId: "a", toolName: "shell", content: [], isError: false },
25+
{ role: "toolResult", toolCallId: "b", toolName: "shell", content: [], isError: false },
26+
] as AgentMessage[];
27+
28+
const visible = visibleMessages(messages);
29+
expect(visible).toHaveLength(2);
30+
expect(visible[0]?.role).toBe("assistant");
31+
expect(visible[1]).toMatchObject({ role: "toolResult", toolCallId: "a" });
32+
});
33+
});

0 commit comments

Comments
 (0)