-
Notifications
You must be signed in to change notification settings - Fork 274
[Fix] Subtasks fail to return when users work across windows #1471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3b07f56
4962d45
632a2c5
6c5e893
a6d4814
6731aa0
8e24f6a
9ff6999
93b7fcb
62a21cf
a774c11
d7099c1
e7c5ef2
4b0dbfd
7a0ca50
2f1b794
c17850e
fa967cf
cb0889c
4b7df0a
b8f7fff
ccf7337
3eacceb
7cb3d5d
d0d077f
68b7ff4
a95d6e3
04746b3
83ce236
74b8d5a
ce652dc
bdb5091
f88a968
59bd7c8
ca8acb9
2bca4d7
fee20d6
84baec8
97ea35b
db4f21f
d0970ba
2f775e4
c5cbdba
d8f64a1
be8f671
3585d47
92d70a2
113c593
c3d2379
226930b
1dd61b8
3c0cc41
97718f1
b3bedc6
2138c0f
1bd8019
884ecd0
92c2ada
1c10245
a4f4654
4228873
582e0b2
c0279fb
aa5e77f
4e8e3ee
6686e68
343d1ad
6ef9819
e53e088
aa03a99
fe927f1
7d17b6c
b345cc2
da36581
c85a1e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import { AsyncTaskTracker } from "./async-task-tracker.js" | ||
|
|
||
| describe("AsyncTaskTracker", () => { | ||
| it("drains resolving and rejecting tasks", async () => { | ||
| const tracker = new AsyncTaskTracker() | ||
| let resolveTask!: () => void | ||
| let rejectTask!: (error: Error) => void | ||
| const resolving = new Promise<void>((resolve) => (resolveTask = resolve)) | ||
| const rejecting = new Promise<void>((_resolve, reject) => (rejectTask = reject)) | ||
|
|
||
| expect(tracker.track(resolving)).toBe(resolving) | ||
| expect(tracker.track(rejecting)).toBe(rejecting) | ||
| const drained = vi.fn() | ||
| const drain = tracker.drain().then(drained) | ||
| await Promise.resolve() | ||
| expect(drained).not.toHaveBeenCalled() | ||
|
|
||
| resolveTask() | ||
| rejectTask(new Error("expected rejection")) | ||
| await drain | ||
| expect(drained).toHaveBeenCalledOnce() | ||
| }) | ||
|
|
||
| it("includes tasks tracked while a drain is in progress", async () => { | ||
| const tracker = new AsyncTaskTracker() | ||
| let resolveFirst!: () => void | ||
| let resolveSecond!: () => void | ||
| tracker.track(new Promise<void>((resolve) => (resolveFirst = resolve))) | ||
| const drain = tracker.drain() | ||
| tracker.track(new Promise<void>((resolve) => (resolveSecond = resolve))) | ||
|
|
||
| resolveFirst() | ||
| let drained = false | ||
| void drain.then(() => (drained = true)) | ||
| await Promise.resolve() | ||
| expect(drained).toBe(false) | ||
| resolveSecond() | ||
| await drain | ||
| expect(drained).toBe(true) | ||
| }) | ||
|
|
||
| it("stops guarded callbacks before draining tracked work", async () => { | ||
| const tracker = new AsyncTaskTracker() | ||
| const callback = vi.fn(async (value: string) => value.length) | ||
| expect(tracker.isActive).toBe(true) | ||
| await expect(tracker.runIfActive(callback, "active")).resolves.toBe(6) | ||
|
|
||
| await tracker.closeAndDrain() | ||
| expect(tracker.isActive).toBe(false) | ||
| await expect(tracker.runIfActive(callback, "closed")).resolves.toBeUndefined() | ||
| expect(callback).toHaveBeenCalledOnce() | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| export class AsyncTaskTracker { | ||
| private readonly tasks = new Set<Promise<unknown>>() | ||
| private active = true | ||
|
|
||
| get isActive(): boolean { | ||
| return this.active | ||
| } | ||
|
|
||
| track<T>(task: Promise<T>): Promise<T> { | ||
| this.tasks.add(task) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline src/core/webview/ClineProvider.ts --match ClineProvider --view expanded
rg -n -C 6 \
'runDelegationTransition\s*\(|closeAndDrain\s*\(|dispose\s*\(|isDisposed|disposed' \
src/core/webview/ClineProvider.tsRepository: Zoo-Code-Org/Zoo-Code Length of output: 21181 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- tracker ---'
cat -n packages/core/src/async-task-tracker.ts
printf '%s\n' '--- direct tracker/runDelegationTransition bindings ---'
rg -n -C 10 \
'tracker\.track|runDelegationTransition\s*\(|runLockedDelegationTransition\s*\(' \
packages/core/src/async-task-tracker.ts src/core/webview/ClineProvider.ts packages/core/src/core/webview/ClineProvider.ts 2>/dev/null || true
printf '%s\n' '--- delegation entry points and disposal guards ---'
sed -n '3865,4165p' src/core/webview/ClineProvider.ts
sed -n '4380,4575p' src/core/webview/ClineProvider.tsRepository: Zoo-Code-Org/Zoo-Code Length of output: 31576 Block delegation transitions after disposal starts.
🤖 Prompt for AI AgentsSource: Path instructions |
||
| void task.then( | ||
| () => this.tasks.delete(task), | ||
| () => this.tasks.delete(task), | ||
| ) | ||
| return task | ||
| } | ||
|
|
||
| runIfActive<T, Result>(callback: (value: T) => Promise<Result>, value: T): Promise<Result | undefined> { | ||
| return this.active ? callback(value) : Promise.resolve(undefined) | ||
| } | ||
|
|
||
| async drain(): Promise<void> { | ||
| while (this.tasks.size) await Promise.allSettled(this.tasks) | ||
| } | ||
|
|
||
| async closeAndDrain(): Promise<void> { | ||
| this.active = false | ||
| await this.drain() | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test lifecycle behavior while work is still pending.
Lines 37-38 resolve the second task before awaiting
drain. A drain that snapshots only its initial task set can pass this test.Lines 48-51 call
closeAndDrain()with no tracked task. This cannot prove thatrunIfActive()stops callbacks before active work drains.Keep a deferred task pending. Assert that
drain()is still pending before resolving it. CallcloseAndDrain()without awaiting it, then assert thatrunIfActive()skips the callback before the deferred task resolves.As per path instructions, tests that assert in-flight behavior only after the call completes cannot prove the behavior during execution.
Also applies to: 48-51
🤖 Prompt for AI Agents
Source: Path instructions