GithubMirror/continue
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: 🐛 Fixes based on reviewer comments

Browse Source
This commit is contained in:
chezsmithy
2025-08-27 23:48:05 -07:00
parent 58cb7815ca
commit acbba9b7d0
10 changed files with 64 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
core/core.ts
View File

@@ -1056,11 +1056,11 @@ export class Core {
},
);
on("process/cancelTerminalCommand", async ({ data: { toolCallId } }) => {
const { cancelTerminalCommand } = await import(
on("process/killTerminalProcess", async ({ data: { toolCallId } }) => {
const { killTerminalProcess } = await import(
"./util/processTerminalStates.js"
);
await cancelTerminalCommand(toolCallId);
await killTerminalProcess(toolCallId);
});
on("mdm/setLicenseKey", ({ data: { licenseKey } }) => {

2
core/protocol/core.ts
View File

@@ -313,6 +313,6 @@ export type ToCoreFromIdeOrWebviewProtocol = {
];
"process/markAsBackgrounded": [{ toolCallId: string }, void];
"process/isBackgrounded": [{ toolCallId: string }, boolean];
"process/cancelTerminalCommand": [{ toolCallId: string }, void];
"process/killTerminalProcess": [{ toolCallId: string }, void];
"mdm/setLicenseKey": [{ licenseKey: string }, boolean];
};

2
core/protocol/passThrough.ts
View File

@@ -86,7 +86,7 @@ export const WEBVIEW_TO_CORE_PASS_THROUGH: (keyof ToCoreFromWebviewProtocol)[] =
"isItemTooBig",
"process/markAsBackgrounded",
"process/isBackgrounded",
"process/cancelTerminalCommand",
"process/killTerminalProcess",
];
// Message types to pass through from core to webview

8
core/tools/implementations/runTerminalCommand.vitest.ts
View File

@@ -39,12 +39,8 @@ describe("runTerminalCommandImpl", () => {
vi.resetAllMocks();
// Setup backgrounded processes handling - don't mock, just make sure it's empty
// Get any processes that might be already tracked and clear them
const processMap = (processTerminalStates as any)
.processTerminalBackgroundStates;
if (processMap && typeof processMap.clear === "function") {
processMap.clear();
}
// Clear any processes that might be already tracked
processTerminalStates.clearAllBackgroundProcesses();
// Setup IDE mocks
mockGetIdeInfo.mockReturnValue(Promise.resolve({ remoteName: "local" }));

39
core/util/processTerminalStates.test.ts
View File

@@ -1,13 +1,14 @@
import { ChildProcess } from "child_process";
import {
cancelAllRunningTerminalCommands,
cancelMultipleTerminalCommands,
cancelTerminalCommand,
clearAllBackgroundProcesses,
getAllBackgroundedProcessIds,
getAllRunningProcessIds,
getRunningProcess,
isProcessBackgrounded,
isProcessRunning,
killAllRunningTerminalProcesses,
killMultipleTerminalProcesses,
killTerminalProcess,
markProcessAsBackgrounded,
markProcessAsRunning,
removeBackgroundedProcess,
@@ -205,7 +206,7 @@ describe("processTerminalStates", () => {
markProcessAsRunning(toolCallId, mockProcess);
expect(isProcessRunning(toolCallId)).toBe(true);
await cancelTerminalCommand(toolCallId);
await killTerminalProcess(toolCallId);
expect(mockProcess.kill).toHaveBeenCalledWith("SIGTERM");
expect(isProcessRunning(toolCallId)).toBe(false);
@@ -214,7 +215,7 @@ describe("processTerminalStates", () => {
test("should handle cancelling non-existent process", async () => {
const toolCallId = "non-existent";
await expect(cancelTerminalCommand(toolCallId)).resolves.not.toThrow();
await expect(killTerminalProcess(toolCallId)).resolves.not.toThrow();
});
test("should handle cancelling already killed process", async () => {
@@ -223,7 +224,7 @@ describe("processTerminalStates", () => {
markProcessAsRunning(toolCallId, mockProcess);
await cancelTerminalCommand(toolCallId);
await killTerminalProcess(toolCallId);
expect(mockProcess.kill).not.toHaveBeenCalled();
});
@@ -242,7 +243,7 @@ describe("processTerminalStates", () => {
markProcessAsRunning(toolCallId, mockProcess);
const cancelPromise = cancelTerminalCommand(toolCallId);
const cancelPromise = killTerminalProcess(toolCallId);
// Fast-forward time to trigger the timeout
jest.advanceTimersByTime(5000);
@@ -259,7 +260,7 @@ describe("processTerminalStates", () => {
markProcessAsRunning(toolCallId, mockProcess);
const cancelPromise = cancelTerminalCommand(toolCallId);
const cancelPromise = killTerminalProcess(toolCallId);
// Fast-forward time to trigger the timeout
jest.advanceTimersByTime(5000);
@@ -314,7 +315,7 @@ describe("processTerminalStates", () => {
markProcessAsRunning(toolCallId1, mockProcess1);
markProcessAsRunning(toolCallId2, mockProcess2);
await cancelMultipleTerminalCommands([toolCallId1, toolCallId2]);
await killMultipleTerminalProcesses([toolCallId1, toolCallId2]);
expect(mockProcess1.kill).toHaveBeenCalledWith("SIGTERM");
expect(mockProcess2.kill).toHaveBeenCalledWith("SIGTERM");
@@ -331,7 +332,7 @@ describe("processTerminalStates", () => {
markProcessAsRunning(toolCallId1, mockProcess1);
markProcessAsRunning(toolCallId2, mockProcess2);
const cancelledIds = await cancelAllRunningTerminalCommands();
const cancelledIds = await killAllRunningTerminalProcesses();
expect(cancelledIds).toContain(toolCallId1);
expect(cancelledIds).toContain(toolCallId2);
@@ -343,8 +344,24 @@ describe("processTerminalStates", () => {
});
test("should return empty array when no running commands to cancel", async () => {
const cancelledIds = await cancelAllRunningTerminalCommands();
const cancelledIds = await killAllRunningTerminalProcesses();
expect(cancelledIds).toEqual([]);
});
test("should clear all background processes", () => {
const toolCallId1 = "test-123";
const toolCallId2 = "test-456";
markProcessAsBackgrounded(toolCallId1);
markProcessAsBackgrounded(toolCallId2);
expect(getAllBackgroundedProcessIds()).toHaveLength(2);
clearAllBackgroundProcesses();
expect(getAllBackgroundedProcessIds()).toEqual([]);
expect(isProcessBackgrounded(toolCallId1)).toBe(false);
expect(isProcessBackgrounded(toolCallId2)).toBe(false);
});
});
});

15
core/util/processTerminalStates.ts
View File

@@ -67,7 +67,7 @@ export function removeRunningProcess(toolCallId: string): void {
processTerminalForegroundStates.delete(toolCallId);
}
export async function cancelTerminalCommand(toolCallId: string): Promise<void> {
export async function killTerminalProcess(toolCallId: string): Promise<void> {
const processInfo = processTerminalForegroundStates.get(toolCallId);
if (processInfo && !processInfo.process.killed) {
const { process } = processInfo;
@@ -86,20 +86,20 @@ export async function cancelTerminalCommand(toolCallId: string): Promise<void> {
}
// Function to cancel multiple terminal commands at once
export async function cancelMultipleTerminalCommands(
export async function killMultipleTerminalProcesses(
toolCallIds: string[],
): Promise<void> {
const cancelPromises = toolCallIds.map((toolCallId) =>
cancelTerminalCommand(toolCallId),
killTerminalProcess(toolCallId),
);
await Promise.all(cancelPromises);
}
// Function to cancel ALL currently running terminal commands
export async function cancelAllRunningTerminalCommands(): Promise<string[]> {
export async function killAllRunningTerminalProcesses(): Promise<string[]> {
const runningIds = getAllRunningProcessIds();
if (runningIds.length > 0) {
await cancelMultipleTerminalCommands(runningIds);
await killMultipleTerminalProcesses(runningIds);
}
return runningIds; // Return the IDs that were cancelled
}
@@ -112,3 +112,8 @@ export function getAllRunningProcessIds(): string[] {
export function getAllBackgroundedProcessIds(): string[] {
return Array.from(processTerminalBackgroundStates.keys());
}
// Utility function for testing - clears all background process states
export function clearAllBackgroundProcesses(): void {
processTerminalBackgroundStates.clear();
}

3
extensions/intellij/src/main/kotlin/com/github/continuedev/continueintellijextension/constants/MessageTypes.kt
View File

@@ -144,10 +144,9 @@ class MessageTypes {
"tools/evaluatePolicy",
"controlPlane/openUrl",
"isItemTooBig",
"controlPlane/openUrl",
"process/markAsBackgrounded",
"process/isBackgrounded",
"process/cancelTerminalCommand",
"process/killTerminalProcess"
)
}
}

9
gui/src/components/mainInput/Lump/LumpToolbar/LumpToolbar.tsx
View File

@@ -19,6 +19,7 @@ import { PendingApplyStatesToolbar } from "./PendingApplyStatesToolbar";
import { PendingToolCallToolbar } from "./PendingToolCallToolbar";
import { StreamingToolbar } from "./StreamingToolbar";
import { TtsActiveToolbar } from "./TtsActiveToolbar";
import { BuiltInToolNames } from "core/tools/builtIn";
// Keyboard shortcut detection utilities
const isExecuteToolCallShortcut = (event: KeyboardEvent) => {
@@ -38,11 +39,7 @@ const isCancelToolCallShortcut = (
// Check if a tool call is a terminal command
const isTerminalCommand = (toolCallState: any) => {
return (
toolCallState?.toolCall?.function?.name === "run_terminal_command" ||
toolCallState?.toolCall?.function?.name === "Bash" ||
toolCallState?.toolCall?.function?.name === "runTerminalCommand"
);
return BuiltInToolNames.RunTerminalCommand;
};
export function LumpToolbar() {
@@ -82,7 +79,7 @@ export function LumpToolbar() {
const stopPromises = runningTerminalCalls.map(async (terminalCall) => {
try {
// Cancel the process on the backend
await ideMessenger.request("process/cancelTerminalCommand", {
await ideMessenger.request("process/killTerminalProcess", {
toolCallId: terminalCall.toolCallId,
});

13
gui/src/components/mainInput/Lump/LumpToolbar/StreamingToolbar.tsx
View File

@@ -1,26 +1,19 @@
import { useAppDispatch } from "../../../../redux/hooks";
import { cancelStream } from "../../../../redux/thunks/cancelStream";
import { getAltKeyLabel, getMetaKeyLabel, isJetBrains } from "../../../../util";
import { GeneratingIndicator } from "./GeneratingIndicator";
interface StreamingToolbarProps {
onStop?: () => void;
onStop: () => void;
displayText?: string;
}
export function StreamingToolbar({
onStop,
displayText = "Stop",
}: StreamingToolbarProps = {}) {
}: StreamingToolbarProps) {
const jetbrains = isJetBrains();
const dispatch = useAppDispatch();
const handleStop = () => {
if (onStop) {
onStop();
} else {
void dispatch(cancelStream());
}
onStop();
};
return (

26
gui/src/redux/util/constructMessages.ts
View File

@@ -17,26 +17,24 @@ import {
import { convertToolCallStatesToSystemCallsAndOutput } from "core/tools/systemMessageTools/convertSystemTools";
import { SystemMessageToolsFramework } from "core/tools/systemMessageTools/types";
import { findLast, findLastIndex } from "core/util/findLast";
import {
normalizeToMessageParts,
renderContextItems,
} from "core/util/messageContent";
import { normalizeToMessageParts } from "core/util/messageContent";
import { toolCallStateToContextItems } from "../../pages/gui/ToolCallDiv/utils";
// Helper function to render context items and append status information
// Helper function to render context items and append status information
function renderContextItemsWithStatus(contextItems: any[]): string {
const content = renderContextItems(contextItems);
return contextItems
.map((item) => {
let result = item.content;
// Check if any context items have status information
const statusItems = contextItems
.filter((item) => item.status)
.map((item) => `Status: ${item.status}`);
// If this item has a status, append it directly after the content
if (item.status) {
result += `\n[Status: ${item.status}]`;
}
if (statusItems.length > 0) {
return `${content}\n\n${statusItems.join("\n")}`;
}
return content;
return result;
})
.join("\n\n");
}
interface MessageWithContextItems {
ctxItems: ContextItemWithId[];