Conversation
📝 WalkthroughWalkthroughAdds idempotent promise-based task execution, a semaphore-backed ChangesTask scheduling
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ClineProvider
participant TaskScheduler
participant Task
ClineProvider->>TaskScheduler: schedule(task, task.run)
TaskScheduler->>Task: run()
Task-->>TaskScheduler: resolve or reject
TaskScheduler->>TaskScheduler: release concurrency permit
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/webview/ClineProvider.ts`:
- Around line 3191-3199: Update createTaskWithHistoryItem so its implicit task
startup uses taskScheduler.schedule(task, () => task.run()) instead of invoking
task.run directly, preserving the startTask: false opt-out and matching
createTask’s concurrency-gated startup behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9116becd-95dc-49c3-a4a2-4822b1a6ac95
📒 Files selected for processing (9)
AGENTS.mdsrc/__tests__/provider-delegation.spec.tssrc/__tests__/single-open-invariant.spec.tssrc/core/task/Task.tssrc/core/task/TaskScheduler.tssrc/core/task/__tests__/Task.dispose.test.tssrc/core/task/__tests__/TaskScheduler.spec.tssrc/core/webview/ClineProvider.tssrc/eslint-suppressions.json
|
|
||
| await this.addClineToStack(task) | ||
| if (options.startTask !== false) { | ||
| task.start() | ||
| void this.taskScheduler | ||
| .schedule(task, () => task.run()) | ||
| .catch((error) => { | ||
| console.error("[createTask] taskScheduler.schedule failed:", error) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'taskScheduler' src/core/webview/ClineProvider.ts
rg -nP -A15 'public async createTaskWithHistoryItem' src/core/webview/ClineProvider.tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 1307
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ClineProvider relevant usages =="
sed -n '150,190p;660,705p;1040,1160p;3168,3220p' src/core/webview/ClineProvider.ts
echo
echo "== locate TaskScheduler definitions =="
rg -n "class TaskScheduler|addClineToStack|maxConcurrency|startTask" src/core -g '*.ts'
echo
echo "== relevant TaskScheduler files =="
fd -a 'TaskScheduler' srcRepository: Zoo-Code-Org/Zoo-Code
Length of output: 32061
Unify resumable task startup with TaskScheduler.
createTask now routes auto-start through taskScheduler, but createTaskWithHistoryItem() still constructs with startTask: false then auto-starts unless the caller passes startTask: false; resumed/rehydrated tasks can bypass the concurrency gate. If this is intentional for history revival, add a short note; otherwise route the implicit start through taskScheduler.schedule(task, () => task.run()) like normal creation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/webview/ClineProvider.ts` around lines 3191 - 3199, Update
createTaskWithHistoryItem so its implicit task startup uses
taskScheduler.schedule(task, () => task.run()) instead of invoking task.run
directly, preserving the startTask: false opt-out and matching createTask’s
concurrency-gated startup behavior.
Related GitHub Issue
Closes: #358
Description
Introduces
TaskScheduler, a semaphore-based concurrency gate that wraps the existingTaskSemaphoreutility, and wires it intoClineProvideras the single launch path for all tasks.Key changes:
TaskScheduler(src/core/task/TaskScheduler.ts): thin wrapper aroundTaskSemaphore;schedule(task, run)acquires a permit, checkstask.abort || task.abandonedbefore executing, and always releases infinally. Ships atmaxConcurrency=1(identical to current serial behaviour — raising it later enables fan-out without touching gate logic).cancelQueued()is called fromClineProvider.dispose()to drain the semaphore on teardown.Task.run()(src/core/task/Task.ts): awaitable twin ofTask.start(); returns the in-flight_runPromiseif already running, or a resolved promise if_startedis set (guards against double-invocation from constructor,start(), or a secondrun()call).ClineProvidernow callstask.run()through the scheduler instead oftask.start()directly.ClineProvider(src/core/webview/ClineProvider.ts): holds aTaskSchedulerinstance; all task launch sites (createTask,delegateParentAndOpenChild) usevoid this.taskScheduler.schedule(task, () => task.run()).dispose()callscancelQueued()before clearing the stack.Tests:
TaskScheduler.spec.ts(8 cases: immediate run, serial queuing, parallel at maxConcurrency=2, permit release on throw, cancelQueued, abort guard, abandoned guard, default concurrency);Task.dispose.test.tsgains aTask.run() idempotencydescribe (3 cases);provider-delegation.spec.tsandsingle-open-invariant.spec.tsupdated to carrytaskScheduleron provider stubs.AGENTS.md: adds an ESLint Suppressions section documenting that counts must never increase, when
as anyis acceptable, and the prune-suppressions maintenance command.eslint-suppressions.jsoncount forTask.dispose.test.tsreduced from 8 → 3.No behaviour change at
maxConcurrency=1— tasks still run serially. The scheduler is the expand step of an expand-contract migration; fan-out (Story 3.2b) raises the limit without changing this gate logic.Test Procedure
Pre-Submission Checklist