Skip to content

Commit 0813a07

Browse files
committed
fix: address PR review feedback for sticky mode feature
- Fix race condition in Task initialization by making taskMode private with safe accessors - Add error handling in initClineWithHistoryItem for graceful fallback - Fix inconsistent mode persistence timing to prevent out-of-sync states - Add validation for restored mode values with fallback to default - Add comprehensive test coverage for edge cases (37 new tests) - Improve documentation with detailed JSDoc comments and usage examples All critical issues identified in PR review have been resolved.
1 parent 4ff1113 commit 0813a07

File tree

5 files changed

+1458
-34
lines changed

5 files changed

+1458
-34
lines changed

src/core/task/Task.ts

Lines changed: 143 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,44 @@ export class Task extends EventEmitter<ClineEvents> {
140140
/**
141141
* The mode associated with this task. Persisted across sessions
142142
* to maintain user context when reopening tasks from history.
143+
*
144+
* ## Lifecycle
145+
*
146+
* ### For new tasks:
147+
* 1. Initially `undefined` during construction
148+
* 2. Asynchronously initialized from provider state via `initializeTaskMode()`
149+
* 3. Falls back to `defaultModeSlug` if provider state is unavailable
150+
*
151+
* ### For history items:
152+
* 1. Immediately set from `historyItem.mode` during construction
153+
* 2. Falls back to `defaultModeSlug` if mode is not stored in history
154+
*
155+
* ## Important
156+
* This property should NOT be accessed directly until `taskModeReady` promise resolves.
157+
* Use `getTaskMode()` for async access or `taskMode` getter for sync access after initialization.
158+
*
159+
* @private
160+
* @see {@link getTaskMode} - For safe async access
161+
* @see {@link taskMode} - For sync access after initialization
162+
* @see {@link waitForModeInitialization} - To ensure initialization is complete
143163
*/
144-
taskMode: string
164+
private _taskMode: string | undefined
165+
145166
/**
146167
* Promise that resolves when the task mode has been initialized.
147168
* This ensures async mode initialization completes before the task is used.
169+
*
170+
* ## Purpose
171+
* - Prevents race conditions when accessing task mode
172+
* - Ensures provider state is properly loaded before mode-dependent operations
173+
* - Provides a synchronization point for async initialization
174+
*
175+
* ## Resolution timing
176+
* - For history items: Resolves immediately (sync initialization)
177+
* - For new tasks: Resolves after provider state is fetched (async initialization)
178+
*
179+
* @private
180+
* @see {@link waitForModeInitialization} - Public method to await this promise
148181
*/
149182
private taskModeReady: Promise<void>
150183

@@ -280,21 +313,15 @@ export class Task extends EventEmitter<ClineEvents> {
280313

281314
// Store the task's mode when it's created
282315
// For history items, use the stored mode; for new tasks, we'll set it after getting state
283-
this.taskMode = historyItem?.mode || defaultModeSlug
284-
285316
if (historyItem) {
317+
this._taskMode = historyItem.mode || defaultModeSlug
318+
this.taskModeReady = Promise.resolve()
286319
TelemetryService.instance.captureTaskRestarted(this.taskId)
287320
} else {
288-
TelemetryService.instance.captureTaskCreated(this.taskId)
289-
}
290-
291-
// Initialize the task mode ready promise
292-
if (!historyItem) {
293-
// For new tasks, initialize mode asynchronously
321+
// For new tasks, don't set the mode yet - wait for async initialization
322+
this._taskMode = undefined
294323
this.taskModeReady = this.initializeTaskMode(provider)
295-
} else {
296-
// For history items, mode is already set
297-
this.taskModeReady = Promise.resolve()
324+
TelemetryService.instance.captureTaskCreated(this.taskId)
298325
}
299326

300327
// Only set up diff strategy if diff is enabled
@@ -333,29 +360,126 @@ export class Task extends EventEmitter<ClineEvents> {
333360
/**
334361
* Initialize the task mode from the provider state.
335362
* This method handles async initialization with proper error handling.
363+
*
364+
* ## Flow
365+
* 1. Attempts to fetch the current mode from provider state
366+
* 2. Sets `_taskMode` to the fetched mode or `defaultModeSlug` if unavailable
367+
* 3. Handles errors gracefully by falling back to default mode
368+
* 4. Logs any initialization errors for debugging
369+
*
370+
* ## Error handling
371+
* - Network failures when fetching provider state
372+
* - Provider not yet initialized
373+
* - Invalid state structure
374+
*
375+
* All errors result in fallback to `defaultModeSlug` to ensure task can proceed.
376+
*
377+
* @private
378+
* @param provider - The ClineProvider instance to fetch state from
379+
* @returns Promise that resolves when initialization is complete
336380
*/
337381
private async initializeTaskMode(provider: ClineProvider): Promise<void> {
338382
try {
339383
const state = await provider.getState()
340-
if (state?.mode) {
341-
this.taskMode = state.mode
342-
}
384+
this._taskMode = state?.mode || defaultModeSlug
343385
} catch (error) {
344-
// If there's an error getting state, keep the default mode
386+
// If there's an error getting state, use the default mode
387+
this._taskMode = defaultModeSlug
345388
// Use the provider's log method for better error visibility
346389
const errorMessage = `Failed to initialize task mode: ${error instanceof Error ? error.message : String(error)}`
347390
provider.log(errorMessage)
348391
}
349392
}
350393

351394
/**
352-
* Wait for the task mode to be initialized.
353-
* This should be called before any operations that depend on the correct mode being set.
395+
* Wait for the task mode to be initialized before proceeding.
396+
* This method ensures that any operations depending on the task mode
397+
* will have access to the correct mode value.
398+
*
399+
* ## When to use
400+
* - Before accessing mode-specific configurations
401+
* - When switching between tasks with different modes
402+
* - Before operations that depend on mode-based permissions
403+
*
404+
* ## Example usage
405+
* ```typescript
406+
* // Wait for mode initialization before mode-dependent operations
407+
* await task.waitForModeInitialization();
408+
* const mode = task.taskMode; // Now safe to access synchronously
409+
*
410+
* // Or use with getTaskMode() for a one-liner
411+
* const mode = await task.getTaskMode(); // Internally waits for initialization
412+
* ```
413+
*
414+
* @returns Promise that resolves when the task mode is initialized
415+
* @public
354416
*/
355417
public async waitForModeInitialization(): Promise<void> {
356418
return this.taskModeReady
357419
}
358420

421+
/**
422+
* Get the task mode asynchronously, ensuring it's properly initialized.
423+
* This is the recommended way to access the task mode as it guarantees
424+
* the mode is available before returning.
425+
*
426+
* ## Async behavior
427+
* - Internally waits for `taskModeReady` promise to resolve
428+
* - Returns the initialized mode or `defaultModeSlug` as fallback
429+
* - Safe to call multiple times - subsequent calls return immediately if already initialized
430+
*
431+
* ## Example usage
432+
* ```typescript
433+
* // Safe async access
434+
* const mode = await task.getTaskMode();
435+
* console.log(`Task is running in ${mode} mode`);
436+
*
437+
* // Use in conditional logic
438+
* if (await task.getTaskMode() === 'architect') {
439+
* // Perform architect-specific operations
440+
* }
441+
* ```
442+
*
443+
* @returns Promise resolving to the task mode string
444+
* @public
445+
*/
446+
public async getTaskMode(): Promise<string> {
447+
await this.taskModeReady
448+
return this._taskMode || defaultModeSlug
449+
}
450+
451+
/**
452+
* Get the task mode synchronously. This should only be used when you're certain
453+
* that the mode has already been initialized (e.g., after waitForModeInitialization).
454+
*
455+
* ## When to use
456+
* - In synchronous contexts where async/await is not available
457+
* - After explicitly waiting for initialization via `waitForModeInitialization()`
458+
* - In event handlers or callbacks where mode is guaranteed to be initialized
459+
*
460+
* ## Example usage
461+
* ```typescript
462+
* // After ensuring initialization
463+
* await task.waitForModeInitialization();
464+
* const mode = task.taskMode; // Safe synchronous access
465+
*
466+
* // In an event handler after task is started
467+
* task.on('taskStarted', () => {
468+
* console.log(`Task started in ${task.taskMode} mode`); // Safe here
469+
* });
470+
* ```
471+
*
472+
* @throws {Error} If the mode hasn't been initialized yet
473+
* @returns The task mode string
474+
* @public
475+
*/
476+
public get taskMode(): string {
477+
if (this._taskMode === undefined) {
478+
throw new Error("Task mode accessed before initialization. Use getTaskMode() or wait for taskModeReady.")
479+
}
480+
return this._taskMode
481+
}
482+
359483
static create(options: TaskOptions): [Task, Promise<void>] {
360484
const instance = new Task({ ...options, startTask: false })
361485
const { images, task, historyItem } = options
@@ -460,7 +584,7 @@ export class Task extends EventEmitter<ClineEvents> {
460584
taskNumber: this.taskNumber,
461585
globalStoragePath: this.globalStoragePath,
462586
workspace: this.cwd,
463-
mode: this.taskMode, // Use the task's own mode, not the current provider mode
587+
mode: this._taskMode || defaultModeSlug, // Use the task's own mode, not the current provider mode
464588
})
465589

466590
this.emit("taskTokenUsageUpdated", this.taskId, tokenUsage)

0 commit comments

Comments
 (0)