feat: configurable compile wait timeout and working timeout recovery - #2036
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
…2034) Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 22 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 (7)
📝 WalkthroughWalkthroughChangesCompile timeout and reattach
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ProjectRunner
participant Unity
participant PendingRecord
CLI->>ProjectRunner: Run compile with timeout
ProjectRunner->>Unity: Poll compile status
Unity-->>ProjectRunner: In-progress status
ProjectRunner->>PendingRecord: Persist timed-out request
CLI->>ProjectRunner: Retry compile
ProjectRunner->>PendingRecord: Read pending request
ProjectRunner->>Unity: Probe original request status
Unity-->>ProjectRunner: Return stored or completed result
ProjectRunner-->>CLI: Return compile result or timeout envelope
Possibly related PRs
🚥 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 |
compile_wait.go grew past the 500-line production-file limit after the timeout/attach work. The VibeLog write and formatting helpers are a separate concern from the wait loop, so relocate them without behavior change. Co-authored-by: Cursor <cursoragent@cursor.com>
The new CLI-only schema property is unreferenced in C# by design (same pattern as other schema fields) and lifts PublicCandidate from 22 to 23. Bump the gate to the measured count so the ceiling stays a no-slack monitor. Co-authored-by: Cursor <cursoragent@cursor.com>
CI fixes pushed①
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cli/project-runner/internal/projectrunner/run.go (1)
239-249: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider also persisting the pending record when the wait is cancelled.
persistCompilePendingRecordOrWarnonly runs on the!completedtimeout path. If the wait ends via context cancellation (line 231-238), Unity is still compiling with the samerequestID, but no pending record exists, so the nextuloop compilecannot reattach and hitsUNITY_SERVER_BUSY— the exact recovery gap this PR targets.requestIDis in scope in that branch, so persisting there too is cheap.🤖 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 `@cli/project-runner/internal/projectrunner/run.go` around lines 239 - 249, Persist the compile-pending record in the context-cancellation branch as well as the existing !completed timeout path. Update the cancellation handling around the wait logic to call persistCompilePendingRecordOrWarn with connection.ProjectRoot, requestID, and stderr before returning, preserving the existing cancellation error behavior.
🤖 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 `@cli/project-runner/internal/projectrunner/run.go`:
- Around line 239-249: Persist the compile-pending record in the
context-cancellation branch as well as the existing !completed timeout path.
Update the cancellation handling around the wait logic to call
persistCompilePendingRecordOrWarn with connection.ProjectRoot, requestID, and
stderr before returning, preserving the existing cancellation error behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: be719c09-e5e3-4271-89f2-39278006e144
📒 Files selected for processing (20)
.agents/skills/uloop-compile/SKILL.md.claude/skills/uloop-compile/SKILL.md.github/workflows/dead-code.ymlPackages/src/Editor/FirstPartyTools/Compile/CompileSchema.csPackages/src/Editor/FirstPartyTools/Compile/Skill/SKILL.mdcli/common/errors/busy_editor_state.gocli/common/errors/busy_editor_state_test.gocli/common/tools/default-tools.jsoncli/dispatcher/shared-inputs-stamp.jsoncli/project-runner/internal/projectrunner/compile_attach.gocli/project-runner/internal/projectrunner/compile_attach_test.gocli/project-runner/internal/projectrunner/compile_pending_record.gocli/project-runner/internal/projectrunner/compile_pending_record_test.gocli/project-runner/internal/projectrunner/compile_wait.gocli/project-runner/internal/projectrunner/compile_wait_deps.gocli/project-runner/internal/projectrunner/compile_wait_log.gocli/project-runner/internal/projectrunner/compile_wait_test.gocli/project-runner/internal/projectrunner/execution_errors.gocli/project-runner/internal/projectrunner/run.gocli/project-runner/shared-inputs-stamp.json
Bring in the grpc v1.82.1 bump so Go CLI Security Analysis can pass. Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # cli/dispatcher/shared-inputs-stamp.json # cli/project-runner/shared-inputs-stamp.json
Follow-up: synced with v3-beta after #2040
Merge commit: |
Attach keeps the first TimedOutAtUtc, so remaining retrieval time is wall-clock until that anchor plus the pending-record TTL. Computing remaining as TTL minus the current wait overstated the window on every reattach timeout and revived the misleading guidance from issue #2032. Co-authored-by: Cursor <cursoragent@cursor.com>
To-Do 18: attach re-timeout retention overstatementFixed in this PR per plan audit:
|
Reviewing #2036 raised the question of why a runner-only change released the dispatcher too, and answering it took several wrong guesses about the mechanism before the facts were checked. The rule was documented but the decision behind its granularity was not, so the settled question had no reversal condition to consult and was relitigated from scratch. Add ADR 0003 with the decision, why the coarse rule stays, and the two finer alternatives (call-graph reachability, reproducible-build comparison) with their reversal conditions. Record the facts that were guessed wrong during the discussion: sharedInputsHash is a change marker no consumer compares across commits, the gap is a cost trade rather than a limit of static analysis, whitelist alignment with go list -deps is already test-enforced rather than manual, and no-op releases already carry a cli-release approval click today. Keep shared-release-inputs.md operational with the observable behaviour and a pointer to the ADR.
Reviewing #2036 raised the question of why a runner-only change released the dispatcher too, and answering it took several wrong guesses about the mechanism before the facts were checked. The rule was documented but the decision behind its granularity was not, so the settled question had no reversal condition to consult and was relitigated from scratch. Add ADR 0003 with the decision, why the coarse rule stays, and the two finer alternatives (call-graph reachability, reproducible-build comparison) with their reversal conditions. Record the facts that were guessed wrong during the discussion: sharedInputsHash is a change marker no consumer compares across commits, the gap is a cost trade rather than a limit of static analysis, whitelist alignment with go list -deps is already test-enforced rather than manual, and no-op releases already carry a cli-release approval click today. Keep shared-release-inputs.md operational with the observable behaviour and a pointer to the ADR.
1 participant
Summary
uloop compilecan wait longer than the previous hard-coded 10 minutes via--compile-wait-timeout-seconds.uloop compilereattaches to the in-flight compile instead of starting a new one that hitsUNITY_SERVER_BUSY.Fixes #2032
User Impact
uloop compileattaches to the same RequestId and returns that result. Timeout and busy messages guide agents toward reattach rather than a useless new request.Changes
CompileWaitTimeoutSecondsto the compile schema/skill catalog and wire it through the project-runner wait loop (default 600s)..uloop/pending-compile-request.jsonon wait timeout; on the next compile, attach viaget-compile-statuswhen the record is still valid.IsCompiling/IsUpdating/IsDomainReloadInProgressandWaitedMsintoCOMPILE_WAIT_TIMEOUTDetails; extend compile-pathUNITY_SERVER_BUSYNextActions with reattach guidance.Verification
scripts/check-go-cli.shpassed on each slice.--compile-wait-timeout-secondsconfirmed timeout Details and attach recovery.