Skip to content

Commit e7916b3

Browse files
authored
Fix three demo bugs: browser beside apps, doubled tool calls, vendor names in steps (#824)
* Leave a computer tool the caller offers to the caller, beside headless plugin tools In a conversation the browser offers the computer_* tools itself. When the Bot also held an app or research tool, the run was wrapped in HeadlessToolsMiddleware, which claimed every computer_* call and, holding no such tool, answered each one as unavailable for this headless turn. It now claims a computer_* call only when nobody else offered that tool, so an unattended turn still answers an unknown one with a refusal. * Test that a caller's computer tool is not answered beside a plugin tool * Note that a Bot with an app or web research can use its browser again * Count a stored tool call the live turn already holds as that message The stored conversation names a tool call's message differently from the live stream (assistant-call_1 where the stream said call_1), and mergeStoredMessages matched on message id only, so it inserted the stored copy as missing. Every tool call that came back from the store that way was drawn twice, which is most visible as two Connect cards for one refusal. A stored assistant message whose every call one live message already holds now anchors on that message instead of being added. * Test merging a stored tool call the live turn already holds * Note that an app a Bot could not use asks to be connected once * Name a brokered app's action without the broker or the app's own prefix Composio publishes every action as <APP>_<VERB>_<NOUN>, so a step read "Airtable list bases", and an Arcade app's key reached the screen as arcade-Gmail because only the composio- prefix was known. A brokered app's action now starts at the verb, "List bases", with the app named once beside it, and connectorName drops either broker's prefix. * Summarise a brokered app's steps as the app, not as Composio Airtable * Test naming brokered actions without any vendor prefix * Test that a brokered app is summarised as the app * Note that an app's steps are named for the app * Name the message a step's tool calls belong to, as the store does agent-langgraph sent TOOL_CALL_START with no parentMessageId, so the surface named each call's message after the call (call_1) while the conversation store kept the step as assistant-call_1. The surface then sent its copy back on the next turn and the store kept both, so every tool call of a finished turn was stored, and drawn, twice. Each step's calls now carry the store's name for their message, taken from the step's first call. * Test that a step's tool calls name their stored message * Say that a conversation no longer keeps a second copy of each tool call
1 parent 74051c1 commit e7916b3

11 files changed

Lines changed: 348 additions & 47 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,27 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
3535
request for an administrator to switch on a work app, grant it to a Bot, or connect a Shared app's
3636
team account is kept, with who answered it and when. It starts empty and nothing is backfilled.
3737

38+
### A Bot with an app or web research can use its browser in a conversation again
39+
40+
A Bot that held any app or research tool, such as Parallel Search, could not open a page, click or
41+
type in a conversation: every browser and computer step came back as unavailable, and the Bot
42+
fetched the page some other way instead. Its browser now works beside those tools, as it does for a
43+
Bot that holds none. Routines and other unattended turns are unchanged.
44+
45+
### An app a Bot could not use asks to be connected once
46+
47+
When a Bot reached for an app the person had not connected, such as Airtable, the conversation drew
48+
the Connect card twice, and still did after a reload. The built-in LangGraph Bot now names the message
49+
its tool calls belong to the way the conversation store does, so a conversation no longer keeps a
50+
second copy of every tool call, and a conversation that already has one draws each call once.
51+
52+
### An app's steps are named for the app, never for the service behind it
53+
54+
A conversation named a Composio app's work after the broker and the vendor's own prefix: "Used
55+
Composio Airtable", then "Airtable list bases". It now says "Used Airtable", and each step reads as
56+
the action, "List bases", with the app beside it. An Arcade app is named the same way, as the app
57+
rather than `arcade-Gmail`.
58+
3859
### Plainer words, and one word for each thing
3960

4061
OpenBot now says **Bot** everywhere a person reads it, where screens used to mix Bot, agent, coworker

‎agent-langgraph/src/stream.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,14 +143,17 @@ export async function streamRun(
143143
| undefined;
144144
// Prose and tool calls cannot interleave inside one message.
145145
closeText();
146+
let parentMessageId: string | undefined;
146147
for (const message of output?.messages ?? []) {
147148
const id = message.tool_call_id ?? "";
148149
const call = pending.get(id);
149150
if (!call) continue;
151+
parentMessageId ??= stepMessageId(id);
150152
send({
151153
type: "TOOL_CALL_START",
152154
toolCallId: id,
153155
toolCallName: call.name,
156+
parentMessageId,
154157
} as BaseEvent);
155158
send({
156159
type: "TOOL_CALL_ARGS",
@@ -183,11 +186,14 @@ export async function streamRun(
183186
* because an empty run is not an error by the protocol, nothing says so. No result is sent
184187
* with them; producing it is the surface's half, and it begins the next run holding it.
185188
*/
189+
let drainedParent: string | undefined;
186190
for (const [id, call] of pending) {
191+
drainedParent ??= stepMessageId(id);
187192
send({
188193
type: "TOOL_CALL_START",
189194
toolCallId: id,
190195
toolCallName: call.name,
196+
parentMessageId: drainedParent,
191197
} as BaseEvent);
192198
send({
193199
type: "TOOL_CALL_ARGS",
@@ -235,3 +241,12 @@ export async function streamRun(
235241
} as BaseEvent);
236242
}
237243
}
244+
245+
/**
246+
* The message a step's tool calls belong to, named the way the conversation store names it: for the
247+
* step's first call. Left unnamed, the surface named each call's message after the call itself, a
248+
* different id from the stored one, and sent its copy back on the next turn as a second message.
249+
*/
250+
function stepMessageId(firstToolCallId: string): string {
251+
return `assistant-${firstToolCallId}`;
252+
}

‎agent-langgraph/tests/stream.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,55 @@ describe("a run that does produce something is left alone", () => {
184184
false,
185185
);
186186
});
187+
188+
test("the calls of one step name the message the store keeps them in", async () => {
189+
// The store keeps a step's calls in one message named for the first call. Without a parent the
190+
// surface named each after itself, and sent that copy back next turn as a second message.
191+
const sent = await collect([
192+
{
193+
event: "on_chat_model_end",
194+
data: {
195+
output: {
196+
tool_calls: [
197+
{ id: "call_a", name: "list_bases", args: {} },
198+
{ id: "call_b", name: "get_schema", args: {} },
199+
],
200+
},
201+
},
202+
},
203+
{
204+
event: "on_chain_end",
205+
name: "tools",
206+
data: {
207+
output: {
208+
messages: [
209+
{ tool_call_id: "call_a", content: "bases" },
210+
{ tool_call_id: "call_b", content: "schema" },
211+
],
212+
},
213+
},
214+
},
215+
]);
216+
const starts = sent.filter((event) => event.type === "TOOL_CALL_START");
217+
expect(starts.map((event) => event.parentMessageId)).toEqual([
218+
"assistant-call_a",
219+
"assistant-call_a",
220+
]);
221+
});
222+
223+
test("a call the surface owns names its message the same way", async () => {
224+
const sent = await collect([
225+
{
226+
event: "on_chat_model_end",
227+
data: {
228+
output: {
229+
tool_calls: [{ id: "call_1", name: "computer_navigate", args: {} }],
230+
},
231+
},
232+
},
233+
]);
234+
expect(sent[0]).toMatchObject({ parentMessageId: "assistant-call_1" });
235+
});
187236
});
188237

189238
describe("message boundaries across a multi-turn run", () => {

‎app/src/components/channels/channel-chat.tsx‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,17 +124,43 @@ export function channelHistoryNotice({
124124
* A shorter read can still contain missing turns after unreadable rows are filtered out. Without a
125125
* following shared anchor, append the missing tail: the store cannot place it among local-only rows.
126126
* Return the original array when nothing was added so refreshes can wait for the store to catch up.
127+
*
128+
* Exported for its tests.
127129
*/
128-
function mergeStoredMessages(local: Message[], stored: Message[]): Message[] {
130+
export function mergeStoredMessages(
131+
local: Message[],
132+
stored: Message[],
133+
): Message[] {
129134
const localIds = new Set(local.map((message) => message.id));
135+
/*
136+
* The store can name a tool call's message differently from the live turn — `assistant-call_1`
137+
* where the stream said `call_1` — so a stored message whose every call the live turn already
138+
* holds is that live message, not a missing one. Added as missing, its card was drawn twice.
139+
*/
140+
const localByCall = new Map<string, string>();
141+
for (const message of local) {
142+
if (message.role !== "assistant") continue;
143+
for (const call of message.toolCalls ?? [])
144+
localByCall.set(call.id, message.id);
145+
}
146+
const heldAs = (message: Message): string | undefined => {
147+
if (localIds.has(message.id)) return message.id;
148+
if (message.role !== "assistant" || !message.toolCalls?.length) return;
149+
const holders = new Set(
150+
message.toolCalls.map((call) => localByCall.get(call.id)),
151+
);
152+
const [holder] = holders;
153+
return holders.size === 1 ? holder : undefined;
154+
};
130155
const seenStored = new Set<string>();
131156
const before = new Map<string, Message[]>();
132157
let pending: Message[] = [];
133158
for (const message of stored) {
134159
if (seenStored.has(message.id)) continue;
135160
seenStored.add(message.id);
136-
if (localIds.has(message.id)) {
137-
if (pending.length > 0) before.set(message.id, pending);
161+
const held = heldAs(message);
162+
if (held !== undefined) {
163+
if (pending.length > 0) before.set(held, pending);
138164
pending = [];
139165
} else {
140166
pending.push(message);

‎app/src/components/channels/tool-activity.tsx‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@ import { useCallback, useContext, useMemo, useSyncExternalStore } from "react";
1010
import { Streamdown } from "streamdown";
1111
import { PendingApprovals } from "@/components/approvals/inline-approval";
1212
import { markdownComponents } from "@/lib/markdown";
13-
import { humanise, ownToolLabel, readToolName } from "@/lib/plugins/tool-name";
13+
import {
14+
connectorName,
15+
humanise,
16+
ownToolLabel,
17+
readToolName,
18+
} from "@/lib/plugins/tool-name";
1419
import {
1520
asText,
1621
forDisplay,
@@ -196,15 +201,6 @@ function hostOf(url: string): string {
196201
}
197202
}
198203

199-
/** "google-drive" as "Google Drive". */
200-
function connectorName(key: string): string {
201-
return key
202-
.split(/[-_]/)
203-
.filter(Boolean)
204-
.map((word) => word[0]?.toUpperCase() + word.slice(1))
205-
.join(" ");
206-
}
207-
208204
/** A value from a tool's arguments, short enough for one line. */
209205
function inputValue(value: unknown): string {
210206
const text = typeof value === "string" ? value : JSON.stringify(value);

‎app/src/lib/plugins/tool-name.ts‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,22 @@ export function readToolName(name: string, running = false): ToolName {
3939
*/
4040
const role = researchRoleOfTool(server, tool);
4141
if (role) return { label: RESEARCH_ROLE_LABELS[role] };
42+
/*
43+
* A BROKERED APP IS NAMED ONCE, AS THE APP, BESIDE THE ACTION. Composio publishes
44+
* `<APP>_<VERB>_<NOUN>`, so its app leads every action and reads as a vendor's prefix
45+
* ("Airtable list bases"); Arcade has already dropped its toolkit from the name. Either way the
46+
* action starts at the verb and the app is said once, in its own name.
47+
*/
48+
if (broker) {
49+
const appWords = wordsOf(broker);
50+
const toolWords = wordsOf(tool);
51+
const leads = appWords.every((word, index) => toolWords[index] === word);
52+
const action = leads ? toolWords.slice(appWords.length) : toolWords;
53+
return {
54+
label: action.length > 0 ? humanise(action.join(" ")) : humanise(tool),
55+
detail: connectorName(server),
56+
};
57+
}
4258
const label = humanise(tool);
4359

4460
/*
@@ -64,17 +80,20 @@ export function readToolName(name: string, running = false): ToolName {
6480
* a known limit of this rule, not a bug to chase with a word list.
6581
*/
6682
const labelWords = label.toLowerCase().split(" ").map(singular);
67-
/*
68-
* A BROKERED APP IS SEARCHED FOR IN THE WHOLE LABEL, VERB INCLUDED. The verb exclusion above
69-
* exists for a server whose name collides with a verb by accident, and a broker's apps cannot
70-
* produce that collision: Composio publishes `<APP>_<VERB>_<NOUN>`, so the app always leads and
71-
* is never the verb. Excluding the first word here would put "googlecalendar" beside a label
72-
* that opens with the word.
73-
*/
74-
const wordsActedOn = broker ? labelWords : labelWords.slice(1);
75-
const serverWords = wordsOf(broker ?? server).map(singular);
83+
const wordsActedOn = labelWords.slice(1);
84+
const serverWords = wordsOf(server).map(singular);
7685
const named = containsPhrase(wordsActedOn, serverWords);
77-
return named ? { label } : { label, detail: broker ?? server };
86+
return named ? { label } : { label, detail: server };
87+
}
88+
89+
/**
90+
* The connector a server key names, the way a person would say it: "google-drive" as "Google
91+
* Drive", and a broker's "composio-airtable" or "arcade-Gmail" as the app alone.
92+
*/
93+
export function connectorName(server: string): string {
94+
return wordsOf(brokeredApp(server) ?? server)
95+
.map((word) => word[0]?.toUpperCase() + word.slice(1))
96+
.join(" ");
7897
}
7998

8099
/**
@@ -102,7 +121,7 @@ export function ownToolLabel(name: string, running = false): string | null {
102121
/**
103122
* The app behind a brokered server key, or null for a server this deployment dials itself.
104123
*
105-
* A brokered server is keyed `<broker>-<app>` — `composio-gmail` — because the broker is how the
124+
* A brokered server is keyed `<broker>-<app>` — `composio-gmail`, `arcade-Gmail` — because the broker is how the
106125
* credential is held and two brokers could both offer Gmail. Neither of those is the reader's
107126
* business: they asked their Bot about their email, not about which company holds the grant. This
108127
* is the same argument the research relabelling above makes, and the answer is the same — which
@@ -111,12 +130,17 @@ export function ownToolLabel(name: string, running = false): string | null {
111130
* Only the prefix is removed, never the app. "Gmail" is what was acted upon and stays legible.
112131
*/
113132
function brokeredApp(server: string): string | null {
114-
const prefix = "composio-";
115-
if (!server.startsWith(prefix)) return null;
116-
const app = server.slice(prefix.length);
117-
return app.length > 0 ? app : null;
133+
for (const prefix of BROKER_PREFIXES) {
134+
if (!server.startsWith(prefix)) continue;
135+
const app = server.slice(prefix.length);
136+
return app.length > 0 ? app : null;
137+
}
138+
return null;
118139
}
119140

141+
/** The brokers whose apps this deployment keys `<broker>-<app>`. See `server/src/plugins/access.ts`. */
142+
const BROKER_PREFIXES = ["composio-", "arcade-"] as const;
143+
120144
/**
121145
* `routines` and `routine` are the same word for this purpose.
122146
*
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { describe, expect, test } from "bun:test";
2+
import type { Message } from "@ag-ui/core";
3+
import { mergeStoredMessages } from "../src/components/channels/channel-chat";
4+
5+
const call = (id: string) => ({
6+
id,
7+
type: "function" as const,
8+
function: {
9+
name: "mcp__composio-airtable__AIRTABLE_LIST_BASES",
10+
arguments: "{}",
11+
},
12+
});
13+
14+
const question: Message = {
15+
id: "user-1",
16+
role: "user",
17+
content: "What bases do I have?",
18+
};
19+
const result: Message = {
20+
id: "result-1",
21+
role: "tool",
22+
toolCallId: "call_1",
23+
content: "Refused. Connect Airtable.",
24+
};
25+
const answer: Message = {
26+
id: "answer-1",
27+
role: "assistant",
28+
content: "Airtable is not connected.",
29+
};
30+
31+
describe("mergeStoredMessages", () => {
32+
test("a stored tool call the live turn already holds under another id is not added again", () => {
33+
const local: Message[] = [
34+
question,
35+
{ id: "call_1", role: "assistant", toolCalls: [call("call_1")] },
36+
result,
37+
answer,
38+
];
39+
const stored: Message[] = [
40+
question,
41+
{
42+
id: "assistant-call_1",
43+
role: "assistant",
44+
toolCalls: [call("call_1")],
45+
},
46+
result,
47+
answer,
48+
];
49+
expect(mergeStoredMessages(local, stored)).toBe(local);
50+
});
51+
52+
test("a missing stored message still lands before the live message holding the same call", () => {
53+
const local: Message[] = [
54+
{ id: "call_1", role: "assistant", toolCalls: [call("call_1")] },
55+
result,
56+
];
57+
const stored: Message[] = [
58+
question,
59+
{
60+
id: "assistant-call_1",
61+
role: "assistant",
62+
toolCalls: [call("call_1")],
63+
},
64+
result,
65+
];
66+
expect(mergeStoredMessages(local, stored).map((m) => m.id)).toEqual([
67+
"user-1",
68+
"call_1",
69+
"result-1",
70+
]);
71+
});
72+
73+
test("a stored tool call the live turn does not hold is still added", () => {
74+
const local: Message[] = [question];
75+
const stored: Message[] = [
76+
question,
77+
{
78+
id: "assistant-call_2",
79+
role: "assistant",
80+
toolCalls: [call("call_2")],
81+
},
82+
];
83+
expect(mergeStoredMessages(local, stored).map((m) => m.id)).toEqual([
84+
"user-1",
85+
"assistant-call_2",
86+
]);
87+
});
88+
});

0 commit comments

Comments
 (0)