Conversation
…automation (Fixes #3149) Cluster C (cross-platform): resolveSubProfileContextWindow in loadBalancerProfile.ts called provider.getModels() per LB member during registration. On CI with fake API keys each call hangs ~20s (SDK connect-timeout), making the two subagentOrchestrator-loadBalancer tests exceed their per-test timeout on both Windows and macOS. Add raceWithTimeout (5s bound) and switch the per-member resolution from a sequential for-loop to concurrent Promise.all so N members resolve in the time of one, not N. Cluster A (Windows): shellJobWindowsSpawn.test.ts runAndWait() awaited spawned.exited with no timeout. If a spawned PowerShell process hangs (ConPTY stall, slow cold-start, interactive prompt), this await blocks forever, consuming the entire per-file timeout budget. Add awaitBoundedExit() that races spawned.exited against a 15s timeout and kills the child on expiry — a genuine source fix, not a timeout widening. Cluster B (React/Ink): ink-testing-library's custom React reconciler is not fully tracked by React's act(). Sync state updates from keypress handlers leak as act() warnings. Add a stdin Proxy in render() that wraps stdin.write in sync act(), eliminating sync leaks (76->40 warnings in PermissionsModifyTrustDialog). The remaining 40 warnings are async state updates from promise chains — an ink-testing-library limitation that async act() cannot capture without breaking AuthDialog's deep async chain. Diagnosability: run-bun-tests.ts retained only the last 6K chars of a failing file's output. For dialog tests that tail was entirely React act() warnings, making assertion failures unrecoverable. Add failureExcerpt() that collapses multi-line act() warning blocks to a one-line count and uses head+tail truncation, keeping assertion failures visible within the budget. Apply to both console and JUnit output. Workflow (Part 2): remove dead macOS-gated step in windows_ci; add if: always() to macos_ci test:scripts and test:shell; add timeout-minutes: 30 to windows_ci and e2e_full; set cancel-in-progress: true. Workflow (Part 3): notify_failure now resolves the milestone from package.json version (read from main via gh api, jq-parsed, exact-title match against open milestones) and applies it to both the create and existing-issue paths. Fails soft: logs a warning and continues if the milestone cannot be resolved.
github-actions
Bot
added
the
maintainer:e2e:ok
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 33 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR improves nightly reliability with bounded process and provider waits, clearer failure excerpts, React ChangesNightly reliability and diagnostics
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
WalkthroughBefore this PR, the nightly CI workflow suffered from multiple reliability issues: cancel-in-progress was disabled, causing queued runs to pile up; jobs lacked timeouts and contained dead or conditionally skipped steps; and failure notifications did not track milestones. Test runs were also fragile — Windows spawn tests could hang indefinitely, load-balancer profile registration could stall on provider model lookups, and CLI test logs were flooded with repetitive React act() warnings that buried actual assertion failures. After this PR, the nightly workflow enables cancel-in-progress, adds explicit timeouts, removes dead steps, and guards critical paths with always() conditions. Failure notifications now auto-resolve and attach the current milestone. On the test side, Windows spawn waits are bounded with full process-tree reaping, load-balancer context-window resolution is capped at 3 seconds and runs concurrently, and failing test output is collapsed and truncated so the real failure remains visible. Release NotesBug Fixes
Tests
Refactor
Chore
Changes
Sequence DiagramsequenceDiagram
participant PM as ProfileManager
participant LB as LoadBalancerProfile
participant RP as resolveLoadBalancerSubProfiles
participant RSP as resolveLoadBalancerSubProfile
participant Prov as ProviderManager
LB->>RP: resolve sub-profiles concurrently with 3s context-window timeout
RP->>RSP: resolve each referenced sub-profile
RSP->>PM: load referenced profile
RSP-->>RP: return resolved sub-profile
RP-->>LB: return resolved sub-profile list
LB->>Prov: register LoadBalancingProvider
Magnitude🎯 3 (L) RelatedNo related items found. Pre-merge Checks
Walkthrough generated by LLxprt PR Review. Planner issue: #2256 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
OpenCodeReview — automatic reviews suspendedAutomatic OCR reviews are suspended for this PR after 2 of 2 automatic reviews. To get more reviews you can:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.github/workflows/nightly.yml:
- Around line 233-242: Update the macOS test steps following “Run tests and
generate reports” by assigning that step an id and gating each dependent step on
its result being success or failure while also requiring the workflow not to be
cancelled. Replace the unconditional always() conditions for “Run shell-script
behavioral tests (`#2606`)” and the adjacent test step, preserving execution after
test failure but preventing execution after cancellation.
- Around line 648-690: Update the milestone lookup in resolve_milestone to use
gh api --paginate when fetching open milestones, so exact title matching
searches every page rather than only the first 100 results. Preserve the
existing filtering, warning, and empty-result behavior.
In `@packages/cli/run-bun-tests.ts`:
- Around line 362-391: Refactor the loop in the output-processing flow to
eliminate the multiple continue statements that violate
sonarjs/too-many-break-or-continue-in-loop. Preserve the existing warning
start/end handling, truncation recovery, skip-count behavior, and kept-line
output by restructuring the branches with if/else-if logic; do not add an inline
lint suppression.
In `@packages/core/src/services/shellJobWindowsSpawn.test.ts`:
- Around line 261-272: Update the timeout cleanup around taskkillTree to await
its taskkill process completion, using its close or error result, before
rejecting the timeout promise. Remove the independent spawned.child.kill call so
recursive tree termination finishes before the timeout rejection; preserve the
existing handling for already-terminated processes.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 23794745-cb58-4220-82ca-bb4b47da073b
📒 Files selected for processing (8)
.github/workflows/nightly.ymlpackages/cli/run-bun-tests.tspackages/cli/src/test-utils/render.tsxpackages/cli/test/run-bun-tests.test.tspackages/core/src/services/shellJobWindowsSpawn.test.tspackages/providers/src/runtime/__tests__/profileApplication.lb.contextWindowTimeout.test.tspackages/providers/src/runtime/profile-application/loadBalancerProfile.tsscripts/tests/nightly-notifier-repository.test.ts
…ontents:read permission The nightly.yml changes (cancel-in-progress: true, contents: read for milestone resolution) require the structural validation tests to match. Both release-process-b.test.ts and nightly-bun-native-smoke.test.ts were asserting the old values.
- nightly.yml: guard macOS test steps with !cancelled() instead of always() so they don't burn runner minutes after workflow cancellation - nightly.yml: paginate milestone resolution (--paginate --slurp) so milestone title search covers all open milestones, not just page 1 - run-bun-tests.ts: refactor collapseActWarnings loop to use if/else-if branches instead of multiple continue statements (sonarjs lint rule) - shellJobWindowsSpawn.test.ts: use boundedTaskkill (awaited) instead of fire-and-forget taskkillTree so the process tree is fully reaped before rejecting the timeout promise
…ut bound - nightly.yml: replace misleading comment about cancel-in-progress that claimed only manual dispatches are cancelled (OCR finding) - contextWindowTimeout.test.ts: widen concurrent bound from 7s to 8s to reduce flaking on slow Windows CI runners while still detecting sequential execution (9s)
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-24.x-ubuntu-latest' artifact from the main CI run. |
This was referenced Aug 9, 2026
acoliver
added a commit
that referenced
this pull request
Aug 14, 2026
…d evals-nightly issue creation (Fixes #3226, #3227, #3228) (#3229) * Clear responseText on tool-call in JSON mode (Fixes #3226) (#3227) (#3228) In JSON (non-quiet) output mode, handleText accumulates ALL text across model iterations into responseText. When the model states the answer before calling tools (common with step-3.7-flash) and repeats it in the final iteration, the accumulated response becomes duplicated (e.g. $blue$$blue$). Quiet mode already discarded intermediate text on tool-call (#728); JSON mode had no equivalent. Fix: clear responseText when a tool-call event is dispatched, mirroring the quiet-mode quietTextBuffer discard. This is safe for stream-json and plain-text modes where handleText returns responseText unchanged (the clear is a no-op). Fixes the save_memory eval failure that broke evals-nightly since 8/07. Also: - Bump Windows CI (Nightly) timeout 30->60 min (#3227). PR #3154 reduced it from 60 to 30, but the suite takes ~47-50 min so the job was always cancelled. - Add notify_failure job to evals-nightly.yml (#3228) so eval failures auto-create tracked issues with milestone/label, matching nightly.yml. * Address CodeRabbit + OCR review: guard JSON.parse in test, prevent duplicate issue creation on retry * Fix actionlint SC2215: single-line gh issue list in create_issue_once
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
Fixes the nightly CI workflow across Windows and macOS — corrects workflow bugs (dead step, skipped harness steps, missing timeouts, cancel-in-progress), adds milestone auto-assignment to failure issues, improves failing-test log diagnosability, and addresses three test-failure clusters: LB registration network hangs, React/Ink act() warning floods, and Windows process-spawn test hangs.
Dive Deeper
nightly.yml workflow corrections
windows_ciande2e_full— prevents runaway jobs from blocking the runner pool indefinitely.windows_ci(the macOS matrix entries never run inside the Windows job).if: always()totest:scriptsandtest:shellinmacos_ci— these platform-coverage steps now run even when the main test step fails, instead of being silently aborted.Milestone auto-assignment
resolve_milestone()bash function readspackage.jsonfrom main viagh api, parses the version withjq, and resolves an exact-title open milestone.contents: readpermission for the Contents API call.--jqargument.Failing-test log diagnosability
failureExcerpt()inrun-bun-tests.tscollapses repetitive React "not wrapped in act(...)" warning blocks (~10 lines each, can repeat dozens of times) to a one-line count, then uses head+tail truncation to keep assertion failures visible within the budget.slice(-N)tail.slice(-0)returning the whole string when budget splits totail = 0.Load-balancer context-window timeout (Cluster C)
resolveSubProfileContextWindowcalledprovider.getModels()(real network) per LB member during registration. On CI with fake keys, each call hung ~20s (SDK connect-timeout). With 3 members, that is 60s — enough to exceed the per-file timeout.raceWithTimeout()helper bounds eachgetModels()to 3s. On timeout, the member degrades gracefully to an undefined context window (advisory — the LB falls back to its configured limit).resolveLoadBalancerSubProfileschanged from sequentialfor...oftoPromise.all, keeping input order preserved.profileApplication.lb.contextWindowTimeout.test.ts— 3 tests covering single hang, concurrent resolution (3 members complete in ~one timeout, not three), and rejection degradation (asserts contextWindow is undefined).React/Ink act() leak reduction (Cluster B)
ink-testing-libraryuses a custom React reconciler that Reactact()from react does NOT fully track.stdin.writetriggers keypress handlers that callsetStatesynchronously (sync leaks) and commit-queue promise chains that callsetStateasynchronously (async leaks).render.tsxwrapsstdin.writein syncact(), eliminating all synchronous state-update leaks (76 to 40 warnings in the worst-affected test).waitForin asyncact()breaks AuthDialog (deep async chain stalls inside async act). The Proxy is safe and reduces warnings without breaking any tests.Windows spawn-test bounded exit (Cluster A)
runAndWaitinshellJobWindowsSpawn.test.tsawaitedspawned.exitedwith no timeout. On Windows, a genuinely hung process (ConPTY stall, interactive prompt) could hang indefinitely, consuming the entire per-file budget.awaitBoundedExit()helper bounds the wait to 15s. On timeout, it usestaskkillTree(pid)(which runstaskkill /T /F) to reap the entire process tree — not just the outer PowerShell wrapper (child.kill()on Windows only kills the outer process and orphans the inner Start-Process child).Reviewer Test Plan
cancel-in-progress,timeout-minutes, dead-step removal, andif: always()additions are self-contained YAML edits.bun test packages/cli/test/run-bun-tests.test.ts— 34 tests including 7 new failureExcerpt tests (edge cases, combined collapse+headTail, truncated warnings, empty string, tiny budget).bun test packages/providers/src/runtime/__tests__/profileApplication.lb.contextWindowTimeout.test.ts— 3 tests (completes in ~6s on macOS).awaitBoundedExithelper is structurally sound — review thetaskkillTreeimport and the race-against-timeout pattern.Testing Matrix
Linked issues / bugs
Fixes #3149
Summary by CodeRabbit
Reliability
Developer Experience
CI Improvements