Skip to content

Follow-up: remove local thread-name override persistence #456

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Dimillian wants to merge 1 commit into from
+56 −87
Closed

Follow-up: remove local thread-name override persistence #456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 2 additions & 28 deletions src/features/threads/hooks/useThreadStorage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
import { act, renderHook, waitFor } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import {
STORAGE_KEY_CUSTOM_NAMES,
STORAGE_KEY_PINNED_THREADS,
loadCustomNames,
loadPinnedThreads,
loadThreadActivity,
savePinnedThreads,
Expand All @@ -14,13 +12,9 @@ import { useThreadStorage } from "./useThreadStorage";

vi.mock("@threads/utils/threadStorage", () => ({
MAX_PINS_SOFT_LIMIT: 2,
STORAGE_KEY_CUSTOM_NAMES: "custom-names",
STORAGE_KEY_PINNED_THREADS: "pinned-threads",
loadCustomNames: vi.fn(),
loadPinnedThreads: vi.fn(),
loadThreadActivity: vi.fn(),
makeCustomNameKey: (workspaceId: string, threadId: string) =>
`${workspaceId}:${threadId}`,
makePinKey: (workspaceId: string, threadId: string) =>
`${workspaceId}:${threadId}`,
savePinnedThreads: vi.fn(),
Expand All @@ -32,15 +26,11 @@ describe("useThreadStorage", () => {
vi.clearAllMocks();
});

it("loads initial data and updates custom names on storage events", async () => {
it("loads initial data and does not provide custom names", () => {
vi.mocked(loadThreadActivity).mockReturnValue({
"ws-1": { "thread-1": 101 },
});
vi.mocked(loadPinnedThreads).mockReturnValue({ "ws-1:thread-1": 202 });
vi
.mocked(loadCustomNames)
.mockReturnValueOnce({ "ws-1:thread-1": "Custom" })
.mockReturnValueOnce({ "ws-1:thread-1": "Updated" });

const { result } = renderHook(() => useThreadStorage());

Expand All @@ -50,26 +40,12 @@ describe("useThreadStorage", () => {
expect(result.current.pinnedThreadsRef.current).toEqual({
"ws-1:thread-1": 202,
});

await waitFor(() => {
expect(result.current.getCustomName("ws-1", "thread-1")).toBe("Custom");
});

act(() => {
window.dispatchEvent(
new StorageEvent("storage", { key: STORAGE_KEY_CUSTOM_NAMES }),
);
});

await waitFor(() => {
expect(result.current.getCustomName("ws-1", "thread-1")).toBe("Updated");
});
expect(result.current.getCustomName("ws-1", "thread-1")).toBeUndefined();
});

it("records thread activity and persists updates", () => {
vi.mocked(loadThreadActivity).mockReturnValue({});
vi.mocked(loadPinnedThreads).mockReturnValue({});
vi.mocked(loadCustomNames).mockReturnValue({});

const { result } = renderHook(() => useThreadStorage());

Expand All @@ -88,7 +64,6 @@ describe("useThreadStorage", () => {
it("pins and unpins threads while updating persistence", () => {
vi.mocked(loadThreadActivity).mockReturnValue({});
vi.mocked(loadPinnedThreads).mockReturnValue({});
vi.mocked(loadCustomNames).mockReturnValue({});

const { result } = renderHook(() => useThreadStorage());

Expand Down Expand Up @@ -117,7 +92,6 @@ describe("useThreadStorage", () => {
it("ignores duplicate pins and reacts to pinned storage changes", async () => {
vi.mocked(loadThreadActivity).mockReturnValue({});
vi.mocked(loadPinnedThreads).mockReturnValue({ "ws-1:thread-1": 123 });
vi.mocked(loadCustomNames).mockReturnValue({});

const { result } = renderHook(() => useThreadStorage());

Expand Down
27 changes: 1 addition & 26 deletions src/features/threads/hooks/useThreadStorage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,17 @@ import { useCallback, useEffect, useRef, useState } from "react";
import type { MutableRefObject } from "react";
import {
MAX_PINS_SOFT_LIMIT,
STORAGE_KEY_CUSTOM_NAMES,
STORAGE_KEY_PINNED_THREADS,
type CustomNamesMap,
type PinnedThreadsMap,
type ThreadActivityMap,
loadCustomNames,
loadPinnedThreads,
loadThreadActivity,
makeCustomNameKey,
makePinKey,
savePinnedThreads,
saveThreadActivity,
} from "@threads/utils/threadStorage";

type UseThreadStorageResult = {
customNamesRef: MutableRefObject<CustomNamesMap>;
pinnedThreadsRef: MutableRefObject<PinnedThreadsMap>;
threadActivityRef: MutableRefObject<ThreadActivityMap>;
pinnedThreadsVersion: number;
Expand All @@ -37,26 +32,7 @@ export function useThreadStorage(): UseThreadStorageResult {
const threadActivityRef = useRef<ThreadActivityMap>(loadThreadActivity());
const pinnedThreadsRef = useRef<PinnedThreadsMap>(loadPinnedThreads());
const [pinnedThreadsVersion, setPinnedThreadsVersion] = useState(0);
const customNamesRef = useRef<CustomNamesMap>({});

useEffect(() => {
if (typeof window === "undefined") {
return undefined;
}
customNamesRef.current = loadCustomNames();
const handleStorage = (event: StorageEvent) => {
if (event.key === STORAGE_KEY_CUSTOM_NAMES) {
customNamesRef.current = loadCustomNames();
}
};
window.addEventListener("storage", handleStorage);
return () => window.removeEventListener("storage", handleStorage);
}, []);

const getCustomName = useCallback((workspaceId: string, threadId: string) => {
const key = makeCustomNameKey(workspaceId, threadId);
return customNamesRef.current[key];
}, []);
const getCustomName = useCallback((_workspaceId: string, _threadId: string) => undefined, []);
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep manual rename signal for title autogeneration guard

Returning undefined unconditionally from getCustomName removes the cancellation signal that useThreadTitleAutogeneration.onUserMessageCreated relies on before and after generateRunMetadata to avoid overwriting user-renamed threads. In the race where a user renames a thread while metadata generation is in flight, the post-await guard no longer trips and renameThread(..., title) can clobber the manual name when the async call resolves; this is a regression from the previous behavior where renameThread immediately updated custom-name state.

Useful? React with 👍 / 👎.


const recordThreadActivity = useCallback(
(workspaceId: string, threadId: string, timestamp = Date.now()) => {
Expand Down Expand Up @@ -138,7 +114,6 @@ export function useThreadStorage(): UseThreadStorageResult {
);

return {
customNamesRef,
pinnedThreadsRef,
threadActivityRef,
pinnedThreadsVersion,
Expand Down
78 changes: 52 additions & 26 deletions src/features/threads/hooks/useThreads.integration.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1114,33 +1114,59 @@ describe("useThreads UX integration", () => {
expect(localStorage.getItem(STORAGE_KEY_DETACHED_REVIEW_LINKS)).toBeNull();
});

it("orders thread lists, applies custom names, and keeps pin ordering stable", async () => {
it("orders thread lists, reflects codex thread names, and keeps pin ordering stable", async () => {
const listThreadsMock = vi.mocked(listThreads);
listThreadsMock.mockResolvedValue({
result: {
data: [
{
id: "thread-a",
preview: "Alpha",
updated_at: 1000,
cwd: workspace.path,
},
{
id: "thread-b",
preview: "Beta",
updated_at: 3000,
cwd: workspace.path,
},
{
id: "thread-c",
preview: "Gamma",
updated_at: 2000,
cwd: workspace.path,
},
],
nextCursor: null,
},
});
listThreadsMock
.mockResolvedValueOnce({
result: {
data: [
{
id: "thread-a",
preview: "Alpha",
updated_at: 1000,
cwd: workspace.path,
},
{
id: "thread-b",
preview: "Beta",
updated_at: 3000,
cwd: workspace.path,
},
{
id: "thread-c",
preview: "Gamma",
updated_at: 2000,
cwd: workspace.path,
},
],
nextCursor: null,
},
})
.mockResolvedValueOnce({
result: {
data: [
{
id: "thread-a",
preview: "Alpha",
updated_at: 1000,
cwd: workspace.path,
},
{
id: "thread-b",
preview: "Custom Beta",
updated_at: 3000,
cwd: workspace.path,
},
{
id: "thread-c",
preview: "Gamma",
updated_at: 2000,
cwd: workspace.path,
},
],
nextCursor: null,
},
});

const { result } = renderHook(() =>
useThreads({
Expand Down
8 changes: 1 addition & 7 deletions src/features/threads/hooks/useThreads.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ import { useThreadTitleAutogeneration } from "./useThreadTitleAutogeneration";
import { setThreadName as setThreadNameService } from "@services/tauri";
import {
loadDetachedReviewLinks,
makeCustomNameKey,
saveCustomName,
saveDetachedReviewLinks,
} from "@threads/utils/threadStorage";
import { getParentThreadIdFromSource } from "@threads/utils/threadRpc";
Expand Down Expand Up @@ -103,7 +101,6 @@ export function useThreads({
useThreadApprovals({ dispatch, onDebug });
const { handleUserInputSubmit } = useThreadUserInput({ dispatch });
const {
customNamesRef,
threadActivityRef,
pinnedThreadsVersion,
getCustomName,
Expand Down Expand Up @@ -162,9 +159,6 @@ export function useThreads({

const renameThread = useCallback(
(workspaceId: string, threadId: string, newName: string) => {
saveCustomName(workspaceId, threadId, newName);
const key = makeCustomNameKey(workspaceId, threadId);
customNamesRef.current[key] = newName;
dispatch({ type: "setThreadName", workspaceId, threadId, name: newName });
void Promise.resolve(
setThreadNameService(workspaceId, threadId, newName),
Expand All @@ -178,7 +172,7 @@ export function useThreads({
});
});
},
[customNamesRef, dispatch, onDebug],
[dispatch, onDebug],
);

const onSubagentThreadDetected = useCallback(
Expand Down
Loading