diff --git a/src/features/chat/app-server/inbound/controller.ts b/src/features/chat/app-server/inbound/controller.ts index f30dfc69..ece64f81 100644 --- a/src/features/chat/app-server/inbound/controller.ts +++ b/src/features/chat/app-server/inbound/controller.ts @@ -16,6 +16,7 @@ import { userCancelledInputRequestMessage, } from "../../application/pending-requests/messages"; import { createApprovalResultItem, createUserInputResultItem } from "../../domain/pending-requests/result-items"; +import { createLocalChatItemIdFactory, type LocalChatItemIdFactory } from "../../domain/local-id"; import { planChatNotification, type ChatNotificationEffect } from "./notification-plan"; import { routeServerRequest } from "./routing"; @@ -33,6 +34,8 @@ export interface ChatInboundControllerActions { } export class ChatInboundController { + private readonly localItemIds: LocalChatItemIdFactory = createLocalChatItemIdFactory(); + constructor( private readonly store: ChatStateStore, private readonly actions: ChatInboundControllerActions, @@ -162,7 +165,7 @@ export class ChatInboundController { } private localItemId(prefix: string): string { - return `${prefix}-${String(Date.now())}-${Math.random().toString(36).slice(2)}`; + return this.localItemIds.next(prefix); } private runNotificationEffect(effect: ChatNotificationEffect): void { diff --git a/src/features/chat/app-server/inbound/notification-plan.ts b/src/features/chat/app-server/inbound/notification-plan.ts index da84f005..119d79ff 100644 --- a/src/features/chat/app-server/inbound/notification-plan.ts +++ b/src/features/chat/app-server/inbound/notification-plan.ts @@ -33,6 +33,7 @@ import { import { streamingFileChangeMessageStreamItem } from "../mappers/message-stream/streaming-items"; import { attachHookRunsToTurn } from "../../domain/message-stream/updates"; import { messageStreamItems } from "../../application/state/message-stream"; +import { isLocalUserMessageId } from "../../domain/local-id"; import { routeServerNotification, type DiagnosticStatusNotificationMethod, @@ -556,10 +557,6 @@ function isFallbackOptimisticUserMessageForTurn( return serverUserFallbackTexts.has(item.copyText ?? item.text); } -function isLocalUserMessageId(id: string): boolean { - return id.startsWith("local-user-") || id.startsWith("local-steer-"); -} - function isString(value: string | null | undefined): value is string { return typeof value === "string"; } diff --git a/src/features/chat/app-server/inbound/routing.ts b/src/features/chat/app-server/inbound/routing.ts index 90f68ba4..e7984760 100644 --- a/src/features/chat/app-server/inbound/routing.ts +++ b/src/features/chat/app-server/inbound/routing.ts @@ -237,6 +237,9 @@ export function routeServerNotification(notification: ServerNotification, scope: export function isMessageInActiveScope(message: ServerNotification | ServerRequest, scope: ActiveRouteScope): boolean { const threadId = messageThreadId(message); + // Scope identifiers are filters only when both the notification and the + // active panel have one. Thread catalog and idle-thread notifications often + // omit active turn scope, so missing ids stay eligible for the active route. if (threadId && scope.activeThreadId && threadId !== scope.activeThreadId) return false; const turnId = messageTurnId(message); diff --git a/src/features/chat/application/conversation/optimistic-turn-start.ts b/src/features/chat/application/conversation/optimistic-turn-start.ts index 72294902..83b07774 100644 --- a/src/features/chat/application/conversation/optimistic-turn-start.ts +++ b/src/features/chat/application/conversation/optimistic-turn-start.ts @@ -4,6 +4,7 @@ import type { MessageStreamItemProvenance } from "../../domain/message-stream/pr import { fileMentionsFromInput } from "../../domain/message-stream/format/file-mentions"; import { userMessageDisplayText } from "../../domain/message-stream/format/user-message-text"; import { attachHookRunsToTurn } from "../../domain/message-stream/updates"; +import { isLocalSteerMessageClientId } from "../../domain/local-id"; import type { CodexInput } from "../../../../domain/chat/input"; export interface LocalUserMessageParams { @@ -68,7 +69,7 @@ function localUserMessageProvenance(id: string): MessageStreamItemProvenance { return { source: "localUser", channel: "optimistic", - interaction: id.startsWith("local-steer-") ? "steer" : "prompt", + interaction: isLocalSteerMessageClientId(id) ? "steer" : "prompt", sourceId: id, }; } diff --git a/src/features/chat/application/conversation/turn-submission-controller.ts b/src/features/chat/application/conversation/turn-submission-controller.ts index c98ffa3a..80ee4b21 100644 --- a/src/features/chat/application/conversation/turn-submission-controller.ts +++ b/src/features/chat/application/conversation/turn-submission-controller.ts @@ -3,6 +3,7 @@ import type { CodexInput } from "../../../../domain/chat/input"; import type { ReferencedThreadMetadata } from "../../../../domain/threads/reference"; import { submissionStateSnapshot } from "../state/selectors"; import type { ChatStateStore } from "../state/reducer"; +import { createLocalChatItemIdFactory, type LocalChatItemIdFactory } from "../../domain/local-id"; import { currentTurnNotSteerableMessage, STATUS_STEERED_CURRENT_TURN, STATUS_TURN_RUNNING } from "./messages"; import { acknowledgeOptimisticTurnStart, @@ -28,9 +29,11 @@ export interface TurnSubmissionControllerHost { } export class TurnSubmissionController { - private static localItemSequence = 0; + private readonly localItemIds: LocalChatItemIdFactory; - constructor(private readonly host: TurnSubmissionControllerHost) {} + constructor(private readonly host: TurnSubmissionControllerHost) { + this.localItemIds = createLocalChatItemIdFactory(); + } async sendTurnText(text: string, codexInputOverride?: CodexInput, referencedThread?: ReferencedThreadMetadata): Promise { if (!(await this.host.ensureRestoredThreadLoaded())) return; @@ -56,7 +59,7 @@ export class TurnSubmissionController { if (!(await this.host.applyPendingThreadSettings())) return; const codexInput = codexInputOverride ?? this.host.codexInput(text); - optimisticUserId = TurnSubmissionController.nextLocalItemId("local-user"); + optimisticUserId = this.localItemIds.next("local-user"); const optimistic = optimisticTurnStart({ id: optimisticUserId, text, @@ -127,7 +130,7 @@ export class TurnSubmissionController { } const codexInput = codexInputOverride ?? this.host.codexInput(text); - const localSteerId = TurnSubmissionController.nextLocalItemId("local-steer"); + const localSteerId = this.localItemIds.next("local-steer"); this.host.setDraft("", { clearSuggestions: true }); try { @@ -155,9 +158,4 @@ export class TurnSubmissionController { const state = submissionStateSnapshot(this.host.stateStore.getState()); return state.activeThreadId === threadId && state.activeTurnId === turnId; } - - private static nextLocalItemId(prefix: "local-user" | "local-steer"): string { - this.localItemSequence += 1; - return `${prefix}-${String(Date.now())}-${String(this.localItemSequence)}`; - } } diff --git a/src/features/chat/application/threads/goal-actions.ts b/src/features/chat/application/threads/goal-actions.ts index d03b22ee..2c13d303 100644 --- a/src/features/chat/application/threads/goal-actions.ts +++ b/src/features/chat/application/threads/goal-actions.ts @@ -4,6 +4,7 @@ import type { ThreadGoal, ThreadGoalStatus, ThreadGoalUpdate } from "../../../.. import type { ChatStateStore } from "../state/reducer"; import type { GoalMessageStreamItem } from "../../domain/message-stream/items"; import { goalChangeItem } from "../../domain/message-stream/factories/goal-items"; +import { createLocalChatItemIdFactory } from "../../domain/local-id"; import { emptyGoalObjectiveMessage } from "./messages"; export interface GoalActionsHost { @@ -24,26 +25,37 @@ export interface GoalActions { } export function createGoalActions(host: GoalActionsHost): GoalActions { + const localItemIds = createLocalChatItemIdFactory(); return { activeGoal: () => host.stateStore.getState().activeThread.goal, - syncThreadGoal: (threadId) => syncThreadGoal(host, threadId), - setObjective: (threadId, objective, tokenBudget) => setObjective(host, threadId, objective, tokenBudget), - setStatus: (threadId, status) => setGoalStatus(host, threadId, status), - clear: (threadId) => clearGoal(host, threadId), + syncThreadGoal: (threadId) => syncThreadGoal(host, localItemIds, threadId), + setObjective: (threadId, objective, tokenBudget) => setObjective(host, localItemIds, threadId, objective, tokenBudget), + setStatus: (threadId, status) => setGoalStatus(host, localItemIds, threadId, status), + clear: (threadId) => clearGoal(host, localItemIds, threadId), }; } -async function syncThreadGoal(host: GoalActionsHost, threadId: string): Promise { +async function syncThreadGoal( + host: GoalActionsHost, + localItemIds: ReturnType, + threadId: string, +): Promise { const client = host.currentClient(); if (!client) return; try { - applyGoalIfActive(host, threadId, await readThreadGoal(client, threadId), { reportChange: false }); + applyGoalIfActive(host, localItemIds, threadId, await readThreadGoal(client, threadId), { reportChange: false }); } catch (error) { addThreadScopedSystemMessage(host, threadId, `Could not load thread goal: ${errorMessage(error)}`); } } -async function setObjective(host: GoalActionsHost, threadId: string, objective: string, tokenBudget: number | null): Promise { +async function setObjective( + host: GoalActionsHost, + localItemIds: ReturnType, + threadId: string, + objective: string, + tokenBudget: number | null, +): Promise { const trimmed = objective.trim(); if (!trimmed) { host.addSystemMessage(emptyGoalObjectiveMessage()); @@ -51,7 +63,7 @@ async function setObjective(host: GoalActionsHost, threadId: string, objective: } const current = host.stateStore.getState().activeThread.goal; const isNewGoal = current === null; - const applied = await setGoal(host, threadId, { + const applied = await setGoal(host, localItemIds, threadId, { objective: trimmed, status: current?.status ?? "active", tokenBudget, @@ -62,17 +74,26 @@ async function setObjective(host: GoalActionsHost, threadId: string, objective: return applied; } -function setGoalStatus(host: GoalActionsHost, threadId: string, status: ThreadGoalStatus): Promise { - return setGoal(host, threadId, { status }); +function setGoalStatus( + host: GoalActionsHost, + localItemIds: ReturnType, + threadId: string, + status: ThreadGoalStatus, +): Promise { + return setGoal(host, localItemIds, threadId, { status }); } -async function clearGoal(host: GoalActionsHost, threadId: string): Promise { +async function clearGoal( + host: GoalActionsHost, + localItemIds: ReturnType, + threadId: string, +): Promise { await host.ensureConnected(); const client = host.currentClient(); if (!client) return false; try { await client.clearThreadGoal(threadId); - applyGoalIfActive(host, threadId, null, { reportChange: true }); + applyGoalIfActive(host, localItemIds, threadId, null, { reportChange: true }); return true; } catch (error) { addThreadScopedSystemMessage(host, threadId, errorMessage(error)); @@ -80,22 +101,33 @@ async function clearGoal(host: GoalActionsHost, threadId: string): Promise { +async function setGoal( + host: GoalActionsHost, + localItemIds: ReturnType, + threadId: string, + params: ThreadGoalUpdate, +): Promise { await host.ensureConnected(); const client = host.currentClient(); if (!client) return false; try { - return applyGoalIfActive(host, threadId, await setThreadGoal(client, threadId, params), { reportChange: true }); + return applyGoalIfActive(host, localItemIds, threadId, await setThreadGoal(client, threadId, params), { reportChange: true }); } catch (error) { addThreadScopedSystemMessage(host, threadId, errorMessage(error)); return false; } } -function applyGoalIfActive(host: GoalActionsHost, threadId: string, goal: ThreadGoal | null, options: { reportChange: boolean }): boolean { +function applyGoalIfActive( + host: GoalActionsHost, + localItemIds: ReturnType, + threadId: string, + goal: ThreadGoal | null, + options: { reportChange: boolean }, +): boolean { const state = host.stateStore.getState(); if (state.activeThread.id !== threadId) return false; - const item = options.reportChange ? goalChangeItem(goalEventId(), state.activeThread.goal, goal) : null; + const item = options.reportChange ? goalChangeItem(localItemIds.next("goal"), state.activeThread.goal, goal) : null; host.stateStore.dispatch({ type: "active-thread/goal-set", goal }); if (item) host.addGoalEvent(item); host.refreshLiveState(); @@ -120,7 +152,3 @@ function addThreadScopedSystemMessage(host: GoalActionsHost, threadId: string, t function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } - -function goalEventId(): string { - return `goal-${String(Date.now())}-${Math.random().toString(36).slice(2)}`; -} diff --git a/src/features/chat/domain/local-id.ts b/src/features/chat/domain/local-id.ts new file mode 100644 index 00000000..752dbe22 --- /dev/null +++ b/src/features/chat/domain/local-id.ts @@ -0,0 +1,40 @@ +export interface LocalChatItemIdFactory { + next(prefix: string): string; +} + +export interface LocalChatItemIdFactoryOptions { + nowMs?: () => number; + seed?: string; +} + +let factorySequence = 0; + +export function createLocalChatItemIdFactory(options: LocalChatItemIdFactoryOptions = {}): LocalChatItemIdFactory { + const nowMs = options.nowMs ?? Date.now; + const seed = sanitizeIdPart(options.seed ?? defaultIdSeed()); + factorySequence += 1; + const factoryId = `${seed}-${factorySequence.toString(36)}`; + let itemSequence = 0; + return { + next(prefix: string): string { + itemSequence += 1; + return `${prefix}-${String(nowMs())}-${factoryId}-${itemSequence.toString(36)}`; + }, + }; +} + +export function isLocalUserMessageId(id: string): boolean { + return id.startsWith("local-user-") || id.startsWith("local-steer-"); +} + +export function isLocalSteerMessageClientId(id: string | null | undefined): boolean { + return id?.startsWith("local-steer-") ?? false; +} + +function defaultIdSeed(): string { + return Date.now().toString(36); +} + +function sanitizeIdPart(value: string): string { + return value.replace(/[^A-Za-z0-9_-]/g, "").slice(0, 24) || "local"; +} diff --git a/src/features/chat/domain/message-stream/semantics/classify.ts b/src/features/chat/domain/message-stream/semantics/classify.ts index d276dde5..bee75304 100644 --- a/src/features/chat/domain/message-stream/semantics/classify.ts +++ b/src/features/chat/domain/message-stream/semantics/classify.ts @@ -1,4 +1,5 @@ import type { MessageStreamItem } from "../items"; +import { isLocalSteerMessageClientId } from "../../local-id"; import type { MessageStreamLifecycle, MessageStreamMeaning, @@ -130,7 +131,7 @@ function isSteeringUserMessage( if (!item.turnId) return false; const seenCount = seenUserMessagesByTurn.get(item.turnId) ?? 0; seenUserMessagesByTurn.set(item.turnId, seenCount + 1); - return (item.clientId?.startsWith("local-steer-") ?? false) || seenCount > 0; + return isLocalSteerMessageClientId(item.clientId) || seenCount > 0; } function definedProp(key: Key, value: Value | undefined): Partial> { diff --git a/src/features/chat/host/session.ts b/src/features/chat/host/session.ts index d731d54f..c48c53e6 100644 --- a/src/features/chat/host/session.ts +++ b/src/features/chat/host/session.ts @@ -21,6 +21,7 @@ import type { ComposerSubmitActions } from "../application/conversation/composer import { codexPanelDisplayTitle } from "../application/threads/title-display"; import type { MessageStreamItem, MessageStreamNoticeSection } from "../domain/message-stream/items"; import { createStructuredSystemItem, createSystemItem } from "../domain/message-stream/factories/system-items"; +import { createLocalChatItemIdFactory, type LocalChatItemIdFactory } from "../domain/local-id"; import { effortStatusLines as buildEffortStatusLines, modelStatusLines as buildModelStatusLines, @@ -174,6 +175,7 @@ export class ChatPanelSession { private readonly connectionWork = new ChatConnectionWorkTracker(); private readonly resumeWork = new ChatResumeWorkTracker(); private readonly messageScrollIntent: ChatMessageScrollIntentState = createChatMessageScrollIntentState(); + private readonly localItemIds: LocalChatItemIdFactory = createLocalChatItemIdFactory(); private opened = false; private closing = false; @@ -951,10 +953,10 @@ export class ChatPanelSession { } private systemItem(text: string): MessageStreamItem { - return createSystemItem(`system-${String(Date.now())}-${Math.random().toString(36).slice(2)}`, text); + return createSystemItem(this.localItemIds.next("system"), text); } private structuredSystemItem(text: string, details: MessageStreamNoticeSection[]): MessageStreamItem { - return createStructuredSystemItem(`system-${String(Date.now())}-${Math.random().toString(36).slice(2)}`, text, details); + return createStructuredSystemItem(this.localItemIds.next("system"), text, details); } } diff --git a/tests/features/chat/conversation/turns/turn-submission-controller.test.ts b/tests/features/chat/conversation/turns/turn-submission-controller.test.ts index b003fe14..0dfe8245 100644 --- a/tests/features/chat/conversation/turns/turn-submission-controller.test.ts +++ b/tests/features/chat/conversation/turns/turn-submission-controller.test.ts @@ -78,7 +78,7 @@ describe("TurnSubmissionController", () => { threadId: "thread", cwd: "/vault", input: textInput("hello"), - clientUserMessageId: expect.stringMatching(/^local-user-\d+-\d+$/), + clientUserMessageId: expect.stringMatching(/^local-user-\d+-[A-Za-z0-9_-]+-[a-z0-9]+$/), }); expect(stateStore.getState().turn.lifecycle).toEqual({ kind: "running", turnId: "turn" }); expect(host.setDraft).toHaveBeenCalledWith(""); @@ -147,7 +147,12 @@ describe("TurnSubmissionController", () => { await controller.sendTurnText("follow up"); - expect(steerTurn).toHaveBeenCalledWith("thread", "turn", textInput("follow up"), expect.stringMatching(/^local-steer-\d+-\d+$/)); + expect(steerTurn).toHaveBeenCalledWith( + "thread", + "turn", + textInput("follow up"), + expect.stringMatching(/^local-steer-\d+-[A-Za-z0-9_-]+-[a-z0-9]+$/), + ); expect(startTurn).not.toHaveBeenCalled(); expect(host.setStatus).toHaveBeenCalledWith("Steered current turn."); const localSteerId = steerTurn.mock.calls[0]?.[3]; @@ -182,8 +187,8 @@ describe("TurnSubmissionController", () => { const firstId = first.startTurn.mock.calls[0]?.[0].clientUserMessageId; const secondId = second.startTurn.mock.calls[0]?.[0].clientUserMessageId; - expect(firstId).toMatch(/^local-user-1234-\d+$/); - expect(secondId).toMatch(/^local-user-1234-\d+$/); + expect(firstId).toMatch(/^local-user-1234-[A-Za-z0-9_-]+-[a-z0-9]+$/); + expect(secondId).toMatch(/^local-user-1234-[A-Za-z0-9_-]+-[a-z0-9]+$/); expect(firstId).not.toBe(secondId); } finally { now.mockRestore(); diff --git a/tests/features/chat/local-id.test.ts b/tests/features/chat/local-id.test.ts new file mode 100644 index 00000000..2948aa07 --- /dev/null +++ b/tests/features/chat/local-id.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; + +import { + createLocalChatItemIdFactory, + isLocalSteerMessageClientId, + isLocalUserMessageId, +} from "../../../src/features/chat/domain/local-id"; + +describe("local chat item ids", () => { + it("generates monotonic IDs per factory with a stable prefix and timestamp", () => { + const ids = createLocalChatItemIdFactory({ nowMs: () => 1234, seed: "test" }); + const first = ids.next("local-user"); + const second = ids.next("local-user"); + const third = ids.next("system"); + + expect(first).toMatch(/^local-user-1234-test-[a-z0-9]+-1$/); + expect(second).toMatch(/^local-user-1234-test-[a-z0-9]+-2$/); + expect(third).toMatch(/^system-1234-test-[a-z0-9]+-3$/); + }); + + it("keeps factories distinct when they share a timestamp and seed", () => { + const first = createLocalChatItemIdFactory({ nowMs: () => 1234, seed: "test" }); + const second = createLocalChatItemIdFactory({ nowMs: () => 1234, seed: "test" }); + + expect(first.next("local-user")).not.toBe(second.next("local-user")); + }); + + it("classifies local prompt and steer message identifiers", () => { + expect(isLocalUserMessageId("local-user-1234-test-1-1")).toBe(true); + expect(isLocalUserMessageId("local-steer-1234-test-1-1")).toBe(true); + expect(isLocalUserMessageId("user-1")).toBe(false); + expect(isLocalSteerMessageClientId("local-steer-1234-test-1-1")).toBe(true); + expect(isLocalSteerMessageClientId("local-user-1234-test-1-1")).toBe(false); + }); +}); diff --git a/tests/features/chat/view-connection.test.ts b/tests/features/chat/view-connection.test.ts index 86cda053..3019c2c5 100644 --- a/tests/features/chat/view-connection.test.ts +++ b/tests/features/chat/view-connection.test.ts @@ -451,7 +451,7 @@ describe("CodexChatView connection lifecycle", () => { threadId: "thread-1", cwd: "/vault", input: [{ type: "text", text: "hello" }], - clientUserMessageId: expect.stringMatching(/^local-user-\d+-\d+$/), + clientUserMessageId: expect.stringMatching(/^local-user-\d+-[A-Za-z0-9_-]+-[a-z0-9]+$/), }); }); expect(view.getState()).toEqual({ version: 1, threadId: "thread-1", threadTitle: "Restored thread" });