Skip to content

Commit bbdcad9

Browse files
Copilothotlong
andauthored
fix: address all code review comments — guard formatToolArgs/formatToolOutput, validate agent selection, remove unenforced requiresConfirmation
Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/f75e2ec2-9b8a-4e71-916e-8193b834ad01 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent ed14650 commit bbdcad9

4 files changed

Lines changed: 37 additions & 8 deletions

File tree

apps/studio/src/components/AiChatPanel.tsx

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,12 @@ function formatToolArgs(input: unknown): string {
6262
return entries
6363
.slice(0, 4)
6464
.map(([k, v]) => {
65-
const val = typeof v === 'string' ? v : JSON.stringify(v);
65+
let val: string;
66+
try {
67+
val = typeof v === 'string' ? v : (JSON.stringify(v) ?? String(v));
68+
} catch {
69+
val = String(v);
70+
}
6671
const display = val.length > 30 ? val.slice(0, 30) + '…' : val;
6772
return `${k}: ${display}`;
6873
})
@@ -80,7 +85,12 @@ function isToolPart(part: UIMessage['parts'][number]): part is Extract<UIMessage
8085
* Format tool output for display, truncating to a max length.
8186
*/
8287
function formatToolOutput(output: unknown, maxLen = 80): string {
83-
const raw = typeof output === 'string' ? output : JSON.stringify(output);
88+
let raw: string;
89+
try {
90+
raw = typeof output === 'string' ? output : (JSON.stringify(output) ?? '');
91+
} catch {
92+
raw = String(output ?? '');
93+
}
8494
return raw.length > maxLen ? raw.slice(0, maxLen) + '…' : raw;
8595
}
8696

@@ -284,6 +294,18 @@ export function AiChatPanel() {
284294
const baseUrl = getApiBaseUrl();
285295
const { agents, loading: agentsLoading } = useAgentList(baseUrl);
286296

297+
// Validate persisted agent against fetched list — fall back to general
298+
// chat if the previously selected agent is no longer available.
299+
useEffect(() => {
300+
if (agentsLoading) return;
301+
if (selectedAgent === GENERAL_CHAT_VALUE) return;
302+
const isValid = agents.some((a) => a.name === selectedAgent);
303+
if (!isValid) {
304+
setSelectedAgent(GENERAL_CHAT_VALUE);
305+
saveSelectedAgent(GENERAL_CHAT_VALUE);
306+
}
307+
}, [agents, agentsLoading, selectedAgent]);
308+
287309
const initialMessages = useMemo(() => loadMessages() as UIMessage[], []);
288310

289311
const transport = useMemo(

packages/services/service-ai/src/__tests__/metadata-tools.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,12 @@ describe('Individual Tool Metadata (.tool.ts)', () => {
123123
});
124124
}
125125

126-
it('should mark create_object as requiresConfirmation', () => {
127-
expect(createObjectTool.requiresConfirmation).toBe(true);
126+
it('should not set requiresConfirmation on create_object (server-side enforcement not yet implemented)', () => {
127+
expect(createObjectTool.requiresConfirmation).toBe(false);
128128
});
129129

130-
it('should mark delete_field as requiresConfirmation', () => {
131-
expect(deleteFieldTool.requiresConfirmation).toBe(true);
130+
it('should not set requiresConfirmation on delete_field (server-side enforcement not yet implemented)', () => {
131+
expect(deleteFieldTool.requiresConfirmation).toBe(false);
132132
});
133133

134134
it('should not mark read-only tools as requiresConfirmation', () => {

packages/services/service-ai/src/tools/create-object.tool.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ export const createObjectTool = defineTool({
1717
'Use this when the user wants to create a new entity, table, or data model.',
1818
category: 'data',
1919
builtIn: true,
20-
requiresConfirmation: true,
20+
// NOTE: requiresConfirmation is intentionally false (default) because the
21+
// server-side tool-call loop in AIService.chatWithTools/streamChatWithTools
22+
// executes tool calls immediately without checking this flag. The flag
23+
// should only be set once server-side approval gating is implemented to
24+
// avoid giving users a false sense of safety.
2125
parameters: {
2226
type: 'object',
2327
properties: {

packages/services/service-ai/src/tools/delete-field.tool.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ export const deleteFieldTool = defineTool({
1616
'Use this when the user explicitly wants to remove an attribute or column from a table.',
1717
category: 'data',
1818
builtIn: true,
19-
requiresConfirmation: true,
19+
// NOTE: requiresConfirmation is intentionally false (default) because the
20+
// server-side tool-call loop in AIService.chatWithTools/streamChatWithTools
21+
// executes tool calls immediately without checking this flag. The flag
22+
// should only be set once server-side approval gating is implemented.
2023
parameters: {
2124
type: 'object',
2225
properties: {

0 commit comments

Comments
 (0)