Skip to content

Commit 7969ba2

Browse files
authored
Remove WeakRef usage (RooCodeInc#2811)
* Remove controllerRef * Remove webviewProviderRef * Remove controllerRef * Fix test * Create odd-jeans-wash.md * Fix test
1 parent 16c0992 commit 7969ba2

File tree

13 files changed

+183
-223
lines changed

13 files changed

+183
-223
lines changed

.changeset/odd-jeans-wash.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"claude-dev": patch
3+
---
4+
5+
Remove WeakRef usage

src/core/context/context-tracking/ContextTrackerTypes.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,3 @@ export interface TaskMetadata {
2121
files_in_context: FileMetadataEntry[]
2222
model_usage: ModelMetadataEntry[]
2323
}
24-
25-
// Interface for the controller to avoid direct dependency
26-
export interface ControllerLike {
27-
context: vscode.ExtensionContext
28-
}

src/core/context/context-tracking/FileContextTracker.test.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,10 @@ import * as vscode from "vscode"
55
import * as path from "path"
66
import { FileContextTracker } from "./FileContextTracker"
77
import * as diskModule from "../../storage/disk"
8-
import type { TaskMetadata, ControllerLike, FileMetadataEntry } from "./ContextTrackerTypes"
8+
import type { TaskMetadata, FileMetadataEntry } from "./ContextTrackerTypes"
99

1010
describe("FileContextTracker", () => {
1111
let sandbox: sinon.SinonSandbox
12-
let mockController: ControllerLike
1312
let mockContext: vscode.ExtensionContext
1413
let mockWorkspace: sinon.SinonStub
1514
let mockFileSystemWatcher: any
@@ -48,18 +47,14 @@ describe("FileContextTracker", () => {
4847
globalStorageUri: { fsPath: "/mock/storage" },
4948
} as unknown as vscode.ExtensionContext
5049

51-
mockController = {
52-
context: mockContext,
53-
}
54-
5550
// Mock disk module functions
5651
mockTaskMetadata = { files_in_context: [], model_usage: [] }
5752
getTaskMetadataStub = sandbox.stub(diskModule, "getTaskMetadata").resolves(mockTaskMetadata)
5853
saveTaskMetadataStub = sandbox.stub(diskModule, "saveTaskMetadata").resolves()
5954

6055
// Create tracker instance
6156
taskId = "test-task-id"
62-
tracker = new FileContextTracker(mockController, taskId)
57+
tracker = new FileContextTracker(mockContext, taskId)
6358
})
6459

6560
afterEach(() => {

src/core/context/context-tracking/FileContextTracker.ts

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as path from "path"
22
import * as vscode from "vscode"
33
import { getTaskMetadata, saveTaskMetadata } from "../../storage/disk"
4-
import type { FileMetadataEntry, ControllerLike } from "./ContextTrackerTypes"
4+
import type { FileMetadataEntry } from "./ContextTrackerTypes"
55

66
// This class is responsible for tracking file operations that may result in stale context.
77
// If a user modifies a file outside of Cline, the context may become stale and need to be updated.
@@ -16,29 +16,19 @@ import type { FileMetadataEntry, ControllerLike } from "./ContextTrackerTypes"
1616
// If the full contents of a file are pass to Cline via a tool, mention, or edit, the file is marked as active.
1717
// If a file is modified outside of Cline, we detect and track this change to prevent stale context.
1818
export class FileContextTracker {
19+
private context: vscode.ExtensionContext
1920
readonly taskId: string
20-
private controllerRef: WeakRef<ControllerLike>
2121

2222
// File tracking and watching
2323
private fileWatchers = new Map<string, vscode.FileSystemWatcher>()
2424
private recentlyModifiedFiles = new Set<string>()
2525
private recentlyEditedByCline = new Set<string>()
2626

27-
constructor(controller: ControllerLike, taskId: string) {
28-
this.controllerRef = new WeakRef(controller)
27+
constructor(context: vscode.ExtensionContext, taskId: string) {
28+
this.context = context
2929
this.taskId = taskId
3030
}
3131

32-
// While a task is ref'd by a controller, it will always have access to the extension context
33-
// This error is thrown if the controller derefs the task after e.g., aborting the task
34-
private context(): vscode.ExtensionContext {
35-
const context = this.controllerRef.deref()?.context
36-
if (!context) {
37-
throw new Error("Unable to access extension context")
38-
}
39-
return context
40-
}
41-
4232
// Gets the current working directory or returns undefined if it cannot be determined
4333
private getCwd(): string | undefined {
4434
const cwd = vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0)
@@ -89,9 +79,8 @@ export class FileContextTracker {
8979
return
9080
}
9181

92-
const context = this.context()
9382
// Add file to metadata
94-
await this.addFileToFileContextTracker(context, this.taskId, filePath, operation)
83+
await this.addFileToFileContextTracker(this.context, this.taskId, filePath, operation)
9584

9685
// Set up file watcher for this file
9786
await this.setupFileWatcher(filePath)

src/core/context/context-tracking/ModelContextTracker.test.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@ import * as sinon from "sinon"
44
import * as vscode from "vscode"
55
import { ModelContextTracker } from "./ModelContextTracker"
66
import * as diskModule from "../../storage/disk"
7-
import type { TaskMetadata, ControllerLike } from "./ContextTrackerTypes"
7+
import type { TaskMetadata } from "./ContextTrackerTypes"
88

99
describe("ModelContextTracker", () => {
1010
let sandbox: sinon.SinonSandbox
11-
let mockController: ControllerLike
1211
let mockContext: vscode.ExtensionContext
1312
let tracker: ModelContextTracker
1413
let taskId: string
@@ -24,18 +23,14 @@ describe("ModelContextTracker", () => {
2423
globalStorageUri: { fsPath: "/mock/storage" },
2524
} as unknown as vscode.ExtensionContext
2625

27-
mockController = {
28-
context: mockContext,
29-
}
30-
3126
// Mock disk module functions
3227
mockTaskMetadata = { files_in_context: [], model_usage: [] }
3328
getTaskMetadataStub = sandbox.stub(diskModule, "getTaskMetadata").resolves(mockTaskMetadata)
3429
saveTaskMetadataStub = sandbox.stub(diskModule, "saveTaskMetadata").resolves()
3530

3631
// Create tracker instance
3732
taskId = "test-task-id"
38-
tracker = new ModelContextTracker(mockController, taskId)
33+
tracker = new ModelContextTracker(mockContext, taskId)
3934
})
4035

4136
afterEach(() => {
@@ -83,8 +78,7 @@ describe("ModelContextTracker", () => {
8378

8479
it("should throw an error when controller is dereferenced", async () => {
8580
// Create a new tracker with a controller that will be garbage collected
86-
const weakMockController = { context: mockContext }
87-
const weakTracker = new ModelContextTracker(weakMockController, taskId)
81+
const weakTracker = new ModelContextTracker(mockContext, taskId)
8882

8983
// Force the WeakRef to return null by overriding the deref method
9084
const weakRef = { deref: sandbox.stub().returns(null) }
Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,17 @@
11
import * as vscode from "vscode"
22
import { getTaskMetadata, saveTaskMetadata } from "../../storage/disk"
3-
import type { ControllerLike } from "./ContextTrackerTypes"
43

54
export class ModelContextTracker {
65
readonly taskId: string
7-
private controllerRef: WeakRef<ControllerLike>
6+
private context: vscode.ExtensionContext
87

9-
constructor(controller: ControllerLike, taskId: string) {
10-
this.controllerRef = new WeakRef(controller)
8+
constructor(context: vscode.ExtensionContext, taskId: string) {
9+
this.context = context
1110
this.taskId = taskId
1211
}
1312

14-
// While a task is ref'd by a controller, it will always have access to the extension context
15-
// This error is thrown if the controller derefs the task after e.g., aborting the task
16-
private context(): vscode.ExtensionContext {
17-
const context = this.controllerRef.deref()?.context
18-
if (!context) {
19-
throw new Error("Unable to access extension context")
20-
}
21-
return context
22-
}
23-
2413
async recordModelUsage(apiProviderId: string, modelId: string, mode: string) {
25-
const context = this.context()
26-
const metadata = await getTaskMetadata(context, this.taskId)
14+
const metadata = await getTaskMetadata(this.context, this.taskId)
2715

2816
if (!metadata.model_usage) {
2917
metadata.model_usage = []
@@ -47,6 +35,6 @@ export class ModelContextTracker {
4735
mode: mode,
4836
})
4937

50-
await saveTaskMetadata(context, this.taskId, metadata)
38+
await saveTaskMetadata(this.context, this.taskId, metadata)
5139
}
5240
}

src/core/controller/index.ts

Lines changed: 51 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@ import { selectImages } from "../../integrations/misc/process-images"
1717
import { getTheme } from "../../integrations/theme/getTheme"
1818
import WorkspaceTracker from "../../integrations/workspace/WorkspaceTracker"
1919
import { ClineAccountService } from "../../services/account/ClineAccountService"
20+
import { discoverChromeInstances } from "../../services/browser/BrowserDiscovery"
21+
import { BrowserSession } from "../../services/browser/BrowserSession"
2022
import { McpHub } from "../../services/mcp/McpHub"
23+
import { searchWorkspaceFiles } from "../../services/search/file-search"
2124
import { telemetryService } from "../../services/telemetry/TelemetryService"
2225
import { ApiProvider, ModelInfo } from "../../shared/api"
23-
import { findLast } from "../../shared/array"
2426
import { ChatContent } from "../../shared/ChatContent"
2527
import { ChatSettings } from "../../shared/ChatSettings"
2628
import { ExtensionMessage, ExtensionState, Invoke, Platform } from "../../shared/ExtensionMessage"
@@ -30,9 +32,10 @@ import { TelemetrySetting } from "../../shared/TelemetrySetting"
3032
import { ClineCheckpointRestore, WebviewMessage } from "../../shared/WebviewMessage"
3133
import { fileExistsAtPath } from "../../utils/fs"
3234
import { searchCommits } from "../../utils/git"
35+
import { getWorkspacePath } from "../../utils/path"
3336
import { getTotalTasksSize } from "../../utils/storage"
34-
import { Task } from "../task"
3537
import { openMention } from "../mentions"
38+
import { GlobalFileNames } from "../storage/disk"
3639
import {
3740
getAllExtensionState,
3841
getGlobalState,
@@ -42,12 +45,7 @@ import {
4245
updateApiConfiguration,
4346
updateGlobalState,
4447
} from "../storage/state"
45-
import { WebviewProvider } from "../webview"
46-
import { BrowserSession } from "../../services/browser/BrowserSession"
47-
import { GlobalFileNames } from "../storage/disk"
48-
import { discoverChromeInstances } from "../../services/browser/BrowserDiscovery"
49-
import { searchWorkspaceFiles } from "../../services/search/file-search"
50-
import { getWorkspacePath } from "../../utils/path"
48+
import { Task } from "../task"
5149

5250
/*
5351
https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts
@@ -56,25 +54,37 @@ https://github.com/KumarVariable/vscode-extension-sidebar-html/blob/master/src/c
5654
*/
5755

5856
export class Controller {
57+
private postMessage: (message: any) => Thenable<boolean> | undefined
58+
5959
private disposables: vscode.Disposable[] = []
6060
private task?: Task
61-
workspaceTracker?: WorkspaceTracker
62-
mcpHub?: McpHub
63-
accountService?: ClineAccountService
61+
workspaceTracker: WorkspaceTracker
62+
mcpHub: McpHub
63+
accountService: ClineAccountService
6464
private latestAnnouncementId = "april-10-2025" // update to some unique identifier when we add a new announcement
65-
private webviewProviderRef: WeakRef<WebviewProvider>
6665

6766
constructor(
6867
readonly context: vscode.ExtensionContext,
6968
private readonly outputChannel: vscode.OutputChannel,
70-
webviewProvider: WebviewProvider,
69+
postMessage: (message: any) => Thenable<boolean> | undefined,
7170
) {
7271
this.outputChannel.appendLine("ClineProvider instantiated")
73-
this.webviewProviderRef = new WeakRef(webviewProvider)
74-
75-
this.workspaceTracker = new WorkspaceTracker(this)
76-
this.mcpHub = new McpHub(this)
77-
this.accountService = new ClineAccountService(this)
72+
this.postMessage = postMessage
73+
74+
this.workspaceTracker = new WorkspaceTracker((msg) => this.postMessageToWebview(msg))
75+
this.mcpHub = new McpHub(
76+
() => this.ensureMcpServersDirectoryExists(),
77+
() => this.ensureSettingsDirectoryExists(),
78+
(msg) => this.postMessageToWebview(msg),
79+
this.context.extension?.packageJSON?.version ?? "1.0.0",
80+
)
81+
this.accountService = new ClineAccountService(
82+
(msg) => this.postMessageToWebview(msg),
83+
async () => {
84+
const { apiConfiguration } = await this.getStateToPostToWebview()
85+
return apiConfiguration?.clineApiKey
86+
},
87+
)
7888

7989
// Clean up legacy checkpoints
8090
cleanupLegacyCheckpoints(this.context.globalStorageUri.fsPath, this.outputChannel).catch((error) => {
@@ -97,11 +107,8 @@ export class Controller {
97107
x.dispose()
98108
}
99109
}
100-
this.workspaceTracker?.dispose()
101-
this.workspaceTracker = undefined
102-
this.mcpHub?.dispose()
103-
this.mcpHub = undefined
104-
this.accountService = undefined
110+
this.workspaceTracker.dispose()
111+
this.mcpHub.dispose()
105112
this.outputChannel.appendLine("Disposed all disposables")
106113

107114
console.error("Controller disposed")
@@ -124,42 +131,40 @@ export class Controller {
124131
await updateGlobalState(this.context, "userInfo", info)
125132
}
126133

127-
async initClineWithTask(task?: string, images?: string[]) {
134+
async initTask(task?: string, images?: string[], historyItem?: HistoryItem) {
128135
await this.clearTask() // ensures that an existing task doesn't exist before starting a new one, although this shouldn't be possible since user must clear task before starting a new one
129136
const { apiConfiguration, customInstructions, autoApprovalSettings, browserSettings, chatSettings } =
130137
await getAllExtensionState(this.context)
131138
this.task = new Task(
132-
this,
139+
this.context,
140+
this.mcpHub,
141+
this.workspaceTracker,
142+
(historyItem) => this.updateTaskHistory(historyItem),
143+
() => this.postStateToWebview(),
144+
(message) => this.postMessageToWebview(message),
145+
(taskId) => this.reinitExistingTaskFromId(taskId),
146+
() => this.cancelTask(),
133147
apiConfiguration,
134148
autoApprovalSettings,
135149
browserSettings,
136150
chatSettings,
137151
customInstructions,
138152
task,
139153
images,
154+
historyItem,
140155
)
141156
}
142157

143-
async initClineWithHistoryItem(historyItem: HistoryItem) {
144-
await this.clearTask()
145-
const { apiConfiguration, customInstructions, autoApprovalSettings, browserSettings, chatSettings } =
146-
await getAllExtensionState(this.context)
147-
this.task = new Task(
148-
this,
149-
apiConfiguration,
150-
autoApprovalSettings,
151-
browserSettings,
152-
chatSettings,
153-
customInstructions,
154-
undefined,
155-
undefined,
156-
historyItem,
157-
)
158+
async reinitExistingTaskFromId(taskId: string) {
159+
const history = await this.getTaskWithId(taskId)
160+
if (history) {
161+
await this.initTask(undefined, undefined, history.historyItem)
162+
}
158163
}
159164

160165
// Send any JSON serializable data to the react app
161166
async postMessageToWebview(message: ExtensionMessage) {
162-
await this.webviewProviderRef.deref()?.view?.webview.postMessage(message)
167+
await this.postMessage(message)
163168
}
164169

165170
/**
@@ -259,7 +264,7 @@ export class Controller {
259264
// Could also do this in extension .ts
260265
//this.postMessageToWebview({ type: "text", text: `Extension: ${Date.now()}` })
261266
// initializing new instance of Cline will make sure that any agentically running promises in old instance don't affect our new task. this essentially creates a fresh slate for the new task
262-
await this.initClineWithTask(message.text, message.images)
267+
await this.initTask(message.text, message.images)
263268
break
264269
case "apiConfiguration":
265270
if (message.apiConfiguration) {
@@ -1083,7 +1088,7 @@ export class Controller {
10831088
// 'abandoned' will prevent this cline instance from affecting future cline instance gui. this may happen if its hanging on a streaming request
10841089
this.task.abandoned = true
10851090
}
1086-
await this.initClineWithHistoryItem(historyItem) // clears task again, so we need to abortTask manually above
1091+
await this.initTask(undefined, undefined, historyItem) // clears task again, so we need to abortTask manually above
10871092
// await this.postStateToWebview() // new Cline instance will post state when it's ready. having this here sent an empty messages array to webview leading to virtuoso having to reload the entire list
10881093
}
10891094
}
@@ -1401,7 +1406,7 @@ export class Controller {
14011406
Here is the project's README to help you get started:\n\n${mcpDetails.readmeContent}\n${mcpDetails.llmsInstallationContent}`
14021407

14031408
// Initialize task and show chat view
1404-
await this.initClineWithTask(task)
1409+
await this.initTask(task)
14051410
await this.postMessageToWebview({
14061411
type: "action",
14071412
action: "chatButtonClicked",
@@ -1735,9 +1740,7 @@ Here is the project's README to help you get started:\n\n${mcpDetails.readmeCont
17351740

17361741
const fileMention = this.getFileMentionFromPath(filePath)
17371742
const problemsString = this.convertDiagnosticsToProblemsString(diagnostics)
1738-
await this.initClineWithTask(
1739-
`Fix the following code in ${fileMention}\n\`\`\`\n${code}\n\`\`\`\n\nProblems:\n${problemsString}`,
1740-
)
1743+
await this.initTask(`Fix the following code in ${fileMention}\n\`\`\`\n${code}\n\`\`\`\n\nProblems:\n${problemsString}`)
17411744

17421745
console.log("fixWithCline", code, filePath, languageId, diagnostics, problemsString)
17431746
}
@@ -1813,7 +1816,7 @@ Here is the project's README to help you get started:\n\n${mcpDetails.readmeCont
18131816
if (id !== this.task?.taskId) {
18141817
// non-current task
18151818
const { historyItem } = await this.getTaskWithId(id)
1816-
await this.initClineWithHistoryItem(historyItem) // clears existing task
1819+
await this.initTask(undefined, undefined, historyItem) // clears existing task
18171820
}
18181821
await this.postMessageToWebview({
18191822
type: "action",

0 commit comments

Comments
 (0)