Skip to content

Commit 8d9055e

Browse files
committed
🤖 refactor: simplify status_set polling and drop persistence
1 parent f6ca750 commit 8d9055e

File tree

15 files changed

+38
-385
lines changed

15 files changed

+38
-385
lines changed

src/browser/components/tools/StatusSetToolCall.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ export const StatusSetToolCall: React.FC<StatusSetToolCallProps> = ({
2424

2525
const iconEmoji = "📡";
2626

27-
const summary = `poll=${args.poll_interval_ms}ms: ${args.script.split(/\r?\n/)[0] ?? ""}`;
27+
const pollLabel = args.poll_interval_s === undefined ? "once" : `${args.poll_interval_s}s`;
28+
const summary = `poll=${pollLabel}: ${args.script.split(/\r?\n/)[0] ?? ""}`;
2829

2930
return (
3031
<ToolContainer expanded={false}>

src/browser/stories/App.chat.stories.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ export const WithAgentStatus: AppStory = {
204204
createStatusTool(
205205
"call-1",
206206
"echo '🚀 PR #1234 waiting for CI https://github.com/example/repo/pull/1234'",
207-
5000
207+
5
208208
),
209209
],
210210
}

src/browser/stories/App.demo.stories.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ export const Comprehensive: AppStory = {
198198
createStatusTool(
199199
"call-4",
200200
"echo '🚀 PR #1234 waiting for CI https://github.com/example/repo/pull/1234'",
201-
5000
201+
5
202202
),
203203
],
204204
}),

src/browser/stories/mockFactory.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,14 +292,14 @@ export function createTerminalTool(
292292
export function createStatusTool(
293293
toolCallId: string,
294294
script: string,
295-
pollIntervalMs: number
295+
pollIntervalS?: number
296296
): MuxPart {
297297
return {
298298
type: "dynamic-tool",
299299
toolCallId,
300300
toolName: "status_set",
301301
state: "output-available",
302-
input: { script, poll_interval_ms: pollIntervalMs },
302+
input: { script, ...(pollIntervalS !== undefined ? { poll_interval_s: pollIntervalS } : {}) },
303303
output: { success: true },
304304
};
305305
}

src/browser/utils/messages/StreamingMessageAggregator.status.test.ts

Lines changed: 4 additions & 277 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,6 @@
1-
import { afterAll, afterEach, beforeEach, describe, expect, it } from "bun:test";
2-
import { getStatusStateKey } from "@/common/constants/storage";
1+
import { describe, expect, it } from "bun:test";
32
import { StreamingMessageAggregator } from "./StreamingMessageAggregator";
43

5-
const originalLocalStorage: Storage | undefined = (globalThis as { localStorage?: Storage })
6-
.localStorage;
7-
8-
const createMockLocalStorage = () => {
9-
const store = new Map<string, string>();
10-
return {
11-
get length() {
12-
return store.size;
13-
},
14-
key: (index: number) => Array.from(store.keys())[index] ?? null,
15-
getItem: (key: string) => (store.has(key) ? store.get(key)! : null),
16-
setItem: (key: string, value: string) => {
17-
store.set(key, value);
18-
},
19-
removeItem: (key: string) => {
20-
store.delete(key);
21-
},
22-
clear: () => {
23-
store.clear();
24-
},
25-
} satisfies Storage;
26-
};
27-
28-
beforeEach(() => {
29-
const mock = createMockLocalStorage();
30-
Object.defineProperty(globalThis, "localStorage", {
31-
value: mock,
32-
configurable: true,
33-
});
34-
});
35-
36-
afterEach(() => {
37-
const ls = (globalThis as { localStorage?: Storage }).localStorage;
38-
ls?.clear?.();
39-
});
40-
41-
afterAll(() => {
42-
if (originalLocalStorage !== undefined) {
43-
Object.defineProperty(globalThis, "localStorage", { value: originalLocalStorage });
44-
} else {
45-
delete (globalThis as { localStorage?: Storage }).localStorage;
46-
}
47-
});
48-
494
describe("StreamingMessageAggregator - Agent Status", () => {
505
it("should start with undefined agent status", () => {
516
const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z");
@@ -111,7 +66,6 @@ describe("StreamingMessageAggregator - Agent Status", () => {
11166
toolName: "status_set",
11267
args: {
11368
script: "echo '🚀 PR #1 https://github.com/example/repo/pull/1'",
114-
poll_interval_ms: 0,
11569
},
11670
tokens: 10,
11771
timestamp: Date.now(),
@@ -268,7 +222,7 @@ describe("StreamingMessageAggregator - Agent Status", () => {
268222
messageId,
269223
toolCallId: "tool1",
270224
toolName: "status_set",
271-
args: { script: "echo 'not important'", poll_interval_ms: 0 },
225+
args: { script: "echo 'not important'" },
272226
tokens: 10,
273227
timestamp: Date.now(),
274228
});
@@ -329,7 +283,6 @@ describe("StreamingMessageAggregator - Agent Status", () => {
329283
toolName: "status_set",
330284
args: {
331285
script: "echo '🚀 PR #1 https://github.com/example/repo/pull/1'",
332-
poll_interval_ms: 0,
333286
},
334287
tokens: 10,
335288
timestamp: Date.now(),
@@ -390,8 +343,8 @@ describe("StreamingMessageAggregator - Agent Status", () => {
390343
toolCallId: "tool1",
391344
toolName: "status_set",
392345
state: "output-available" as const,
393-
input: { emoji: "🔍", message: "Analyzing code" },
394-
output: { success: true, emoji: "🔍", message: "Analyzing code" },
346+
input: { script: "echo 'Analyzing code'" },
347+
output: { success: true },
395348
timestamp: Date.now(),
396349
},
397350
],
@@ -406,167 +359,6 @@ describe("StreamingMessageAggregator - Agent Status", () => {
406359
expect(aggregator.getAgentStatus()).toBeUndefined();
407360
});
408361

409-
it("should not reconstruct agent status even if multiple historical status_set tool calls exist", () => {
410-
const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z");
411-
412-
// Create historical messages with multiple status_set calls
413-
const historicalMessages = [
414-
{
415-
id: "msg1",
416-
role: "assistant" as const,
417-
parts: [
418-
{
419-
type: "dynamic-tool" as const,
420-
toolCallId: "tool1",
421-
toolName: "status_set",
422-
state: "output-available" as const,
423-
input: { emoji: "🔍", message: "First status" },
424-
output: { success: true, emoji: "🔍", message: "First status" },
425-
timestamp: Date.now(),
426-
},
427-
],
428-
metadata: { timestamp: Date.now(), historySequence: 1 },
429-
},
430-
{
431-
id: "msg2",
432-
role: "assistant" as const,
433-
parts: [
434-
{
435-
type: "dynamic-tool" as const,
436-
toolCallId: "tool2",
437-
toolName: "status_set",
438-
state: "output-available" as const,
439-
input: { emoji: "📝", message: "Second status" },
440-
output: { success: true, emoji: "📝", message: "Second status" },
441-
timestamp: Date.now(),
442-
},
443-
],
444-
metadata: { timestamp: Date.now(), historySequence: 2 },
445-
},
446-
];
447-
448-
// Load historical messages
449-
aggregator.loadHistoricalMessages(historicalMessages);
450-
451-
expect(aggregator.getAgentStatus()).toBeUndefined();
452-
});
453-
454-
it("should not reconstruct status from failed status_set in historical messages", () => {
455-
const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z");
456-
457-
// Create historical message with failed status_set
458-
const historicalMessages = [
459-
{
460-
id: "msg1",
461-
role: "assistant" as const,
462-
parts: [
463-
{
464-
type: "dynamic-tool" as const,
465-
toolCallId: "tool1",
466-
toolName: "status_set",
467-
state: "output-available" as const,
468-
input: { emoji: "not-emoji", message: "test" },
469-
output: { success: false, error: "emoji must be a single emoji character" },
470-
timestamp: Date.now(),
471-
},
472-
],
473-
metadata: { timestamp: Date.now(), historySequence: 1 },
474-
},
475-
];
476-
477-
// Load historical messages
478-
aggregator.loadHistoricalMessages(historicalMessages);
479-
480-
// Status should remain undefined (failed validation)
481-
expect(aggregator.getAgentStatus()).toBeUndefined();
482-
});
483-
484-
it("should retain last status_set even if later assistant messages omit it", () => {
485-
const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z");
486-
487-
const historicalMessages = [
488-
{
489-
id: "assistant1",
490-
role: "assistant" as const,
491-
parts: [
492-
{
493-
type: "dynamic-tool" as const,
494-
toolCallId: "tool1",
495-
toolName: "status_set",
496-
state: "output-available" as const,
497-
input: { emoji: "🧪", message: "Running tests" },
498-
output: { success: true, emoji: "🧪", message: "Running tests" },
499-
timestamp: 1000,
500-
},
501-
],
502-
metadata: { timestamp: 1000, historySequence: 1 },
503-
},
504-
{
505-
id: "assistant2",
506-
role: "assistant" as const,
507-
parts: [{ type: "text" as const, text: "[compaction summary]" }],
508-
metadata: { timestamp: 2000, historySequence: 2 },
509-
},
510-
];
511-
512-
aggregator.loadHistoricalMessages(historicalMessages);
513-
514-
const status = aggregator.getAgentStatus();
515-
expect(status?.emoji).toBe("🧪");
516-
expect(status?.message).toBe("Running tests");
517-
});
518-
519-
it("should restore persisted status when history is compacted away", () => {
520-
const workspaceId = "workspace1";
521-
const persistedStatus = {
522-
emoji: "🔗",
523-
message: "PR open",
524-
url: "https://example.com/pr/123",
525-
} as const;
526-
localStorage.setItem(getStatusStateKey(workspaceId), JSON.stringify(persistedStatus));
527-
528-
const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z", workspaceId);
529-
530-
// History with no status_set (e.g., after compaction removes older tool calls)
531-
const historicalMessages = [
532-
{
533-
id: "assistant2",
534-
role: "assistant" as const,
535-
parts: [{ type: "text" as const, text: "[compacted history]" }],
536-
metadata: { timestamp: 3000, historySequence: 1 },
537-
},
538-
];
539-
540-
aggregator.loadHistoricalMessages(historicalMessages);
541-
542-
expect(aggregator.getAgentStatus()).toEqual(persistedStatus);
543-
});
544-
545-
it("should rehydrate agent status from persisted localStorage", () => {
546-
const workspaceId = "workspace1";
547-
const key = getStatusStateKey(workspaceId);
548-
549-
const aggregator1 = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z");
550-
aggregator1.handleMessage({
551-
type: "agent-status-update",
552-
workspaceId,
553-
status: {
554-
emoji: "🚀",
555-
message: "PR #1 checks running",
556-
url: "https://github.com/example/repo/pull/1",
557-
},
558-
});
559-
560-
expect((globalThis as { localStorage: Storage }).localStorage.getItem(key)).not.toBeNull();
561-
562-
const aggregator2 = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z");
563-
expect(aggregator2.getAgentStatus()).toEqual({
564-
emoji: "🚀",
565-
message: "PR #1 checks running",
566-
url: "https://github.com/example/repo/pull/1",
567-
});
568-
});
569-
570362
it("should store URL when provided in agent-status-update", () => {
571363
const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z");
572364

@@ -673,69 +465,4 @@ describe("StreamingMessageAggregator - Agent Status", () => {
673465
url: testUrl,
674466
});
675467
});
676-
677-
it("should not reconstruct URL from history tool calls", () => {
678-
// status is ephemeral and persisted outside chat history; historical tool calls should not
679-
// reconstruct the status/link.
680-
const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z");
681-
const testUrl = "https://github.com/owner/repo/pull/123";
682-
683-
// Historical messages: first assistant sets URL, second assistant updates status without URL
684-
const historicalMessages = [
685-
{
686-
id: "user1",
687-
role: "user" as const,
688-
parts: [{ type: "text" as const, text: "Make a PR" }],
689-
metadata: { timestamp: 1000, historySequence: 1 },
690-
},
691-
{
692-
id: "assistant1",
693-
role: "assistant" as const,
694-
parts: [
695-
{
696-
type: "dynamic-tool" as const,
697-
toolName: "status_set",
698-
toolCallId: "tool1",
699-
state: "output-available" as const,
700-
input: { emoji: "🔗", message: "PR submitted", url: testUrl },
701-
output: { success: true, emoji: "🔗", message: "PR submitted", url: testUrl },
702-
timestamp: 1001,
703-
tokens: 10,
704-
},
705-
],
706-
metadata: { timestamp: 1001, historySequence: 2 },
707-
},
708-
{
709-
id: "user2",
710-
role: "user" as const,
711-
parts: [{ type: "text" as const, text: "Continue" }],
712-
metadata: { timestamp: 2000, historySequence: 3 },
713-
},
714-
{
715-
id: "assistant2",
716-
role: "assistant" as const,
717-
parts: [
718-
{
719-
type: "dynamic-tool" as const,
720-
toolName: "status_set",
721-
toolCallId: "tool2",
722-
state: "output-available" as const,
723-
input: { emoji: "✅", message: "Tests passed" },
724-
output: { success: true, emoji: "✅", message: "Tests passed" }, // No URL!
725-
timestamp: 2001,
726-
tokens: 10,
727-
},
728-
],
729-
metadata: { timestamp: 2001, historySequence: 4 },
730-
},
731-
];
732-
733-
aggregator.loadHistoricalMessages(historicalMessages);
734-
735-
expect(aggregator.getAgentStatus()).toBeUndefined();
736-
});
737-
738-
// Note: URL persistence through compaction is handled via localStorage,
739-
// which is tested in integration tests. The aggregator saves lastStatusUrl
740-
// to localStorage when it changes, and loads it on construction.
741468
});

src/browser/utils/messages/StreamingMessageAggregator.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ describe("StreamingMessageAggregator", () => {
312312
toolCallId: "tool2",
313313
toolName: "status_set",
314314
state: "output-available" as const,
315-
input: { script: "echo 'Working on it'", poll_interval_ms: 0 },
315+
input: { script: "echo 'Working on it'" },
316316
output: { success: true },
317317
},
318318
],

0 commit comments

Comments
 (0)