Unverified 提交 72050d9e authored 作者: Will Chen's avatar Will Chen 提交者: GitHub

Make undo more robust by using sourceCommitHash (#2015)

<!-- CURSOR_SUMMARY --> > [!NOTE] > Strengthens undo to work even when an assistant message produced no code. > > - Use the last assistant message’s `sourceCommitHash` for `revertVersion` and pass `currentChatMessageId` to prune messages at/after the triggering user message; refresh chat state > - Backend `revert-version` now conditionally commits only if there are staged changes and supports message deletion via `gte` with `currentChatMessageId`, falling back to commit-hash-based pruning > - Extend IPC types: add `Message.sourceCommitHash` and `RevertVersionParams.currentChatMessageId` > - Add e2e test and fixture for undo after a no-code assistant response > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit a97e153d3cb703461b66bb7eaec28b4c7ae32cc4. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Make undo reliable by reverting to the message’s source commit instead of relying on a previous assistant message or the chat’s initial state. This fixes undo when an assistant reply contains no code. - **Bug Fixes** - Use current message’s sourceCommitHash for revertVersion and pass currentChatMessageId to prune messages at/after the triggering user message; then refresh chat. - Extend Message type with sourceCommitHash and show a warning if it’s missing. - Add e2e test for undo after a no-code assistant response with a new fixture. <sup>Written for commit a97e153d3cb703461b66bb7eaec28b4c7ae32cc4. Summary will update automatically on new commits.</sup> <!-- End of auto-generated description by cubic. -->
上级 c4ee578c
I understand your request. This is a response without any code changes.
...@@ -40,3 +40,29 @@ testSkipIfWindows("undo", async ({ po }) => { ...@@ -40,3 +40,29 @@ testSkipIfWindows("undo", async ({ po }) => {
testSkipIfWindows("undo with native git", async ({ po }) => { testSkipIfWindows("undo with native git", async ({ po }) => {
await runUndoTest(po, true); await runUndoTest(po, true);
}); });
testSkipIfWindows("undo after assistant with no code", async ({ po }) => {
await po.setUp({ autoApprove: true, nativeGit: false });
// First prompt - no code generated
await po.sendPrompt("tc=no-code-response");
// Second prompt - generates code
await po.sendPrompt("tc=write-index");
const iframe = po.getPreviewIframeElement();
await expect(
iframe.contentFrame().getByText("Testing:write-index!"),
).toBeVisible({
timeout: Timeout.LONG,
});
// Undo should work even though first assistant had no commit
await po.clickUndo();
await expect(
iframe.contentFrame().getByText("Welcome to Your Blank App"),
).toBeVisible({
timeout: Timeout.LONG,
});
});
...@@ -88,8 +88,7 @@ function FooterComponent({ context }: { context?: FooterContext }) { ...@@ -88,8 +88,7 @@ function FooterComponent({ context }: { context?: FooterContext }) {
{!isStreaming && ( {!isStreaming && (
<div className="flex max-w-3xl mx-auto gap-2"> <div className="flex max-w-3xl mx-auto gap-2">
{!!messages.length && {!!messages.length &&
messages[messages.length - 1].role === "assistant" && messages[messages.length - 1].role === "assistant" && (
messages[messages.length - 1].commitHash && (
<Button <Button
variant="outline" variant="outline"
size="sm" size="sm"
...@@ -102,51 +101,34 @@ function FooterComponent({ context }: { context?: FooterContext }) { ...@@ -102,51 +101,34 @@ function FooterComponent({ context }: { context?: FooterContext }) {
setIsUndoLoading(true); setIsUndoLoading(true);
try { try {
if (messages.length >= 3) { const currentMessage = messages[messages.length - 1];
const previousAssistantMessage = // The user message that triggered this assistant response
messages[messages.length - 3]; const userMessage = messages[messages.length - 2];
if ( if (currentMessage?.sourceCommitHash) {
previousAssistantMessage?.role === "assistant" && console.debug(
previousAssistantMessage?.commitHash "Reverting to source commit hash",
) { currentMessage.sourceCommitHash,
console.debug( );
"Reverting to previous assistant version", await revertVersion({
); versionId: currentMessage.sourceCommitHash,
await revertVersion({ currentChatMessageId: userMessage
versionId: previousAssistantMessage.commitHash, ? {
}); chatId: selectedChatId,
const chat = messageId: userMessage.id,
await IpcClient.getInstance().getChat(selectedChatId); }
setMessagesById((prev) => { : undefined,
const next = new Map(prev); });
next.set(selectedChatId, chat.messages);
return next;
});
}
} else {
const chat = const chat =
await IpcClient.getInstance().getChat(selectedChatId); await IpcClient.getInstance().getChat(selectedChatId);
if (chat.initialCommitHash) { setMessagesById((prev) => {
await revertVersion({ const next = new Map(prev);
versionId: chat.initialCommitHash, next.set(selectedChatId, chat.messages);
}); return next;
try { });
await IpcClient.getInstance().deleteMessages( } else {
selectedChatId, showWarning(
); "No source commit hash found for message. Need to manually undo code changes",
setMessagesById((prev) => { );
const next = new Map(prev);
next.set(selectedChatId, []);
return next;
});
} catch (err) {
showError(err);
}
} else {
showWarning(
"No initial commit hash found for chat. Need to manually undo code changes",
);
}
} }
} catch (error) { } catch (error) {
console.error("Error during undo operation:", error); console.error("Error during undo operation:", error);
......
...@@ -42,9 +42,18 @@ export function useVersions(appId: number | null) { ...@@ -42,9 +42,18 @@ export function useVersions(appId: number | null) {
const revertVersionMutation = useMutation< const revertVersionMutation = useMutation<
RevertVersionResponse, RevertVersionResponse,
Error, Error,
{ versionId: string } {
versionId: string;
currentChatMessageId?: { chatId: number; messageId: number };
}
>({ >({
mutationFn: async ({ versionId }: { versionId: string }) => { mutationFn: async ({
versionId,
currentChatMessageId,
}: {
versionId: string;
currentChatMessageId?: { chatId: number; messageId: number };
}) => {
const currentAppId = appId; const currentAppId = appId;
if (currentAppId === null) { if (currentAppId === null) {
throw new Error("App ID is null"); throw new Error("App ID is null");
...@@ -53,6 +62,7 @@ export function useVersions(appId: number | null) { ...@@ -53,6 +62,7 @@ export function useVersions(appId: number | null) {
return ipcClient.revertVersion({ return ipcClient.revertVersion({
appId: currentAppId, appId: currentAppId,
previousVersionId: versionId, previousVersionId: versionId,
currentChatMessageId,
}); });
}, },
onSuccess: async (result) => { onSuccess: async (result) => {
......
import { db } from "../../db"; import { db } from "../../db";
import { apps, messages, versions } from "../../db/schema"; import { apps, messages, versions } from "../../db/schema";
import { desc, eq, and, gt } from "drizzle-orm"; import { desc, eq, and, gt, gte } from "drizzle-orm";
import type { import type {
Version, Version,
BranchResult, BranchResult,
...@@ -23,6 +23,7 @@ import { ...@@ -23,6 +23,7 @@ import {
getCurrentCommitHash, getCurrentCommitHash,
gitCurrentBranch, gitCurrentBranch,
gitLog, gitLog,
isGitStatusClean,
} from "../utils/git_utils"; } from "../utils/git_utils";
import { import {
...@@ -156,7 +157,7 @@ export function registerVersionHandlers() { ...@@ -156,7 +157,7 @@ export function registerVersionHandlers() {
"revert-version", "revert-version",
async ( async (
_, _,
{ appId, previousVersionId }: RevertVersionParams, { appId, previousVersionId, currentChatMessageId }: RevertVersionParams,
): Promise<RevertVersionResponse> => { ): Promise<RevertVersionResponse> => {
return withLock(appId, async () => { return withLock(appId, async () => {
let successMessage = "Restored version"; let successMessage = "Restored version";
...@@ -193,48 +194,76 @@ export function registerVersionHandlers() { ...@@ -193,48 +194,76 @@ export function registerVersionHandlers() {
path: appPath, path: appPath,
targetOid: previousVersionId, targetOid: previousVersionId,
}); });
const isClean = await isGitStatusClean({ path: appPath });
if (!isClean) {
await gitCommit({
path: appPath,
message: `Reverted all changes back to version ${previousVersionId}`,
});
}
await gitCommit({ // Delete messages based on currentChatMessageId if provided, otherwise use commit hash lookup
path: appPath, if (currentChatMessageId) {
message: `Reverted all changes back to version ${previousVersionId}`, // Delete all messages including and after the specified message
}); const { chatId, messageId } = currentChatMessageId;
// Find the chat and message associated with the commit hash
const messageWithCommit = await db.query.messages.findFirst({
where: eq(messages.commitHash, previousVersionId),
with: {
chat: true,
},
});
// If we found a message with this commit hash, delete all subsequent messages (but keep this message)
if (messageWithCommit) {
const chatId = messageWithCommit.chatId;
// Find all messages in this chat with IDs > the one with our commit hash
const messagesToDelete = await db.query.messages.findMany({ const messagesToDelete = await db.query.messages.findMany({
where: and( where: and(
eq(messages.chatId, chatId), eq(messages.chatId, chatId),
gt(messages.id, messageWithCommit.id), gte(messages.id, messageId),
), ),
orderBy: desc(messages.id), orderBy: desc(messages.id),
}); });
logger.log( logger.log(
`Deleting ${messagesToDelete.length} messages after commit ${previousVersionId} from chat ${chatId}`, `Deleting ${messagesToDelete.length} messages (id >= ${messageId}) from chat ${chatId}`,
); );
// Delete the messages
if (messagesToDelete.length > 0) { if (messagesToDelete.length > 0) {
await db await db
.delete(messages) .delete(messages)
.where( .where(
and( and(eq(messages.chatId, chatId), gte(messages.id, messageId)),
eq(messages.chatId, chatId),
gt(messages.id, messageWithCommit.id),
),
); );
} }
} else {
// Find the chat and message associated with the commit hash
const messageWithCommit = await db.query.messages.findFirst({
where: eq(messages.commitHash, previousVersionId),
with: {
chat: true,
},
});
// If we found a message with this commit hash, delete all subsequent messages (but keep this message)
if (messageWithCommit) {
const chatId = messageWithCommit.chatId;
// Find all messages in this chat with IDs > the one with our commit hash
const messagesToDelete = await db.query.messages.findMany({
where: and(
eq(messages.chatId, chatId),
gt(messages.id, messageWithCommit.id),
),
orderBy: desc(messages.id),
});
logger.log(
`Deleting ${messagesToDelete.length} messages after commit ${previousVersionId} from chat ${chatId}`,
);
// Delete the messages
if (messagesToDelete.length > 0) {
await db
.delete(messages)
.where(
and(
eq(messages.chatId, chatId),
gt(messages.id, messageWithCommit.id),
),
);
}
}
} }
if (app.neonProjectId && app.neonDevelopmentBranchId) { if (app.neonProjectId && app.neonDevelopmentBranchId) {
......
...@@ -80,6 +80,7 @@ export interface Message { ...@@ -80,6 +80,7 @@ export interface Message {
content: string; content: string;
approvalState?: "approved" | "rejected" | null; approvalState?: "approved" | "rejected" | null;
commitHash?: string | null; commitHash?: string | null;
sourceCommitHash?: string | null;
dbTimestamp?: string | null; dbTimestamp?: string | null;
createdAt?: Date | string; createdAt?: Date | string;
requestId?: string | null; requestId?: string | null;
...@@ -442,6 +443,10 @@ export interface GetNeonProjectResponse { ...@@ -442,6 +443,10 @@ export interface GetNeonProjectResponse {
export interface RevertVersionParams { export interface RevertVersionParams {
appId: number; appId: number;
previousVersionId: string; previousVersionId: string;
currentChatMessageId?: {
chatId: number;
messageId: number;
};
} }
export type RevertVersionResponse = export type RevertVersionResponse =
......
Markdown 格式
0%
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论