Skip to content

Commit a36b8f8

Browse files
committed
CR feedback
Signed-off-by: Hubert Zub <hubert.zub@databricks.com>
1 parent 3c23285 commit a36b8f8

4 files changed

Lines changed: 136 additions & 7 deletions

File tree

integrations/appkit-agent/src/agent-plugin/agent.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -239,9 +239,12 @@ export class AgentPlugin extends Plugin<IAgentConfig> {
239239
if (!this.agentImpl) {
240240
throw new Error("AgentPlugin not initialized");
241241
}
242-
const lastUser = [...messages].reverse().find((m) => m.role === "user");
243-
const input = lastUser?.content ?? "";
244-
const chatHistory = messages.slice(0, -1);
242+
const lastUserIdx = messages.findLastIndex((m) => m.role === "user");
243+
if (lastUserIdx < 0) {
244+
throw new Error("No user message found in input");
245+
}
246+
const input = messages[lastUserIdx].content;
247+
const chatHistory = messages.slice(0, lastUserIdx);
245248
const items = await this.agentImpl.invoke({
246249
input,
247250
chat_history: chatHistory,
@@ -258,9 +261,12 @@ export class AgentPlugin extends Plugin<IAgentConfig> {
258261
if (!this.agentImpl) {
259262
throw new Error("AgentPlugin not initialized");
260263
}
261-
const lastUser = [...messages].reverse().find((m) => m.role === "user");
262-
const input = lastUser?.content ?? "";
263-
const chatHistory = messages.slice(0, -1);
264+
const lastUserIdx = messages.findLastIndex((m) => m.role === "user");
265+
if (lastUserIdx < 0) {
266+
throw new Error("No user message found in input");
267+
}
268+
const input = messages[lastUserIdx].content;
269+
const chatHistory = messages.slice(0, lastUserIdx);
264270
yield* this.agentImpl.stream({
265271
input,
266272
chat_history: chatHistory,

integrations/appkit-agent/src/agent-plugin/invoke-handler.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,10 @@ export function createInvokeHandler(
123123
userInput = lastUserMessage.content as string;
124124
}
125125

126-
const chatHistory = input.slice(0, -1).map(flattenHistoryItem);
126+
const lastUserIdx = input.findLastIndex(
127+
(msg: any) => msg.role === "user",
128+
);
129+
const chatHistory = input.slice(0, lastUserIdx).map(flattenHistoryItem);
127130

128131
const agentParams = { input: userInput, chat_history: chatHistory };
129132
const agent = getAgent();

integrations/appkit-agent/src/tests/agent-plugin/agent.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,37 @@ describe("AgentPlugin", () => {
164164
expect(completedEvent).toBeDefined();
165165
});
166166

167+
test("invoke history contains only messages before the last user message", async () => {
168+
const spyAgent = {
169+
invoke: vi.fn().mockResolvedValue([
170+
{
171+
id: "msg_1",
172+
type: "message",
173+
role: "assistant",
174+
status: "completed",
175+
content: [{ type: "output_text", text: "ok", annotations: [] }],
176+
},
177+
]),
178+
stream: vi.fn(),
179+
};
180+
const config: IAgentConfig = { agentInstance: spyAgent as any };
181+
const plugin = new AgentPlugin(config);
182+
await plugin.setup();
183+
184+
await plugin.exports().invoke([
185+
{ role: "user", content: "first" },
186+
{ role: "user", content: "second" },
187+
{ role: "assistant", content: "interrupted" },
188+
]);
189+
190+
expect(spyAgent.invoke).toHaveBeenCalledWith(
191+
expect.objectContaining({
192+
input: "second",
193+
chat_history: [{ role: "user", content: "first" }],
194+
}),
195+
);
196+
});
197+
167198
test("throws when not initialized", async () => {
168199
const config: IAgentConfig = { agentInstance: new StubAgent() };
169200
const plugin = new AgentPlugin(config);
@@ -174,6 +205,29 @@ describe("AgentPlugin", () => {
174205
});
175206
});
176207

208+
describe("abortActiveOperations()", () => {
209+
function injectMcpClient(
210+
plugin: AgentPlugin,
211+
close: ReturnType<typeof vi.fn>,
212+
) {
213+
Reflect.set(plugin, "mcpClient", { getTools: vi.fn(), close });
214+
}
215+
216+
test("closes MCP client when present", async () => {
217+
const stub = new StubAgent();
218+
const config: IAgentConfig = { agentInstance: stub };
219+
const plugin = new AgentPlugin(config);
220+
await plugin.setup();
221+
222+
const closeFn = vi.fn().mockResolvedValue(undefined);
223+
injectMcpClient(plugin, closeFn);
224+
225+
await plugin.abortActiveOperations();
226+
227+
expect(closeFn).toHaveBeenCalledOnce();
228+
});
229+
});
230+
177231
describe("addTools()", () => {
178232
test("throws when using agentInstance mode", async () => {
179233
const stub = new StubAgent();

integrations/appkit-agent/src/tests/agent-plugin/invoke-handler.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,48 @@ describe("createInvokeHandler", () => {
206206
);
207207
});
208208

209+
test("history contains only messages before the last user message", async () => {
210+
const spyAgent = {
211+
invoke: vi.fn().mockResolvedValue([
212+
{
213+
id: "msg_1",
214+
type: "message",
215+
role: "assistant",
216+
status: "completed",
217+
content: [
218+
{ type: "output_text", text: "response", annotations: [] },
219+
],
220+
},
221+
]),
222+
stream: vi.fn(),
223+
};
224+
const historyHandler = createInvokeHandler(() => spyAgent as any);
225+
226+
const req = makeReq({
227+
input: [
228+
{ role: "user", content: "First message" },
229+
{ role: "user", content: "Second message" },
230+
{ role: "assistant", content: "Interrupted reply" },
231+
],
232+
stream: false,
233+
});
234+
const res = makeRes();
235+
236+
await historyHandler(req, res, vi.fn());
237+
238+
expect(spyAgent.invoke).toHaveBeenCalledWith(
239+
expect.objectContaining({
240+
input: "Second message",
241+
chat_history: [
242+
expect.objectContaining({
243+
role: "user",
244+
content: "First message",
245+
}),
246+
],
247+
}),
248+
);
249+
});
250+
209251
test("handles function_call items in history", async () => {
210252
const spyAgent = {
211253
invoke: vi.fn().mockResolvedValue([
@@ -278,6 +320,30 @@ describe("createInvokeHandler", () => {
278320
);
279321
});
280322

323+
test("returns 500 when non-streaming agent invoke throws", async () => {
324+
const errorAgent = {
325+
invoke: vi.fn().mockRejectedValue(new Error("invoke exploded")),
326+
stream: vi.fn(),
327+
};
328+
const errorHandler = createInvokeHandler(() => errorAgent as any);
329+
330+
const req = makeReq({
331+
input: [{ role: "user", content: "boom" }],
332+
stream: false,
333+
});
334+
const res = makeRes();
335+
336+
await errorHandler(req, res, vi.fn());
337+
338+
expect(res.status).toHaveBeenCalledWith(500);
339+
expect(res.json).toHaveBeenCalledWith(
340+
expect.objectContaining({
341+
error: "Internal server error",
342+
message: "invoke exploded",
343+
}),
344+
);
345+
});
346+
281347
test("handles agent errors gracefully in streaming mode", async () => {
282348
const errorAgent = {
283349
invoke: vi.fn(),

0 commit comments

Comments
 (0)