Skip to content

Commit ba84b15

Browse files
committed
fix(chat): adapt to main's coding cards, check-ins and shared tool names
Main moved tool names to shared/ and split out appToolName for approval words (#929), added Claude Code's coding steps (#940), and gave user preferences a check-ins switch. The chat branch now meets them: - appToolName carries the call's state, so the person's own mail and calendar keep their running, done and plain words through it. approvalWords asks for the app's own action, so a task's waiting line still reads "send message on Mail" as main's does. - A coding step carries plain, declined, alreadyWaiting, stopped and notYet, and a run of reads and searches counts every one that finished, as Claude Code counts them, with its issues beside it. A run where none worked reads "Listed 2 directories, didn't work". - Imports of the old app tool-name path point at the shared one, and two sidebar tests seed the proactive preference. - Main's approval-words and task-approval-buttons tests assert the redesigned card: titled with the action in its step's words, the app by its name, Decline and Allow once, and no Always allow here on a check-in task's approval.
1 parent 96dd723 commit ba84b15

10 files changed

Lines changed: 75 additions & 39 deletions

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,8 @@ export function stepDetails(step: ToolItem): StepDetails {
385385
refused: false,
386386
declined: false,
387387
alreadyWaiting: false,
388+
stopped: false,
389+
notYet: false,
388390
pages: 0,
389391
inputs: [],
390392
...(output ? { output, verbatim: true } : {}),
@@ -514,13 +516,19 @@ export function summarise(steps: readonly StepDetails[]): string {
514516
const done = steps.filter(isDone);
515517
const of = (kind: StepKind) => done.filter((step) => step.kind === kind);
516518
const phrases: string[] = [];
517-
// Claude Code's own words for a run of reads and searches.
518-
const files = of("file").length;
519+
/*
520+
* Claude Code's own words for a run of reads and searches, which count every one that finished,
521+
* as Claude Code counts them: one that didn't work is still counted here, and the issues beside
522+
* the line say how many didn't.
523+
*/
524+
const ran = (kind: StepKind) =>
525+
steps.filter((step) => step.kind === kind && !step.pending);
526+
const files = ran("file").length;
519527
if (files) phrases.push(`read ${plural(files, "file", "files")}`);
520-
const patterns = of("pattern").length;
528+
const patterns = ran("pattern").length;
521529
if (patterns)
522530
phrases.push(`searched for ${plural(patterns, "pattern", "patterns")}`);
523-
const directories = of("directory").length;
531+
const directories = ran("directory").length;
524532
if (directories)
525533
phrases.push(`listed ${plural(directories, "directory", "directories")}`);
526534
if (of("search").length) phrases.push("searched the web");

‎app/src/lib/approval-view.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import type { ApprovalRequest } from "./approvals";
22
import { BROWSER_STEP_PLAIN } from "./computers/step-names";
3-
import { connectorName, humanise, readToolName } from "./plugins/tool-name";
3+
import {
4+
connectorName,
5+
humanise,
6+
readToolName,
7+
} from "@openbot/shared/tool-name";
48

59
/**
610
* How an approval request reads on its card: a title in plain words, the app it is for, the fields a

‎app/src/lib/channels/step.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
ownToolLabel,
1111
personToolLabel,
1212
readToolName,
13-
} from "@/lib/plugins/tool-name";
13+
} from "@openbot/shared/tool-name";
1414

1515
/**
1616
* What the line under a conversation's name may say about the step its Bot is on, where the step's

‎app/tests/approval-words.test.tsx‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import { settleReactWork } from "./settle-react-work";
1010
/*
1111
* What an approval says it is waiting on. An app's tool is asked about as `<app>/<tool>` on the app,
1212
* and the card in the conversation once read "Waiting for your approval: requests/get request on
13-
* requests", the app's key twice and the tool's raw name, where a computer action reads "click on
14-
* example.com". Both places a person decides one in the browser: the card in the conversation, and
13+
* requests", the app's key twice and the tool's raw name. The card is titled with the action in the
14+
* words its step uses and names the app by its name. Both places a person decides one in the browser: the card in the conversation, and
1515
* the row on the Bot's page.
1616
*/
1717

@@ -111,27 +111,36 @@ test("in the conversation, an app's approval names the action in words and the a
111111
<InlineApproval toolCallId="call-computer" fallback={null} />
112112
</>,
113113
);
114-
const cards = await view.findAllByRole("region", {
115-
name: "Waiting for your approval",
116-
});
114+
await view.findAllByRole("button", { name: "Allow once" });
115+
// Each card is named by its title, the action in the words its step uses.
116+
const cards = view.getAllByRole("region");
117117
expect(cards.map((card) => card.querySelector("p")?.textContent)).toEqual([
118-
"Waiting for your approval: get request on Requests",
119-
"Waiting for your approval: send email on Gmail",
120-
"Waiting for your approval: click on example.com",
118+
"Get request",
119+
"Send your email",
120+
"Click on the page",
121121
]);
122+
// The app by its name, never its key; a computer action on the site it acts on.
123+
expect(cards[0]?.textContent).toContain("Requests");
124+
expect(cards[1]?.textContent).toContain("Gmail");
125+
expect(cards[2]?.textContent).toContain("example.com");
126+
for (const card of cards) {
127+
expect(card.textContent).not.toContain("requests/");
128+
expect(card.textContent).not.toContain("composio");
129+
}
122130
});
123131

124-
test("on the Bot's page, the row says what the Bot wants to do, over the app's name", async () => {
132+
test("on the Bot's page, the row says what the Bot wants to do, over the Bot and the app's name", async () => {
125133
serving();
126134
const view = draw(<WaitingForYou agentId="assistant" />);
127-
const row = (await view.findByText("General Assistant wants to get request"))
128-
.parentElement as HTMLElement;
129-
expect(row.textContent).toContain("Requests");
135+
const row = (await view.findByText("Get request")).closest(
136+
"section",
137+
) as HTMLElement;
138+
expect(row.textContent).toContain("General Assistant · Requests");
130139
expect(row.textContent).not.toContain("requests/");
131140
expect(
132-
view.getByText("General Assistant wants to send email").parentElement
133-
?.textContent,
134-
).toContain("Gmail");
135-
expect(view.getByText("General Assistant wants to click")).toBeTruthy();
141+
(view.getByText("Send your email").closest("section") as HTMLElement)
142+
.textContent,
143+
).toContain("General Assistant · Gmail");
144+
expect(view.getByText("Click on the page")).toBeTruthy();
136145
expect(view.container.textContent).not.toContain("composio");
137146
});

‎app/tests/chat-step.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
useConversationStatus,
1818
useReportConversation,
1919
} from "@/components/channels/conversation-presence";
20-
import { readToolName } from "@/lib/plugins/tool-name";
20+
import { readToolName } from "@openbot/shared/tool-name";
2121
import { type StepApproval, stepFor, THINKING } from "@/lib/channels/step";
2222
import { settleReactWork } from "./settle-react-work";
2323

‎app/tests/coding-tool-cards.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ test("a coding turn folds only its reads and searches, and keeps every change an
312312
]);
313313
// A lone read keeps its own line; the two listings fold into one, which says both didn't work.
314314
expect(view.getByText("Read 39 lines")).toBeTruthy();
315-
expect(view.getByText("Listed 2 directories")).toBeTruthy();
315+
expect(view.getByText("Listed 2 directories, didn't work")).toBeTruthy();
316316
expect(view.getByText("2 issues")).toBeTruthy();
317317
});
318318

‎app/tests/sidebar-row-actions.test.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ function renderSidebar(channels: ChannelSummary[], inbox?: unknown) {
123123
selfHostBannerDismissed: true,
124124
narration: null,
125125
skin: "openbot",
126+
proactive: "on",
126127
});
127128
queryClient.setQueryData(agentKeys.list(false), []);
128129
queryClient.setQueryDefaults(botLifecycleKeys.attention, {

‎app/tests/sidebar-search-hotkey.test.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ function signedIn() {
101101
selfHostBannerDismissed: true,
102102
narration: null,
103103
skin: "openbot",
104+
proactive: "on",
104105
});
105106
queryClient.setQueryData(agentKeys.list(false), []);
106107
queryClient.setQueryDefaults(botLifecycleKeys.attention, {

‎app/tests/task-approval-buttons.test.tsx‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { WaitingForYou } from "@/components/approvals/waiting";
88
import { settleReactWork } from "./settle-react-work";
99

1010
/*
11-
* An approval a task started from a check-in raised offers Allow once and Deny, never "Always allow
11+
* An approval a task started from a check-in raised offers Decline and Allow once, never "Always allow
1212
* here": tasks ask before every change whatever the person's rules say, so a rule saved from one
1313
* would be ignored the next time. Any other approval still offers it. Both places a person decides
1414
* one in the browser: the card in the conversation, and the row on the Bot's page.
@@ -99,7 +99,7 @@ const names = (scope: HTMLElement) =>
9999
.getAllByRole("button")
100100
.map((button) => button.textContent);
101101

102-
test("in the conversation, a check-in task's approval offers Allow once and Deny, and no standing rule", async () => {
102+
test("in the conversation, a check-in task's approval offers Decline and Allow once, and no standing rule", async () => {
103103
serving();
104104
const view = draw(
105105
<>
@@ -111,37 +111,38 @@ test("in the conversation, a check-in task's approval offers Allow once and Deny
111111
</section>
112112
</>,
113113
);
114-
await view.findAllByRole("region", { name: "Waiting for your approval" });
114+
await view.findAllByRole("button", { name: "Allow once" });
115115
expect(names(view.getByRole("region", { name: "task" }))).toEqual([
116+
"Decline",
116117
"Allow once",
117-
"Deny",
118118
]);
119119
expect(
120120
within(view.getByRole("region", { name: "task" })).getByText(
121121
"Tasks started from a check-in ask before every change.",
122122
),
123123
).toBeTruthy();
124124
expect(names(view.getByRole("region", { name: "own" }))).toEqual([
125+
"Decline",
125126
"Allow once",
126127
"Always allow here",
127-
"Deny",
128128
]);
129129
});
130130

131131
test("on the Bot's page, the same: the task's row has no Always allow here, the other row keeps it", async () => {
132132
serving();
133133
const view = draw(<WaitingForYou agentId="assistant" />);
134+
// Each row is the conversation's approval card, a region named by its title.
134135
const row = async (path: string) =>
135-
(await view.findByText(path, { exact: false })).closest(
136-
"[data-slot=item]",
136+
(await view.findAllByText(path, { exact: false }))[0]?.closest(
137+
"section",
137138
) as HTMLElement;
138139
expect(names(await row("deno-rollback-note.md"))).toEqual([
140+
"Decline",
139141
"Allow once",
140-
"Deny",
141142
]);
142143
expect(names(await row("notes.md"))).toEqual([
144+
"Decline",
143145
"Allow once",
144146
"Always allow here",
145-
"Deny",
146147
]);
147148
});

‎shared/tool-name.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,20 @@ export function readToolName(
3737
if (parts.length < 3 || parts[0] !== "mcp") return { label: name };
3838

3939
const [, rawServer, ...rest] = parts;
40-
return appToolName(rawServer ?? "", rest.join("__"));
40+
return appToolName(rawServer ?? "", rest.join("__"), running);
4141
}
4242

4343
/**
4444
* One app's tool, from its server key and the tool's own name: what `readToolName` reads out of an
4545
* `mcp__server__tool` name, and what an approval is asked about as `server/tool`.
4646
*/
47-
export function appToolName(server: string, tool: string): ToolName {
47+
export function appToolName(
48+
server: string,
49+
tool: string,
50+
running: ToolState = false,
51+
/** Whether the person's own mail and calendar are named for what happened to them. */
52+
personal = true,
53+
): ToolName {
4854
const broker = brokeredApp(server);
4955
/*
5056
* Research is named for what it did, not for who did it. `web_search_exa` would read "Web search
@@ -62,10 +68,10 @@ export function appToolName(server: string, tool: string): ToolName {
6268
* them, with the app beside the words so two mailboxes are still told apart. See `personalAction`
6369
* for which calls qualify.
6470
*/
65-
const personal = personalAction(server, tool);
66-
if (personal)
71+
const action = personal ? personalAction(server, tool) : null;
72+
if (action)
6773
return {
68-
label: PERSONAL_WORDS[personal][stateOf(running)],
74+
label: PERSONAL_WORDS[action][stateOf(running)],
6975
detail: connectorName(server),
7076
};
7177
/*
@@ -143,7 +149,13 @@ export function approvalWords(action: { toolRef: string; scope: string }): {
143149
const slash = action.toolRef.indexOf("/");
144150
const app = action.toolRef.slice(0, slash);
145151
if (slash > 0 && app === action.scope) {
146-
const { label } = appToolName(app, action.toolRef.slice(slash + 1));
152+
// The app's own action, as a task's line and a chat card have always said it: "send message".
153+
const { label } = appToolName(
154+
app,
155+
action.toolRef.slice(slash + 1),
156+
"plain",
157+
false,
158+
);
147159
return {
148160
action: label.charAt(0).toLowerCase() + label.slice(1),
149161
place: connectorName(app),

0 commit comments

Comments
 (0)