Skip to content

fix: clear stale update-success notice after restarting into an old install#190

Merged
elkaix merged 1 commit into
mainfrom
fix/stale-update-restart-notice
Jul 1, 2026
Merged

fix: clear stale update-success notice after restarting into an old install#190
elkaix merged 1 commit into
mainfrom
fix/stale-update-restart-notice

Conversation

@elkaix

@elkaix elkaix commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Homebrew installs can restart into the same pre-update binary when the formula bump doesn't actually land, and the persisted "Restart to apply" status survived that restart forever since it only checked job state and target version.
  • update_restart_pending() now also requires the recorded pid to match the current process, so a restart into a still-old version correctly falls back to "Update available — /update" instead of repeating an already-ineffective restart prompt.
  • Dedupes the "installed" check that was previously inlined separately in _compute_update_notice and _welcome_banner_chip into the shared helper.

Test plan

  • make check-pythinker-code (ruff + format + pyright + ty) — all checks passed
  • uv run pytest tests/ui_and_conv/test_shell_welcome_info.py tests/ui_and_conv/test_silent_auto_update.py — 46 passed

Summary by CodeRabbit

  • Bug Fixes
    • Fixed update status banners so they no longer get stuck on “Restart to apply” after a restart into an older install.
    • Update messaging now correctly switches back to “Update available” when the previous update no longer applies.
    • Improved consistency in the welcome banner and update notices when showing restart vs. update prompts.

…nstall

Homebrew installs can restart into the same pre-update binary when the
formula bump doesn't actually land. The persisted "Restart to apply"
status survived that restart forever because it only checked job state
and target version, not which process wrote it. Scope the notice to the
process that recorded the successful update (pid match) so a restart
into a still-old version falls back to "Update available" instead of
repeating an already-ineffective restart prompt.
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7fc18f45-76f1-4baf-9927-4df3d9ccbc0c

📥 Commits

Reviewing files that changed from the base of the PR and between d73d849 and 21deefd.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • src/pythinker_code/ui/shell/__init__.py
  • src/pythinker_code/ui/shell/update_orchestrator.py
  • tests/ui_and_conv/test_shell_welcome_info.py
  • tests/ui_and_conv/test_silent_auto_update.py

📝 Walkthrough

Walkthrough

Adds a update_restart_pending helper in update_orchestrator.py that checks job state, target version, PID match, and smoke-check-failure exclusion. Shell __init__.py now uses this helper instead of inline logic for update-notice and welcome-banner restart decisions. Tests updated to use os.getpid() and cover new PID-mismatch scenarios. Changelog updated.

Changes

Restart-pending notice fix

Layer / File(s) Summary
PID-aware restart helper
src/pythinker_code/ui/shell/update_orchestrator.py
Adds update_restart_pending(status, target_version) checking job state, target version match, PID match, and smoke-check-failure exclusion.
Wire helper into notice/banner logic
src/pythinker_code/ui/shell/__init__.py
Replaces inline "installed" boolean logic in _compute_update_notice and _welcome_banner_chip with calls to update_restart_pending.
Welcome banner tests
tests/ui_and_conv/test_shell_welcome_info.py
Uses os.getpid() instead of hardcoded PIDs and adds a test verifying "Update available" is shown when the installed version remains old after restart.
Silent auto-update tests
tests/ui_and_conv/test_silent_auto_update.py
Adjusts assertions, extends _updated_status with a pid parameter, and adds a test for the fallback "Update available" message when PIDs mismatch.
Changelog entry
CHANGELOG.md
Documents the fix for stale update-success notices after restarting into an older install.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

Suggested labels: bug

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the change and tests, but it misses the required Related Issue section and checklist from the template. Add the template sections: Related Issue with Resolve #(issue_number), a Description section, and the checklist items requested by the repo.
Docstring Coverage ⚠️ Warning Docstring coverage is 23.53% which is insufficient. The required threshold is 70.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title uses a valid conventional-commit format and clearly matches the update-notice fix in the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/stale-update-restart-notice

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@elkaix elkaix merged commit 3c44c7e into main Jul 1, 2026
35 checks passed
@elkaix elkaix deleted the fix/stale-update-restart-notice branch July 1, 2026 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant