fix(runner): queue work when already running instead of discarding#10
Conversation
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesRunner queue lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Runner
participant CurrentWork
participant PendingWork
Caller->>Runner: ensureRunning
Runner->>CurrentWork: execute active work
Caller->>Runner: ensureRunning while Running
Runner->>PendingWork: queue second execution
CurrentWork-->>Runner: complete
Runner->>PendingWork: start queued work
PendingWork-->>Caller: return pending result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
|
@coderabbitai full review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 48 minutes. |
Ported from upstream anomalyco#36375.
d258fd1 to
f4d4f33
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/opencode/test/effect/runner.test.ts (1)
145-178: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the shared pending work executes only once.
This test tracks
callsbut never checks it. The whole point of "shares pending" is that both queued callers resolve from a single execution ofwork. Assertingcalls === 1would catch a regression where each queued caller spawns its own run.💚 Proposed assertion
if (Exit.isSuccess(exitB)) expect(exitB.value).toBe("queued") if (Exit.isSuccess(exitC)) expect(exitC.value).toBe("queued") + expect(yield* Ref.get(calls)).toBe(1) // pending work runs once, shared by both callers }),🤖 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 `@packages/opencode/test/effect/runner.test.ts` around lines 145 - 178, Update the “third ensureRunning shares pending in RunningThenRun” test to assert that the shared queued work executes exactly once by checking the tracked calls Ref after both pending fibers complete. Keep the existing success and result assertions unchanged.
🤖 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.
Nitpick comments:
In `@packages/opencode/test/effect/runner.test.ts`:
- Around line 145-178: Update the “third ensureRunning shares pending in
RunningThenRun” test to assert that the shared queued work executes exactly once
by checking the tracked calls Ref after both pending fibers complete. Keep the
existing success and result assertions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01013b0d-f121-46b9-8d6f-c6e54afde5ab
📒 Files selected for processing (2)
packages/opencode/src/effect/runner.tspackages/opencode/test/effect/runner.test.ts
Addresses CodeRabbit nitpick on PR #10.
Port of upstream anomalyco/opencode#36375.
Adds a
RunningThenRunstate to the runner state machine so anensureRunningcall that arrives while a drain is in flight queues one follow-up run instead of being silently dropped — previously new work admitted during a drain could sit unprocessed until the next unrelated wake.Verified:
bun test test/effect/runner.test.ts(28 pass, includes new queued-run regression tests) andbun typecheckfrom packages/opencode. Applied cleanly.Summary by CodeRabbit
New Features
Bug Fixes