feat(update): install + self-update from prebuilt wheel asset (0.60.0) - #408
Conversation
Closes #353. git-installing kbagent rebuilds the bundled React SPA via npm on every install (the uv cache misses it), taking 2-4 min on WSL2 and tripping the hardcoded 120s auto-update timeout. Vrstva 1 -- eliminate the user-side build: - release.yml publishes the universal py3-none-any wheel as a Release asset on every (pre)release; workflow_dispatch backfills older tags. - install.sh bootstrap (curl|sh) installs the prebuilt wheel: no build, no gh CLI; [server] extras by default (KBAGENT_NO_SERVER=1 opts out). - resolve_kbagent_wheel_url HEAD-probes the asset; build_kbagent_upgrade_command installs it via a PEP 508 direct ref, falling back to git+ when absent. Both the startup hook and `kbagent update` use it. Vrstva 2 -- update UX: - UPDATE_TIMEOUT_SECONDS (300) + KBAGENT_UPDATE_TIMEOUT env override replace the two hardcoded 120s timeouts. - UpdateOutcome enum distinguishes TIMEOUT (slow build, retried next run) from FAILED, so the startup hook stops printing a false "Auto-update failed" banner. - _should_skip_all scans all argv so `kbagent --json update` skips the startup hook (Bug 3: startup banner vs explicit-command JSON output). Tests: wheel-URL resolver, wheel install path + git+ fallback, timeout resolver, TIMEOUT outcome, Bug 3. conftest defaults the HEAD probe to 404 so no test reaches the network.
The argv scan in _should_skip_all matched "update"/"version" ANYWHERE in argv, so `kbagent config update` / `flow update` / `agent update` wrongly skipped the startup auto-update check (Devin Review finding on PR #408). Replace it with _top_level_subcommand_is_versioning, which walks past global flags (and --config-dir's value) to the first positional token -- the real subcommand -- so `kbagent --json update` still skips (Bug 3) while nested *-update subcommands do not. Parametrized tests cover both.
padak
left a comment
There was a problem hiding this comment.
Review of #408 — feat(update): install + self-update from prebuilt wheel asset (0.60.0)
Generated by
kbagent-pr-reviewersubagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed viamake check, not duplicated here.
Summary
This PR eliminates the WSL2 build-timeout problem (issue #353) by publishing a prebuilt py3-none-any wheel as a GitHub Release asset and wiring both the startup auto-update hook and kbagent update to prefer that wheel over the git+ source build. Supporting infrastructure includes a install.sh bootstrap script, the release.yml CI workflow that builds and uploads the wheel, an UpdateOutcome enum that distinguishes slow builds from genuine failures, and the KBAGENT_UPDATE_TIMEOUT env-var escape hatch. The implementation is clean, test coverage is solid, and make check passes (3947 passed, 8 skipped). Two non-blocking gaps were found: (1) kbagent version's upgrade_command field still shows the git+ command even when a wheel asset is available, because the check_for_updates path does not call resolve_kbagent_wheel_url; (2) the PR description says unit tests for get_update_timeout (default / env override / invalid value) were added, but they are absent from the diff. No blocking findings. Verdict: COMMENT.
Verdict
- Verdict: COMMENT
- Blocking findings: 0
- Non-blocking findings: 3
- Nits: 2
Blocking findings
(none)
Non-blocking findings
[NB-1] src/keboola_agent_cli/services/version_service.py:743 — kbagent version upgrade_command field still shows git+ when a wheel asset is available
_check_for_updates (called by kbagent version) builds the upgrade_command string shown in JSON output via build_kbagent_upgrade_command(prerelease=..., target_version=...) without resolving wheel_url first (line 743). The parallel _update_kbagent path (called by kbagent update) does call resolve_kbagent_wheel_url(kbagent_latest) and passes the result in. So kbagent --json version will always advertise a git+ install command even when the prebuilt wheel is available, which means a user or AI agent copy-pasting upgrade_command from the JSON output will get the slow source build. Fix: add the same wheel_url = resolve_kbagent_wheel_url(kbagent_latest) call before line 743 and pass it in.
[NB-2] tests/test_version_service.py and tests/test_auto_update.py — get_update_timeout unit tests missing despite PR description claiming otherwise
The PR description explicitly lists "get_update_timeout: default / env override / invalid value" as added tests under "Tests". A grep over both test files confirms no TestGetUpdateTimeout class or test.*get_update_timeout functions appear in the diff. The function has meaningful env-var parsing logic with integer validation and fallback: KBAGENT_UPDATE_TIMEOUT with non-numeric or non-positive values must fall back silently to the constant. Without explicit tests the env-var fallback paths (invalid string, negative value, zero) are untested. Fix: add three tests: test_default_returns_300, test_env_override_returns_custom, test_invalid_env_falls_back_to_default.
[NB-3] src/keboola_agent_cli/commands/context.py and CLAUDE.md — KBAGENT_UPDATE_TIMEOUT env var not documented in AGENT_CONTEXT or the env-vars table
context.py has a documented table of environment variables (KBAGENT_INCLUDE_PRERELEASE, KBAGENT_SKIP_UPDATE, etc.) at the bottom of the AGENT_CONTEXT string. KBAGENT_UPDATE_TIMEOUT is not listed. Per the plugin synchronization map rule, AGENT_CONTEXT is the primary reference an AI agent consults at session start; an undocumented env var means the agent can't tell users about it when they report slow WSL updates. The CLAUDE.md ## All CLI Commands section also has the kbagent update [--beta] entry with an env-var comment block that does not mention KBAGENT_UPDATE_TIMEOUT. Fix: add a one-line entry to the env-vars section in context.py (KBAGENT_UPDATE_TIMEOUT: integer seconds, overrides the 300s self-update subprocess timeout; raise for slow WSL git+ builds) and mirror it in the CLAUDE.md update command block.
Nits
-
[NIT-1]install.sh:200—uv tool install --force "${spec} @ ${wheel_url}"passes the entire PEP 508 spec as a single shell-quoted string. Whenspecorwheel_urlcontains spaces (unlikely but possible in user-provided envs), the word-splitting may silently fail. The pattern already works on current inputs, but a--separator or double-quoting each component individually would be more defensive. -
[NIT-2].github/workflows/release.yml:73—tag="${{ github.event.release.tag_name || inputs.tag }}"inlines the GitHub expression directly into the shell command. Forworkflow_dispatchtheinputs.tagvalue is maintainer-supplied and the checked-outrefalready constrains what the tag can be, so this is not a practical injection path, but the GitHub security hardening guide recommends routing user-controlled inputs through a stepenv:variable (env: TAG: ...then"$TAG") to make the trust boundary explicit. Low severity; flag for awareness.
Verification log
gh pr view 408 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state→ 14 files, +650/-38, state=OPEN,fix/353-prebuilt-wheel→main. Conventionalfeat(update):prefix for infra-only change is acceptable (adds user-visible behavior: prebuilt wheel, new install.sh,UpdateOutcome.TIMEOUTmessage change). ✓git rev-parse --abbrev-ref HEAD→fix/353-prebuilt-wheel✓ matches PR branchgh auth status→ authenticated aspadaktogithub.laiyagushi.com✓- Read
CONTRIBUTING.md,CLAUDE.md,plugins/kbagent/agents/keboola-expert.md✓ grep typer src/keboola_agent_cli/services/version_service.py→ empty ✓ (no layer violation)grep httpx src/keboola_agent_cli/services/version_service.py→httpx.head+httpx.get(bare function calls, no context manager) -- this is a pre-existing pattern in the file; the newresolve_kbagent_wheel_urlmatches the existing style. Not a new violation.- Magic numbers check (
grep '\b(timeout|retries|interval)\s*=\s*[0-9]+'on+lines) → empty ✓ (120 removed; 300 lives inconstants.UPDATE_TIMEOUT_SECONDS) - Raw error_code strings, bare except, print() in src/ → empty ✓
- Token/secret exposure scan → empty ✓ (only
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}in workflow, which is the standard pattern) grep 'update\|version' permissions.py→"version": "read"and"update": "admin"present ✓ (no new command needing registration)- Plugin synchronization map check: this PR adds NO new CLI commands; it modifies internal update infrastructure. The release-version checklist items are satisfied:
pyproject.tomlbumped to 0.60.0 ✓,changelog.pyhas 0.60.0 entry ✓,plugin.jsonandmarketplace.jsonbumped ✓. TheAGENT_CONTEXT/CLAUDE.mdgaps forKBAGENT_UPDATE_TIMEOUTare flagged as NB-3 above. grep 'KBAGENT_UPDATE_TIMEOUT' tests/test_version_service.py tests/test_auto_update.py→ no output;get_update_timeouttests absent from diff → NB-2 above.kbagent version upgrade_commandgap in_check_for_updatesat line 743 → NB-1 above.make check→3947 passed, 8 skipped, 124 deselected, 14 warnings in 129.78sexit 0 ✓- Behavior verification: could not run
kbagent updateagainst the live wheel path because v0.60.0 is not yet released and no asset exists at the expected URL. The conftest_no_wheel_asset_probefixture correctly defaults the HEAD probe to 404 to prevent tests from hitting the network. Thetest_installs_wheel_when_asset_presentinTestPerformUpdateWheelcovers the wheel path at the unit level. install.shsed pattern:echo 'https://github.com/.../releases/tag/v0.59.0' | sed -n 's#.*/releases/tag/v\{0,1\}##p'→0.59.0✓ (macOS BSD sed interprets\{0,1\}correctly in BRE mode)- GitHub Actions versions in
release.ymlvsci.yml: both usecheckout@v5,setup-uv@v7,setup-node@v6,setup-python@v6✓ (consistent) scripts/check_wheel_ui.pyreferenced inrelease.yml→ file exists ✓
Open questions for the author
[NB-1]was thekbagent versionupgrade_commandomission intentional? It is a small overhead (one extra HEAD probe duringkbagent version) but keeping the two paths consistent would make the JSON output more useful for programmatic consumers who readupgrade_commandto decide what to run.
…ests) kbagent-pr-reviewer findings on PR #408 (all non-blocking): - NB-1: kbagent version's JSON `upgrade_command` now resolves the wheel asset too (it was advertising git+ even when the prebuilt wheel exists), matching the `kbagent update` path. - NB-2: add TestGetUpdateTimeout (default / env override / invalid-value fallback) -- the PR description claimed these but the diff lacked them. - NB-3: document KBAGENT_UPDATE_TIMEOUT in AGENT_CONTEXT (context.py) and the CLAUDE.md update block (plugin-sync silent-drift surface). - NIT-2: route the release tag through a job env var in release.yml instead of inlining the GitHub expression into shell run steps.
|
Thanks for the review -- addressed all actionable findings in f001ff5:
NIT-1 left as-is by design:
|
…review) Devin Review on PR #408: the _no_wheel_asset_probe autouse fixture patched httpx.head on the shared module object, so any future httpx.head caller would silently get a 404. Now it returns 404 only for the kbagent release-asset URL and raises loudly on any other URL, so an accidental reliance fails visibly instead of getting a surprise 404.
|
Re: the additional findings on the Devin web view (not posted as GitHub inline comments):
|
Closes #353.
Problem
uv tool install git+https://github.com/keboola/clibuilds the package from source on every install. The wheel build recompiles the bundled React SPA vianpm ci+vite build(the uv cache never covers the npm step), which takes 2-4 minutes on WSL2 and trips the hardcoded 120s auto-update timeout -- sokbagent doctor/kbagent updateprint a false "Auto-update failed".Measured locally (fast Mac): a CLI-only wheel builds in 0.49s, the full UI build in 7.39s -- ~93% of build time is the npm step. On WSL2 that 7s balloons to minutes. Raising the timeout to 300s is not enough (the reporter measured 4m09s warm-cache).
Vrstva 1 -- eliminate the user-side build
release.ymlbuilds the universalpy3-none-anywheel once on Linux CI and uploads it as a Release asset (release: published;workflow_dispatchbackfills older tags like v0.59.0). CI already builds + verifies the wheel via thebuild-windowsjob.install.shbootstrap (curl … | sh) resolves the latest release and installs the prebuilt wheel -- no source build, noghCLI, justcurl+uv. Mirrors the pattern the install guide already uses for uv / Claude Code.[server]extras by default;KBAGENT_NO_SERVER=1opts out.resolve_kbagent_wheel_urlHEAD-probes the asset;build_kbagent_upgrade_commandinstalls it via a PEP 508 direct reference (keboola-agent-cli[server] @ <wheel-url>), falling back togit+when the asset is absent (older releases). Both the startup hook andkbagent updateuse it.Vrstva 2 -- update UX
UPDATE_TIMEOUT_SECONDS(300) +KBAGENT_UPDATE_TIMEOUTenv override replace the two hardcoded 120s timeouts (startup hook +kbagent update).UpdateOutcomeenum distinguishes a build TIMEOUT (slow git+ build, finishes on the next run) from a genuine FAILED, so the startup hook stops printing a false "Auto-update failed"._should_skip_allscans all argv, sokbagent --json updateskips the startup hook -- fixing the reporter's Bug 3 (startup banner disagreeing with the explicit command's JSON output).Net effect
Install and update drop from minutes-of-build to a seconds-long download, on WSL and everywhere else. The timeout stops mattering on the happy path.
Tests
resolve_kbagent_wheel_url: 200 → URL, 404 / HTTP error → None, empty version → None[server], pip fallback, precedence over prerelease, no-tools → None_perform_update: wheel asset present vs git+ fallbackget_update_timeout: default / env override / invalid valueUpdateOutcome.TIMEOUTis not a failure (no re-exec, no "failed" banner)kbagent --json updateskips the startup hookconftestdefaults the HEAD probe to 404 so no test reaches the networkFull gate green --
make check: 3934 passed, 8 skipped.Follow-ups (not in this PR)
kbagent updatereports already-up-to-date when it isn't") looked like a consequence of the timeout race; the wheel fast path + TIMEOUT handling should remove the trigger. Worth confirming live before closing it out separately rather than changing_is_up_to_dateblind.install.sh(the verified client guide currently documents thegit+workaround).