Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude-plugin/marketplace.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"plugins": [
{
"name": "kbagent",
"version": "0.43.6",
"version": "0.43.7",
"source": "./plugins/kbagent",
"description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, call MCP tools, manage dev branches, and debug SQL in workspaces",
"category": "development"
Expand Down
51 changes: 51 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,54 @@ jobs:

- name: Tests
run: uv run pytest tests/ -v -m "not integration"

# Real wheel build on Windows -- the only place the issue #320 fixes can be
# verified end-to-end without a Windows developer machine. GitHub provides
# windows-latest runners (with Node/npm preinstalled) for free, so the
# `npm.cmd` invocation (Bug 1) and the force-include path resolution (Bug 2)
# are exercised against a real `uv build` rather than mocks.
build-windows:
name: Windows wheel build (issue #320)
runs-on: windows-latest
steps:
- uses: actions/checkout@v5

- uses: astral-sh/setup-uv@v6
with:
version: "latest"

- uses: actions/setup-python@v5
with:
python-version: "3.12"

- uses: actions/setup-node@v4
with:
node-version: "20"

# Bug 1: with npm present, the hook must resolve `npm.cmd` via
# shutil.which and run it (shell=False) instead of crashing with
# `FileNotFoundError [WinError 2]`, and the SPA must land in the wheel.
- name: Build wheel WITH bundled UI
run: uv build --wheel

- name: Assert the SPA is bundled (Bug 1 fixed)
run: python scripts/check_wheel_ui.py --expect-ui

- name: Install the built wheel and smoke-test the CLI on Windows
shell: pwsh
run: |
$whl = (Get-ChildItem dist/*.whl | Select-Object -First 1).FullName
uv run --no-project --with "$whl" kbagent --help

# Bug 2: with the SPA bundle skipped, the force-include must still
# resolve (empty `_ui_dist/`) and the wheel must build successfully.
- name: Build CLI-only wheel WITHOUT UI
shell: pwsh
env:
KBAGENT_SKIP_UI_BUILD: "1"
run: |
Remove-Item -Recurse -Force dist -ErrorAction SilentlyContinue
uv build --wheel

- name: Assert CLI-only wheel built without the SPA (Bug 2 fixed)
run: python scripts/check_wheel_ui.py --no-ui
175 changes: 127 additions & 48 deletions hatch_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,68 +24,147 @@
still work; only ``kbagent serve --ui`` will fail with a "no UI bundled"
error pointing the user at install instructions.

**Cross-platform note (issue #320).** Two Windows-specific traps are
handled here:

- ``npm`` on Windows is a batch launcher (``npm.cmd``). A bare
``subprocess.check_call(["npm", ...])`` cannot find it and raises
``FileNotFoundError`` (a subclass of ``OSError``, *not*
``CalledProcessError``). We pass the full path returned by
``shutil.which("npm")`` -- which is ``...\\npm.cmd`` on Windows, and
``CreateProcess`` happily runs a ``.cmd`` via the system shell even with
``shell=False`` -- and we widen the ``except`` to ``OSError`` so a failed
invocation degrades to a UI-less wheel instead of killing the build.
- hatchling's ``force-include`` (see pyproject.toml) fails the whole build
if its source path is missing. Every code path here therefore guarantees
``_ui_dist/`` exists on return (empty is fine -- hatchling includes zero
files from it and the runtime UI detector keys on ``index.html``).

Set ``KBAGENT_SKIP_UI_BUILD=1`` to skip the on-the-fly npm build and ship a
CLI-only wheel deliberately (fast builds; CI exercising the no-UI path).

Wired in via ``[tool.hatch.build.hooks.custom]`` in pyproject.toml.
"""

from __future__ import annotations

import os
import shutil
import subprocess
from collections.abc import Callable
from pathlib import Path
from typing import Any

from hatchling.builders.hooks.plugin.interface import BuildHookInterface
from typing import TYPE_CHECKING, Any

if TYPE_CHECKING:
from hatchling.builders.hooks.plugin.interface import BuildHookInterface
else:
# hatchling is only present in the build environment (uv/pip provision it
# from ``[build-system].requires``). Fall back to ``object`` at runtime so
# the pure helpers below (``_bundle_ui`` / ``_ensure_target``) stay
# importable for unit tests in a plain dev venv that has no hatchling.
try:
from hatchling.builders.hooks.plugin.interface import BuildHookInterface
except ModuleNotFoundError: # pragma: no cover - exercised only without hatchling
BuildHookInterface = object

# Set to "1" to ship a CLI-only wheel: skip bundling the SPA entirely, even if
# a prebuilt ``web/frontend/dist`` exists. The wheel still builds -- an empty
# ``_ui_dist/`` placeholder is created so hatchling's force-include resolves,
# and ``kbagent serve --ui`` surfaces the friendly "no UI bundled" error.
# Useful for fast CLI-only builds and for exercising the no-UI path in CI
# without uninstalling Node.
SKIP_UI_BUILD_ENV = "KBAGENT_SKIP_UI_BUILD"


def _ensure_target(target: Path) -> None:
"""Guarantee the force-include source dir exists so the wheel build works.

hatchling's ``force-include`` fails the *entire* wheel build if its source
path is missing (issue #320, Bug 2). An empty directory satisfies it --
hatchling includes zero files from it, and the runtime UI detector keys on
``index.html`` (absent here), so ``kbagent serve --ui`` degrades to a
friendly "no UI bundled" error rather than a build-time crash.
"""
target.mkdir(parents=True, exist_ok=True)


def _bundle_ui(repo_root: Path, log: Callable[[str], None] = print) -> None:
"""Populate ``src/keboola_agent_cli/_ui_dist/`` for wheel inclusion.

Extracted from :class:`CustomBuildHook` so it can be unit-tested without a
full hatchling build context. ``log`` is injected for the same reason.

Postcondition: ``_ui_dist/`` always exists on return (see
:func:`_ensure_target`).
"""
dist = repo_root / "web" / "frontend" / "dist"
target = repo_root / "src" / "keboola_agent_cli" / "_ui_dist"
frontend_dir = repo_root / "web" / "frontend"

# Always start from a clean slate so stale assets from a previous build
# don't leak into the new wheel. We rebuild target each time.
if target.exists():
shutil.rmtree(target)

# 1) Explicit opt-out: ship a CLI-only wheel. Checked first so it wins even
# over a prebuilt dist -- "skip UI build" means "no UI in this wheel".
if os.environ.get(SKIP_UI_BUILD_ENV) == "1":
log(f"{SKIP_UI_BUILD_ENV}=1 set; skipping SPA bundle (CLI-only wheel).")
_ensure_target(target)
return

# 2) Prebuilt dist on disk -- the maintainer ran ``make web-build`` first.
if (dist / "index.html").exists():
log(f"copying {dist} -> {target}")
shutil.copytree(dist, target)
return

# 3) No prebuilt dist. Build it iff the source tree exists AND npm is on
# PATH. ``shutil.which`` returns the resolved path -- on Windows that is
# ``...\\npm.cmd``; passing the full path lets CreateProcess run the
# batch launcher even with shell=False (a bare "npm" raises WinError 2).
npm = shutil.which("npm")
if not (frontend_dir.exists() and npm):
why = "no `npm` on PATH" if frontend_dir.exists() else "no web/frontend/ dir"
log(
f"WARNING: no prebuilt SPA and {why}; wheel will not bundle the UI. "
"`kbagent serve --ui` will fail until the user rebuilds the SPA manually."
)
_ensure_target(target)
return

log("no prebuilt dist found; running npm build")
try:
subprocess.check_call(
[npm, "ci", "--prefer-offline", "--no-audit", "--no-fund"],
cwd=frontend_dir,
)
subprocess.check_call([npm, "run", "build"], cwd=frontend_dir)
except (subprocess.CalledProcessError, OSError) as exc:
# OSError covers FileNotFoundError/PermissionError from the spawn
# itself (the Windows ``npm.cmd`` trap); CalledProcessError covers a
# non-zero npm exit. Either way: degrade to a UI-less wheel.
log(
f"WARNING: npm build failed ({exc}); wheel will not bundle the UI. "
"`kbagent serve --ui` will fail until the user rebuilds the SPA manually."
)
_ensure_target(target)
return

if not (dist / "index.html").exists():
log("WARNING: build did not produce dist/index.html; skipping UI bundle.")
_ensure_target(target)
return

log(f"copying {dist} -> {target}")
shutil.copytree(dist, target)


class CustomBuildHook(BuildHookInterface):
PLUGIN_NAME = "build-ui"

def initialize(self, version: str, build_data: dict[str, Any]) -> None:
repo_root = Path(self.root).resolve()
dist = repo_root / "web" / "frontend" / "dist"
target = repo_root / "src" / "keboola_agent_cli" / "_ui_dist"
frontend_dir = repo_root / "web" / "frontend"

# Always start from a clean slate so stale assets from a previous
# build don't leak into the new wheel. We rebuild target each time.
if target.exists():
shutil.rmtree(target)

if not (dist / "index.html").exists():
# No prebuilt dist. Try to build it iff npm is present AND the
# source tree exists (true for `uv tool install git+...` and
# local development; false if someone uploaded an sdist that
# excluded `web/`).
if frontend_dir.exists() and shutil.which("npm"):
self._log("no prebuilt dist found; running npm build")
try:
subprocess.check_call(
["npm", "ci", "--prefer-offline", "--no-audit", "--no-fund"],
cwd=frontend_dir,
)
subprocess.check_call(["npm", "run", "build"], cwd=frontend_dir)
except subprocess.CalledProcessError as exc:
self._log(
f"WARNING: npm build failed ({exc}); wheel will not bundle "
"the UI. `kbagent serve --ui` will fail until the user "
"rebuilds the SPA manually.",
)
return
else:
why = "no `npm` on PATH" if frontend_dir.exists() else "no web/frontend/ dir"
self._log(
f"WARNING: no prebuilt SPA and {why}; wheel will not bundle "
"the UI. `kbagent serve --ui` will fail until the user "
"rebuilds the SPA manually.",
)
return

if not (dist / "index.html").exists():
self._log("WARNING: build did not produce dist/index.html; skipping UI bundle.")
return

self._log(f"copying {dist} -> {target}")
shutil.copytree(dist, target)
_bundle_ui(Path(self.root).resolve(), self._log)

def _log(self, message: str) -> None:
# Hatchling's BuilderInterface exposes ``app`` for nicely-formatted
Expand Down
2 changes: 1 addition & 1 deletion plugins/kbagent/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "kbagent",
"version": "0.43.6",
"version": "0.43.7",
"description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, call MCP tools, manage dev branches, and debug SQL in workspaces",
"author": {
"name": "Keboola",
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "keboola-agent-cli"
version = "0.43.6"
version = "0.43.7"
description = "AI-friendly CLI for managing Keboola projects"
readme = "README.md"
requires-python = ">=3.12"
Expand Down
81 changes: 81 additions & 0 deletions scripts/check_wheel_ui.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
"""CI helper: assert whether a built wheel bundles the React SPA.

Used by the Windows wheel-build CI job to verify the issue #320 fixes
end-to-end on a real Windows runner (where no developer machine is needed):

- a normal ``uv build`` must bundle ``_ui_dist/index.html`` (Bug 1: the
``npm.cmd`` invocation actually succeeds), and
- a ``KBAGENT_SKIP_UI_BUILD=1`` build must still produce a valid wheel with
no SPA (Bug 2: the empty ``_ui_dist`` placeholder lets force-include
resolve instead of crashing the build).

The check is OS-independent, so the same assertion guards local builds too.

Usage:
python scripts/check_wheel_ui.py --expect-ui [--dist DIR]
python scripts/check_wheel_ui.py --no-ui [--dist DIR]
"""

from __future__ import annotations

import argparse
import glob
import sys
import zipfile

# Path of the bundled SPA entry point inside the wheel (zip paths always use
# forward slashes, including on Windows).
UI_MARKER = "keboola_agent_cli/_ui_dist/index.html"


def wheel_bundles_ui(wheel_path: str) -> bool:
"""Return True iff the wheel contains the bundled SPA entry point."""
with zipfile.ZipFile(wheel_path) as zf:
return UI_MARKER in zf.namelist()


def main(argv: list[str] | None = None) -> int:
parser = argparse.ArgumentParser(description=__doc__)
group = parser.add_mutually_exclusive_group(required=True)
group.add_argument(
"--expect-ui",
action="store_true",
help="fail unless the wheel bundles the SPA (normal build)",
)
group.add_argument(
"--no-ui",
action="store_true",
help="fail if the wheel bundles the SPA (CLI-only build)",
)
parser.add_argument("--dist", default="dist", help="directory containing the built wheel(s)")
args = parser.parse_args(argv)

wheels = sorted(glob.glob(f"{args.dist}/*.whl"))
if not wheels:
print(f"ERROR: no wheel found in {args.dist}/", file=sys.stderr)
return 1

# Newest by name -- a single build produces one wheel anyway.
wheel = wheels[-1]
has_ui = wheel_bundles_ui(wheel)
print(f"wheel={wheel} bundles_ui={has_ui}")

if args.expect_ui and not has_ui:
print(
f"FAIL: expected '{UI_MARKER}' in the wheel (issue #320 Bug 1 regression).",
file=sys.stderr,
)
return 1
if args.no_ui and has_ui:
print(
f"FAIL: did not expect '{UI_MARKER}' in a CLI-only wheel.",
file=sys.stderr,
)
return 1

print("OK")
return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading