fix: address PR review feedback

- Add pendingByParent cleanup to ALL completion paths (session.idle, polling, stability)
- Add null guard for task.parentSessionID before Map access
- Add consistency guard in prune function (set concurrencyKey = undefined)
- Remove redundant setTimeout release (already released at completion)
This commit is contained in:
Gladdonilli 2026-01-13 14:40:50 +08:00
parent c196db2a0e
commit 129388387b

View File

@ -426,6 +426,16 @@ export class BackgroundManager {
this.concurrencyManager.release(task.concurrencyKey) this.concurrencyManager.release(task.concurrencyKey)
task.concurrencyKey = undefined // Prevent double-release task.concurrencyKey = undefined // Prevent double-release
} }
// Clean up pendingByParent to prevent stale entries
if (task.parentSessionID) {
const pending = this.pendingByParent.get(task.parentSessionID)
if (pending) {
pending.delete(task.id)
if (pending.size === 0) {
this.pendingByParent.delete(task.parentSessionID)
}
}
}
this.markForNotification(task) this.markForNotification(task)
await this.notifyParentSession(task) await this.notifyParentSession(task)
log("[background-agent] Task completed via session.idle event:", task.id) log("[background-agent] Task completed via session.idle event:", task.id)
@ -453,11 +463,13 @@ export class BackgroundManager {
task.concurrencyKey = undefined // Prevent double-release task.concurrencyKey = undefined // Prevent double-release
} }
// Clean up pendingByParent to prevent stale entries // Clean up pendingByParent to prevent stale entries
const pending = this.pendingByParent.get(task.parentSessionID) if (task.parentSessionID) {
if (pending) { const pending = this.pendingByParent.get(task.parentSessionID)
pending.delete(task.id) if (pending) {
if (pending.size === 0) { pending.delete(task.id)
this.pendingByParent.delete(task.parentSessionID) if (pending.size === 0) {
this.pendingByParent.delete(task.parentSessionID)
}
} }
} }
this.tasks.delete(task.id) this.tasks.delete(task.id)
@ -678,6 +690,7 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea
const taskId = task.id const taskId = task.id
setTimeout(() => { setTimeout(() => {
// Concurrency already released at completion - just cleanup notifications and task
this.clearNotificationsForTask(taskId) this.clearNotificationsForTask(taskId)
this.tasks.delete(taskId) this.tasks.delete(taskId)
log("[background-agent] Removed completed task from memory:", taskId) log("[background-agent] Removed completed task from memory:", taskId)
@ -717,6 +730,7 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea
task.completedAt = new Date() task.completedAt = new Date()
if (task.concurrencyKey) { if (task.concurrencyKey) {
this.concurrencyManager.release(task.concurrencyKey) this.concurrencyManager.release(task.concurrencyKey)
task.concurrencyKey = undefined // Prevent double-release
} }
this.clearNotificationsForTask(taskId) this.clearNotificationsForTask(taskId)
this.tasks.delete(taskId) this.tasks.delete(taskId)
@ -775,6 +789,16 @@ try {
this.concurrencyManager.release(task.concurrencyKey) this.concurrencyManager.release(task.concurrencyKey)
task.concurrencyKey = undefined // Prevent double-release task.concurrencyKey = undefined // Prevent double-release
} }
// Clean up pendingByParent to prevent stale entries
if (task.parentSessionID) {
const pending = this.pendingByParent.get(task.parentSessionID)
if (pending) {
pending.delete(task.id)
if (pending.size === 0) {
this.pendingByParent.delete(task.parentSessionID)
}
}
}
this.markForNotification(task) this.markForNotification(task)
await this.notifyParentSession(task) await this.notifyParentSession(task)
log("[background-agent] Task completed via polling:", task.id) log("[background-agent] Task completed via polling:", task.id)
@ -846,6 +870,16 @@ if (lastMessage) {
this.concurrencyManager.release(task.concurrencyKey) this.concurrencyManager.release(task.concurrencyKey)
task.concurrencyKey = undefined // Prevent double-release task.concurrencyKey = undefined // Prevent double-release
} }
// Clean up pendingByParent to prevent stale entries
if (task.parentSessionID) {
const pending = this.pendingByParent.get(task.parentSessionID)
if (pending) {
pending.delete(task.id)
if (pending.size === 0) {
this.pendingByParent.delete(task.parentSessionID)
}
}
}
this.markForNotification(task) this.markForNotification(task)
await this.notifyParentSession(task) await this.notifyParentSession(task)
log("[background-agent] Task completed via stability detection:", task.id) log("[background-agent] Task completed via stability detection:", task.id)