fix(update): retry post-install smoke check and surface verification failure#241
Conversation
…failure The smoke check ran once, immediately after the package-manager upgrade. Homebrew can report success moments before its opt launcher symlink is repointed at the new keg, so the probe exercised the old binary, reported the old version, and recorded a false VERIFICATION_FAILED — leaving the footer stuck on the stale update notice while the screen said "Updated successfully!". Retry the smoke check up to 3 times, 1s apart, off the event loop (asyncio.to_thread) so the probe can no longer block the shell either. When verification genuinely fails after all attempts, print the failure where the install success was shown so the on-screen outcome matches the recorded status.
…guards Three tests assert 'Working' is absent from the rendered activity spinner to guard against the old static 'Working…' placeholder. The verb rotates on time.monotonic() over a list that legitimately includes 'Working', so the suite fails deterministically for the verb's 10-minute rotation window every ~21 hours. Pin spinner_message in those tests so the guard only catches the real regression.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 37 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: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe update orchestrator now retries post-install smoke checks asynchronously, reports final verification failures on screen, and tests transient and persistent failures. UI spinner tests pin rotating text to deterministic output. ChangesUpdate verification
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant UpdateJob
participant RetryHelper
participant SmokeCheck
participant Console
UpdateJob->>RetryHelper: await post-install verification
RetryHelper->>SmokeCheck: run smoke check in worker thread
SmokeCheck-->>RetryHelper: verification result
RetryHelper->>RetryHelper: retry transient failure
RetryHelper-->>UpdateJob: final result
UpdateJob->>Console: print verification failure
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/pythinker_code/ui/shell/update_orchestrator.py`:
- Around line 575-584: Update _run_smoke_check_with_retry and its cancellation
handling so asyncio cancellation during run_post_install_smoke_check produces a
terminal job status before run_update_job releases the update lock. Ensure the
smoke-check thread/subprocess is not left running untracked, while preserving
the existing retry behavior for non-cancellation failures.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 50d84c1f-2c76-4861-a9d1-063e5d11f8c9
📒 Files selected for processing (5)
CHANGELOG.mdsrc/pythinker_code/ui/shell/update_orchestrator.pytests/ui_and_conv/test_empty_think_part_indicator.pytests/ui_and_conv/test_modal_lifecycle.pytests/ui_and_conv/test_update_orchestrator.py
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Cancellation lands on the job's await points and skipped the failure handler, releasing the update lock while the status file still said RUNNING. Catch CancelledError, write a terminal FAILED status, and re-raise.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
1 participant
Summary
VERIFICATION_FAILED. Homebrew can report success moments before itsoptlauncher symlink is repointed at the new keg; the single immediate probe then exercised the OLD binary, reported the old version, and recorded a falseVERIFICATION_FAILED— leaving the footer stuck on the stale "↑ Update available" notice after a successful upgrade (observed live on a 0.60.0 → 0.62.0 brew upgrade). A genuinely bad install still fails every attempt.asyncio.to_thread) so the up-to-10s--versionsubprocess can no longer stall the shell.print_output=True, print theUpdated, but smoke check did not pass: …message where "Updated successfully!" was shown, so the visible outcome matches the recorded status (failure-truthfulness / C01/C13)."Working" not in rendered, but the spinner verb rotates on wall-clock through a list that legitimately includes "Working" — the suite failed deterministically for that verb's 10-minute rotation window every ~21 hours (it blocked this branch's first push). The tests now pinspinner_message.Testing
test_update_job_smoke_check_retry_absorbs_launcher_relink_race(fail-then-pass →UPDATED, retry logged) and the existing verification-failure test now asserts all attempts run.make check-pythinker-codegreen (ruff + pyright + ty); fullmake test-pythinker-codegreen via the pre-push gate.Changelog
## Unreleasedentry added.Summary by CodeRabbit
Bug Fixes
Tests