Repository navigation
fix(update): show why a startup update failed, and how to move an install from the old package name - #840
Conversation
…tall from the old package name (#771)
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: needs_human (risk 2/5) · profile _default
Safe, well-tested auto-update messaging fix, but it edits an agent-instruction file (gotchas.md), which policy bars from auto-approval.
Concerns:
plugins/kbagent/skills/kbagent/references/gotchas.md: Agent-instruction file change: policy requires human review, cannot auto-approve.
soustruh
left a comment
There was a problem hiding this comment.
No blocking findings. Checked:
- Banner:
_format_update_failureprints the last non-empty uv line asCause:, then either the legacy advice orRecover with:. The installer output reaches it becauserun_installmerges stderr into the captured output, and the banner is written tosys.stderr. A banner test captures stderr and fails if theCause:line is missing or the output is empty. - Legacy detection:
APP_NAME != LEGACY_APP_NAMEmatches the dynamic name resolution inconstants.py, so the advice appears only when the bridge wheel under the old name runs this code. The advice is withheld when there is no recovery command, so an uninstall is never shown without the install after it. - Order: uninstall comes before install. Both the service test and the banner test assert this by string index, so a swapped order fails them.
- Docs and claim: README and
gotchas.mdmatch the code and carryvNEXT. The claim aboutkbagent updateis correct:_update_kbagentputsrun.outputin the message and_summarize_failure_tailtakes its last line.tests/test_auto_update.pyandtests/test_version_service.pypass locally (179 passed).
One observation, not a finding: on a legacy install the advice replaces Recover with: for every failed install, not only for the Executable already exists conflict. A plain network failure on such an install is also told to uninstall first. The Cause: line is shown, so the user can tell the difference.
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: auto_approve (risk 2/5) · profile keboola-mcp-server
Contained bug-fix to the auto-update failure banner, fully covered by new regression tests.
zajca
left a comment
There was a problem hiding this comment.
Actionable findings from the automated review.
… forced reinstall never hits the conflict
Dismissing prior approval — a new commit was pushed and this review was for an earlier SHA. Run @keboola-pr-reviewer-bot review to get a fresh verdict.
|
New commit on |
…l-update # Conflicts: # plugins/kbagent/skills/kbagent/references/gotchas.md
Refs #771
What was wrong
keboola-agent-clitokeboola-cliin 0.63.0. Both packages install thekbagentexecutable. On an install made under the old name, uv treats the update as a new tool and stops withExecutable already exists: kbagent.Auto-update failed; continuing with current version.It dropped the uv error, so the user could not see the cause.kbagent updateand theupgrade_commandofkbagent version --jsonrun the same failing command.What changed
Cause:line with the last line of the uv output, thenRecover with: <command>, on stderr.uv tool list.gotchas.md: an entry for agents, tagged(since vNEXT, #771).kbagent updateis unchanged. Itsupdate FAILED:summary already ends with the last uv line.Limits
keboola-agent-clion purpose. The move does not touch the configuration.Tests
tests/test_auto_update.py: the banner is checked line by line (failure line,Cause:with the last uv line,Recover with:), and no installer output means noCause:line. A failed update carries the installer output.tests/test_version_service.py:summarize_install_failure.make checkpasses.Update 2026-10-10
The banner no longer prints the two move commands. The startup update always runs
uv tool install --force --reinstall, and--forcetakes thekbagentexecutable over from the old tool entry, so uv never stops withExecutable already existsthere. Checked with uv 0.12.15 in a throwaway tool directory. The banner shows theCause:line andRecover with:. The README keeps the manual two-command move for installs that cannot update at all (0.59.x and older).