Repository navigation
Feature #2112 Rename tasks in history #2122
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2450,6 +2450,33 @@ | |
| } | ||
| } | ||
|
|
||
| /* | ||
| * Sets a user-assigned display title for a history item. A blank title | ||
| * clears it so the history falls back to the original prompt. | ||
| */ | ||
| async renameTaskWithId(id: string, title: string | undefined) { | ||
| if (!this.taskHistoryStore.get(id)) { | ||
| this.log(`[renameTaskWithId] task ${id} not found in history`) | ||
| return | ||
| } | ||
|
|
||
| // Read and write under the store lock so a queued task save or delegation | ||
| // transition cannot be overwritten by a stale snapshot of the item. | ||
| const trimmed = title?.trim() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the title have a length cap? This only trims. |
||
| await this.taskHistoryStore.atomicReadAndUpdate(id, (historyItem) => ({ | ||
|
designPrj marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A question about stale caches. The store diffs this update against its cache, not against the file on disk. Suppose another window set a title on disk and this window's cache has none. Would a clear then give an empty delta, so the disk title survives while the rename reports success? The same weakness applies to every field, so a follow-up is fine. |
||
| ...historyItem, | ||
| title: trimmed ? trimmed : undefined, | ||
| })) | ||
| this.recentTasksCache = undefined | ||
|
|
||
| if (this.isViewLaunched) { | ||
|
Check warning on line 2472 in src/core/webview/ClineProvider.ts
|
||
| const updatedItem = this.taskHistoryStore.get(id) | ||
| if (updatedItem) { | ||
|
Check warning on line 2474 in src/core/webview/ClineProvider.ts
|
||
| await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem }) | ||
| } | ||
| } | ||
| } | ||
|
designPrj marked this conversation as resolved.
|
||
|
|
||
| /* Condenses a task's message history to use fewer tokens. */ | ||
| async condenseTaskContext(taskId: string) { | ||
| const task = this.taskRegistry.getById(taskId) | ||
|
|
@@ -3247,20 +3274,28 @@ | |
| * Updates a task in the task history and optionally broadcasts the updated history to the webview. | ||
| * Now delegates to TaskHistoryStore for per-task file persistence. | ||
| * | ||
| * This writer never owns the `title` field: `title` is written only by the rename path | ||
| * (`renameTaskWithId` via `taskHistoryStore.atomicReadAndUpdate`). Callers here build items | ||
| * from snapshots read before taking the store lock, so any title they carry can be stale. | ||
| * | ||
| * @param item The history item to update or add | ||
| * @param options.broadcast Whether to broadcast the updated history to the webview (default: true) | ||
| * @returns The updated task history array | ||
| */ | ||
| async updateTaskHistory(item: HistoryItem, options: { broadcast?: boolean } = {}): Promise<HistoryItem[]> { | ||
| const { broadcast = true } = options | ||
|
|
||
| const history = await this.taskHistoryStore.upsert(item) | ||
| // Drop the snapshot's title so upsertCore preserves the value held under the store lock | ||
| // instead of treating a stale title as a changed field and reverting a concurrent rename. | ||
| const { title: _title, ...itemWithoutTitle } = item | ||
|
|
||
| const history = await this.taskHistoryStore.upsert(itemWithoutTitle) | ||
| this.recentTasksCache = undefined | ||
|
|
||
| // Broadcast the updated history to the webview if requested. | ||
| // Prefer per-item updates to avoid repeatedly cloning/sending the full history. | ||
| if (broadcast && this.isViewLaunched) { | ||
| const updatedItem = this.taskHistoryStore.get(item.id) ?? item | ||
| const updatedItem = this.taskHistoryStore.get(item.id) ?? itemWithoutTitle | ||
|
Check warning on line 3298 in src/core/webview/ClineProvider.ts
|
||
| await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem }) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ import { useSelectedModel } from "@/components/ui/hooks/useSelectedModel" | |
| import { vscode } from "@src/utils/vscode" | ||
|
|
||
| import Thumbnails from "../common/Thumbnails" | ||
| import { TaskTitleEditor } from "../history/TaskTitleEditor" | ||
| import { TaskActions } from "./TaskActions" | ||
| import { ContextWindowProgress } from "./ContextWindowProgress" | ||
| import { Mention } from "./Mention" | ||
|
|
@@ -66,6 +67,8 @@ const TaskHeader = ({ | |
| const { apiConfiguration, currentTaskItem } = useExtensionState() | ||
| const { id: modelId, info: model } = useSelectedModel(apiConfiguration) | ||
| const [isTaskExpanded, setIsTaskExpanded] = useState(false) | ||
| const [isRenaming, setIsRenaming] = useState(false) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should |
||
| const customTitle = currentTaskItem?.title | ||
|
|
||
| const textContainerRef = useRef<HTMLDivElement>(null) | ||
| const textRef = useRef<HTMLDivElement>(null) | ||
|
|
@@ -179,10 +182,23 @@ const TaskHeader = ({ | |
| <div className="flex justify-between items-center gap-0"> | ||
| <div className="flex items-center select-none grow min-w-0"> | ||
| <div className="grow min-w-0"> | ||
| {isTaskExpanded && <span className="font-bold">{t("chat:task.title")}</span>} | ||
| {isTaskExpanded && | ||
| (isRenaming && currentTaskItem ? ( | ||
| <TaskTitleEditor | ||
| item={currentTaskItem} | ||
| onDone={() => setIsRenaming(false)} | ||
| className="text-sm" | ||
| /> | ||
| ) : ( | ||
| <span className="font-bold">{customTitle || t("chat:task.title")}</span> | ||
| ))} | ||
| {!isTaskExpanded && ( | ||
| <div className="flex items-center gap-2 whitespace-nowrap overflow-hidden text-ellipsis"> | ||
| <Mention text={task.text} /> | ||
| {customTitle ? ( | ||
| <span className="overflow-hidden text-ellipsis">{customTitle}</span> | ||
| ) : ( | ||
| <Mention text={task.text} /> | ||
| )} | ||
| </div> | ||
| )} | ||
| </div> | ||
|
|
@@ -336,7 +352,11 @@ const TaskHeader = ({ | |
| {task.images && task.images.length > 0 && <Thumbnails images={task.images} />} | ||
|
|
||
| <div onClick={(e) => e.stopPropagation()}> | ||
| <TaskActions item={currentTaskItem} buttonsDisabled={buttonsDisabled} /> | ||
| <TaskActions | ||
| item={currentTaskItem} | ||
| buttonsDisabled={buttonsDisabled} | ||
| onRename={isRenaming ? undefined : () => setIsRenaming(true)} | ||
| /> | ||
| </div> | ||
|
|
||
| <div className="pt-3 mt-2 -mx-2.5 px-2.5 border-t border-vscode-sideBar-background"> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This checks that the task exists before the store lock. If the task is deleted before
atomicReadAndUpdateruns, that call throws "not found in cache". I could not see a catch in therenameTaskWithIdcase of the message handler. Should this catch and log the error, so a rename of a just-deleted task does not become an unhandled rejection?