Skip to content

Commit b030cd9

Browse files
committed
fix(task): guard saveClineMessages against abandoned tasks to prevent race in abandonSubtask
Fire-and-forget saveClineMessages() calls could execute updateTaskHistory() after abandonSubtask's atomicUpdatePair() cleared parentTaskId/rootTaskId, silently reattaching the severed parent-child link. Check this.abandoned before updateTaskHistory() to catch both the explicit abort save and any in-flight fire-and-forget saves.
1 parent 4d602b1 commit b030cd9

1 file changed

Lines changed: 50 additions & 45 deletions

File tree

src/core/task/Task.ts

Lines changed: 50 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -365,14 +365,14 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
365365

366366
providerRef: WeakRef<ClineProvider>
367367
private readonly globalStoragePath: string
368-
368+
369369
/**
370370
* Usage event recorder. Called only at terminal finalize of API attempts.
371371
* Null if store initialization failed; in that case recording is silently skipped.
372372
* (Architecture report section 5.5-5.8, rollback: writer injected as optional service)
373373
*/
374374
private readonly usageRecorder: UsageRecorder | null = null
375-
375+
376376
abort: boolean = false
377377
currentRequestAbortController?: AbortController
378378
skipPrevResponseIdOnce: boolean = false
@@ -1249,6 +1249,14 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
12491249
// - Final state is emitted when updates stop (trailing: true)
12501250
this.debouncedEmitTokenUsage(tokenUsage, this.toolUsage)
12511251

1252+
// Guard: don't update history item for abandoned tasks — fire-and-forget
1253+
// saves can arrive after abandonSubtask's atomicUpdatePair has already
1254+
// cleared parentTaskId/rootTaskId, and writing the live Task's stale
1255+
// values would silently reattach the severed link.
1256+
if (this.abandoned) {
1257+
return false
1258+
}
1259+
12521260
const provider = this.providerRef.deref()
12531261
const existingStatus = provider?.taskHistoryStore.get(this.taskId)?.status
12541262
await provider?.updateTaskHistory(existingStatus ? { ...historyItem, status: existingStatus } : historyItem)
@@ -3237,48 +3245,47 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
32373245
cost: tokens.total ?? costResult.totalCost,
32383246
})
32393247

3240-
// ── Usage Stats: terminal finalize ──────────────────────────
3241-
// captureUsageData is the single terminal boundary for completed/cancelled
3242-
// API attempts. We record the final usage event here.
3243-
// (Architecture report section 5.5-5.8: terminal finalize only, no chunk-level append)
3244-
if (this.usageRecorder) {
3245-
// B1 fix: include apiReqIndex so each tool-use turn produces a unique
3246-
// requestKey. Previously requestKey = taskId:retryAttempt, which was
3247-
// identical for every turn of a task (retryAttempt resets to 0 per turn),
3248-
// causing the idempotency dedupe to drop all but the first turn's usage.
3249-
const requestKey = `${this.taskId}:${apiReqIndex}:${currentItem.retryAttempt ?? 0}`
3250-
const ctx: UsageRecordingContext = {
3251-
taskId: this.taskId,
3252-
parentTaskId: this.parentTaskId,
3253-
provider: String(
3254-
this.apiConfiguration.apiProvider && !isRetiredProvider(this.apiConfiguration.apiProvider)
3255-
? this.apiConfiguration.apiProvider
3256-
: "unknown",
3257-
),
3258-
model: getModelId(this.apiConfiguration) || "unknown",
3259-
mode: this._taskMode || defaultModeSlug,
3260-
attempt: currentItem.retryAttempt ?? 0,
3261-
inputTokens: tokens.input,
3262-
outputTokens: tokens.output,
3263-
cacheWriteTokens: tokens.cacheWrite,
3264-
cacheReadTokens: tokens.cacheRead,
3265-
totalCost: tokens.total,
3266-
// V1 semantics: provider-reported values, inclusion unknown
3267-
// (aggregator handles double-counting via inclusion metadata)
3268-
cacheReadInInput: "unknown",
3269-
cacheWriteInInput: "unknown",
3270-
reasoningInOutput: "unknown",
3271-
costSource: "provider",
3272-
tokenSource: "provider",
3273-
endpoint: resolveEndpoint(this.apiConfiguration),
3248+
// ── Usage Stats: terminal finalize ──────────────────────────
3249+
// captureUsageData is the single terminal boundary for completed/cancelled
3250+
// API attempts. We record the final usage event here.
3251+
// (Architecture report section 5.5-5.8: terminal finalize only, no chunk-level append)
3252+
if (this.usageRecorder) {
3253+
// B1 fix: include apiReqIndex so each tool-use turn produces a unique
3254+
// requestKey. Previously requestKey = taskId:retryAttempt, which was
3255+
// identical for every turn of a task (retryAttempt resets to 0 per turn),
3256+
// causing the idempotency dedupe to drop all but the first turn's usage.
3257+
const requestKey = `${this.taskId}:${apiReqIndex}:${currentItem.retryAttempt ?? 0}`
3258+
const ctx: UsageRecordingContext = {
3259+
taskId: this.taskId,
3260+
parentTaskId: this.parentTaskId,
3261+
provider: String(
3262+
this.apiConfiguration.apiProvider &&
3263+
!isRetiredProvider(this.apiConfiguration.apiProvider)
3264+
? this.apiConfiguration.apiProvider
3265+
: "unknown",
3266+
),
3267+
model: getModelId(this.apiConfiguration) || "unknown",
3268+
mode: this._taskMode || defaultModeSlug,
3269+
attempt: currentItem.retryAttempt ?? 0,
3270+
inputTokens: tokens.input,
3271+
outputTokens: tokens.output,
3272+
cacheWriteTokens: tokens.cacheWrite,
3273+
cacheReadTokens: tokens.cacheRead,
3274+
totalCost: tokens.total,
3275+
// V1 semantics: provider-reported values, inclusion unknown
3276+
// (aggregator handles double-counting via inclusion metadata)
3277+
cacheReadInInput: "unknown",
3278+
cacheWriteInInput: "unknown",
3279+
reasoningInOutput: "unknown",
3280+
costSource: "provider",
3281+
tokenSource: "provider",
3282+
endpoint: resolveEndpoint(this.apiConfiguration),
3283+
}
3284+
// Fire-and-forget: store error must not block task
3285+
this.usageRecorder.finalizeUsageEvent(requestKey, status, ctx).catch(() => {})
32743286
}
3275-
// Fire-and-forget: store error must not block task
3276-
this.usageRecorder
3277-
.finalizeUsageEvent(requestKey, status, ctx)
3278-
.catch(() => {})
3287+
// ── End Usage Stats ──────────────────────────────────────────
32793288
}
3280-
// ── End Usage Stats ──────────────────────────────────────────
3281-
}
32823289
}
32833290

32843291
try {
@@ -3420,9 +3427,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
34203427
endpoint: resolveEndpoint(this.apiConfiguration),
34213428
}
34223429
// Fire-and-forget: store error must not block task
3423-
this.usageRecorder
3424-
.finalizeUsageEvent(requestKey, failedStatus, ctx)
3425-
.catch(() => {})
3430+
this.usageRecorder.finalizeUsageEvent(requestKey, failedStatus, ctx).catch(() => {})
34263431
}
34273432
// ── End Usage Stats ──────────────────────────────────────────
34283433

0 commit comments

Comments
 (0)