Skip to content

Commit 61123cf

Browse files
committed
Fix Codex P1 issues:
- Fix subscription filtering to preserve correct indexes - Move notification trigger to server-side for PWA support - Remove frontend notification logic (now server-side only)
1 parent c47f067 commit 61123cf

File tree

5 files changed

+31
-25
lines changed

5 files changed

+31
-25
lines changed

src/debug/agentSessionCli.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { HistoryService } from "@/services/historyService";
99
import { PartialService } from "@/services/partialService";
1010
import { AIService } from "@/services/aiService";
1111
import { InitStateManager } from "@/services/initStateManager";
12+
import { NotificationService } from "@/services/NotificationService";
1213
import { AgentSession, type AgentSessionChatEvent } from "@/services/agentSession";
1314
import {
1415
isCaughtUpMessage,
@@ -211,6 +212,7 @@ async function main(): Promise<void> {
211212
const partialService = new PartialService(config, historyService);
212213
const aiService = new AIService(config, historyService, partialService);
213214
const initStateManager = new InitStateManager(config);
215+
const notificationService = new NotificationService(config.rootDir, true);
214216
ensureProvidersConfig(config);
215217

216218
const session = new AgentSession({
@@ -220,6 +222,7 @@ async function main(): Promise<void> {
220222
partialService,
221223
aiService,
222224
initStateManager,
225+
notificationService,
223226
});
224227

225228
session.ensureMetadata({

src/services/NotificationService.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -141,15 +141,11 @@ export class NotificationService {
141141

142142
const results = await Promise.allSettled(sendPromises);
143143

144-
// Remove failed subscriptions
145-
const validSubscriptions = results
146-
.filter((result) => {
147-
if (result.status === "fulfilled" && result.value.success) {
148-
return true;
149-
}
150-
return false;
151-
})
152-
.map((_result, index) => subscriptions[index]);
144+
// Remove failed subscriptions - filter by original index to preserve correct mapping
145+
const validSubscriptions = subscriptions.filter((_, index) => {
146+
const result = results[index];
147+
return result.status === "fulfilled" && result.value.success;
148+
});
153149

154150
this.subscriptions.set(workspaceId, validSubscriptions);
155151
}

src/services/agentSession.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type { AIService } from "@/services/aiService";
77
import type { HistoryService } from "@/services/historyService";
88
import type { PartialService } from "@/services/partialService";
99
import type { InitStateManager } from "@/services/initStateManager";
10+
import type { NotificationService } from "@/services/NotificationService";
1011
import type { WorkspaceMetadata } from "@/types/workspace";
1112
import type { WorkspaceChatMessage, StreamErrorMessage, SendMessageOptions } from "@/types/ipc";
1213
import type { SendMessageError } from "@/types/errors";
@@ -39,6 +40,7 @@ interface AgentSessionOptions {
3940
partialService: PartialService;
4041
aiService: AIService;
4142
initStateManager: InitStateManager;
43+
notificationService: NotificationService;
4244
}
4345

4446
export class AgentSession {
@@ -48,6 +50,7 @@ export class AgentSession {
4850
private readonly partialService: PartialService;
4951
private readonly aiService: AIService;
5052
private readonly initStateManager: InitStateManager;
53+
private readonly notificationService: NotificationService;
5154
private readonly emitter = new EventEmitter();
5255
private readonly aiListeners: Array<{ event: string; handler: (...args: unknown[]) => void }> =
5356
[];
@@ -57,8 +60,15 @@ export class AgentSession {
5760

5861
constructor(options: AgentSessionOptions) {
5962
assert(options, "AgentSession requires options");
60-
const { workspaceId, config, historyService, partialService, aiService, initStateManager } =
61-
options;
63+
const {
64+
workspaceId,
65+
config,
66+
historyService,
67+
partialService,
68+
aiService,
69+
initStateManager,
70+
notificationService,
71+
} = options;
6272

6373
assert(typeof workspaceId === "string", "workspaceId must be a string");
6474
const trimmedWorkspaceId = workspaceId.trim();
@@ -70,6 +80,7 @@ export class AgentSession {
7080
this.partialService = partialService;
7181
this.aiService = aiService;
7282
this.initStateManager = initStateManager;
83+
this.notificationService = notificationService;
7384

7485
this.attachAiListeners();
7586
this.attachInitListeners();
@@ -393,7 +404,11 @@ export class AgentSession {
393404

394405
forward("stream-start", (payload) => this.emitChatEvent(payload));
395406
forward("stream-delta", (payload) => this.emitChatEvent(payload));
396-
forward("stream-end", (payload) => this.emitChatEvent(payload));
407+
forward("stream-end", (payload) => {
408+
this.emitChatEvent(payload);
409+
// Trigger completion notification (server-side so it works when app is closed)
410+
void this.notificationService.sendCompletionNotification(this.workspaceId, this.workspaceId);
411+
});
397412
forward("tool-call-start", (payload) => this.emitChatEvent(payload));
398413
forward("tool-call-delta", (payload) => this.emitChatEvent(payload));
399414
forward("tool-call-end", (payload) => this.emitChatEvent(payload));

src/services/ipcMain.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ export class IpcMain {
162162
partialService: this.partialService,
163163
aiService: this.aiService,
164164
initStateManager: this.initStateManager,
165+
notificationService: this.notificationService,
165166
});
166167

167168
const chatUnsubscribe = session.onChatEvent((event) => {

src/stores/WorkspaceStore.ts

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import type { FrontendWorkspaceMetadata } from "@/types/workspace";
55
import type { WorkspaceChatMessage } from "@/types/ipc";
66
import type { TodoItem } from "@/types/tools";
77
import { StreamingMessageAggregator } from "@/utils/messages/StreamingMessageAggregator";
8-
import { updatePersistedState, readPersistedState } from "@/hooks/usePersistedState";
9-
import { getRetryStateKey, NOTIFICATION_ENABLED_KEY } from "@/constants/storage";
8+
import { updatePersistedState } from "@/hooks/usePersistedState";
9+
import { getRetryStateKey } from "@/constants/storage";
1010
import { CUSTOM_EVENTS } from "@/constants/events";
1111
import { useSyncExternalStore } from "react";
1212
import { isCaughtUpMessage, isStreamError, isDeleteMessage, isCmuxMessage } from "@/types/ipc";
@@ -154,17 +154,8 @@ export class WorkspaceStore {
154154
this.states.bump(workspaceId);
155155
this.checkAndBumpRecencyIfChanged();
156156
this.finalizeUsageStats(workspaceId, (data as { metadata?: never }).metadata);
157-
158-
// Trigger completion notification if enabled
159-
const notificationsEnabled = readPersistedState(NOTIFICATION_ENABLED_KEY, false);
160-
if (notificationsEnabled) {
161-
// Only notify if document is hidden (tab backgrounded) or on desktop
162-
const shouldNotify = typeof document === "undefined" || document.hidden;
163-
if (shouldNotify) {
164-
// Use workspaceId as the display name for notifications
165-
void window.api.notification.send(workspaceId, workspaceId);
166-
}
167-
}
157+
// Note: Completion notifications are now triggered server-side in AgentSession
158+
// to support push notifications when the app is closed
168159
},
169160
"stream-abort": (workspaceId, aggregator, data) => {
170161
aggregator.clearTokenState((data as { messageId: string }).messageId);

0 commit comments

Comments
 (0)