From c888e774a5d5792c191f91021756802debd5760f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Thu, 20 Aug 2026 10:45:37 +0200 Subject: [PATCH 1/5] fix(AI-3750): stop duplicate _ui_dist wheel entry on VCS-less source builds hatchling's force-include for _ui_dist/ relies on .gitignore-based exclusion to avoid double-adding the path, but that exclusion needs a .git directory to run `git check-ignore` against. A git+ install that hands hatchling a plain exported tree (no .git) skips it, so _ui_dist/index.html gets added twice and the build aborts. Add an explicit wheel-target exclude so it works regardless of VCS state, and document the undocumented Python >=3.12 floor in the README install instructions. Co-Authored-By: Claude Sonnet 5 --- README.md | 2 ++ pyproject.toml | 14 +++++++-- tests/test_build_hook.py | 61 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 71 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 60e144fb..416c6158 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,8 @@ curl -LsSf https://raw.githubusercontent.com/keboola/cli/main/install.sh | sh This installs a **prebuilt wheel** from the latest GitHub release -- a few-seconds download, no source build. Building from `git+` instead recompiles the bundled React SPA via npm on every install, which takes minutes on WSL ([#353](https://github.com/keboola/cli/issues/353)). The script bundles the `[server]` extras by default (set `KBAGENT_NO_SERVER=1` for a CLI-only install) and needs only `curl` + [`uv`](https://docs.astral.sh/uv/). +Requires **Python >=3.12**. `uv` normally fetches a matching interpreter on its own even if your default Python is older, but if it doesn't (offline, or Python downloads disabled) the install fails with `does not satisfy Python>=3.12` -- prefix the command with `UV_PYTHON=3.12` (a standard `uv` env var, works with the `curl | sh` one-liner too) to pin one explicitly. + Prefer to build from source, or pin a specific ref? ```bash diff --git a/pyproject.toml b/pyproject.toml index 45dff823..57856c43 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,9 +47,17 @@ packages = ["src/keboola_agent_cli"] # ``uv tool install git+...`` or ``pip install`` from PyPI without the # user needing the ``web/`` source tree. # -# ``force-include`` overrides the default ``.gitignore`` exclusion so the -# generated dist actually lands in the wheel. Key on the LHS is the path -# on disk; value on the RHS is the path inside the wheel. +# ``force-include`` (below) is meant to be the ONLY way ``_ui_dist/`` enters +# the wheel. Hatchling normally also skips it via .gitignore-based exclusion, +# but that exclusion depends on a ``.git`` directory being present to run +# `git check-ignore` against -- absent one (e.g. a VCS-url install that hands +# hatchling a plain exported source tree, no `.git`), the default package +# globbing is not excluded and picks up `_ui_dist/` too, colliding with +# force-include and aborting the build with "A second file is being added to +# the wheel archive at the same path" (issue AI-3750). `exclude` here is a +# plain glob, evaluated unconditionally regardless of VCS state, so the +# duplicate can't happen either way. +exclude = ["src/keboola_agent_cli/_ui_dist"] [tool.hatch.build.targets.wheel.force-include] "src/keboola_agent_cli/_ui_dist" = "keboola_agent_cli/_ui_dist" diff --git a/tests/test_build_hook.py b/tests/test_build_hook.py index 2cdcb889..18f71376 100644 --- a/tests/test_build_hook.py +++ b/tests/test_build_hook.py @@ -12,14 +12,22 @@ any OS. - **Bug 2** -- an early ``return`` left ``_ui_dist/`` missing, and hatchling's ``force-include`` then failed the whole build. We assert every code path - leaves ``_ui_dist/`` existing on disk. (The force-include interaction itself - is OS-independent and is additionally exercised end-to-end by the CI wheel - build.) + leaves ``_ui_dist/`` existing on disk. + +Also covers issue AI-3750: the CI wheel build always runs inside a real `git` +checkout, so it never exercised the no-``.git`` case (a VCS-url install that +hands hatchling a plain exported source tree). Without a ``.git`` dir, +hatchling's gitignore-based default-exclusion can't run, so the (gitignored) +``_ui_dist/`` the hook just populated got picked up by BOTH the default +package globbing and ``force-include``, and the wheel build aborted with "A +second file is being added to the wheel archive at the same path". See +``TestForceIncludeNoDuplicate`` below for the end-to-end regression test. """ from __future__ import annotations import importlib.util +import shutil import subprocess import sys import zipfile @@ -289,3 +297,50 @@ def test_no_ui_fails_on_ui_wheel(self, tmp_path: Path) -> None: def test_missing_wheel_is_an_error(self, tmp_path: Path) -> None: assert check_wheel_ui.main(["--expect-ui", "--dist", str(tmp_path)]) == 1 + + +class TestForceIncludeNoDuplicate: + """AI-3750: building without a ``.git`` dir must not duplicate ``_ui_dist/``. + + Reproduces the real failure end-to-end (not mocked): a minimal project + laid out with the actual ``pyproject.toml`` / ``hatch_build.py``, a + prebuilt SPA dist on disk (so the hook populates ``_ui_dist/`` with a real + file), and deliberately NO ``.git`` directory -- the exact shape of a + VCS-url install where the build backend never sees repo history. + """ + + def test_wheel_builds_without_git_directory(self, tmp_path: Path) -> None: + if shutil.which("uv") is None: + pytest.skip("uv not on PATH") + + repo_root = Path(__file__).resolve().parents[1] + project = tmp_path / "project" + (project / "src" / "keboola_agent_cli").mkdir(parents=True) + (project / "src" / "keboola_agent_cli" / "__init__.py").write_text("", encoding="utf-8") + (project / "src" / "keboola_agent_cli" / "py.typed").write_text("", encoding="utf-8") + (project / "scripts").mkdir() + shutil.copy( + repo_root / "scripts" / "hatch_build.py", project / "scripts" / "hatch_build.py" + ) + shutil.copy(repo_root / "pyproject.toml", project / "pyproject.toml") + (project / "README.md").write_text("test project", encoding="utf-8") + + dist = project / "web" / "frontend" / "dist" + dist.mkdir(parents=True) + (dist / "index.html").write_text("app", encoding="utf-8") + + assert not (project / ".git").exists() + + result = subprocess.run( + ["uv", "build", "--wheel", "-o", str(tmp_path / "out")], + cwd=project, + capture_output=True, + text=True, + timeout=120, + ) + assert result.returncode == 0, result.stderr + + (wheel,) = list((tmp_path / "out").glob("*.whl")) + with zipfile.ZipFile(wheel) as zf: + ui_entries = [n for n in zf.namelist() if n.endswith("_ui_dist/index.html")] + assert ui_entries == ["keboola_agent_cli/_ui_dist/index.html"] From 557ad04f702ec91862badb46c8169e31f4981d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Fri, 21 Aug 2026 15:39:30 +0200 Subject: [PATCH 2/5] fix(AI-3750): drop internal Linear issue references from code comments Miro flagged that AI-3750 references in pyproject.toml/test docstrings aren't publicly accessible; the technical description stands on its own without them. --- pyproject.toml | 2 +- tests/test_build_hook.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 57856c43..88b7e214 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ packages = ["src/keboola_agent_cli"] # hatchling a plain exported source tree, no `.git`), the default package # globbing is not excluded and picks up `_ui_dist/` too, colliding with # force-include and aborting the build with "A second file is being added to -# the wheel archive at the same path" (issue AI-3750). `exclude` here is a +# the wheel archive at the same path". `exclude` here is a # plain glob, evaluated unconditionally regardless of VCS state, so the # duplicate can't happen either way. exclude = ["src/keboola_agent_cli/_ui_dist"] diff --git a/tests/test_build_hook.py b/tests/test_build_hook.py index 18f71376..b775b568 100644 --- a/tests/test_build_hook.py +++ b/tests/test_build_hook.py @@ -14,7 +14,7 @@ ``force-include`` then failed the whole build. We assert every code path leaves ``_ui_dist/`` existing on disk. -Also covers issue AI-3750: the CI wheel build always runs inside a real `git` +Also covers a gap where the CI wheel build always runs inside a real `git` checkout, so it never exercised the no-``.git`` case (a VCS-url install that hands hatchling a plain exported source tree). Without a ``.git`` dir, hatchling's gitignore-based default-exclusion can't run, so the (gitignored) @@ -300,7 +300,7 @@ def test_missing_wheel_is_an_error(self, tmp_path: Path) -> None: class TestForceIncludeNoDuplicate: - """AI-3750: building without a ``.git`` dir must not duplicate ``_ui_dist/``. + """Building without a ``.git`` dir must not duplicate ``_ui_dist/``. Reproduces the real failure end-to-end (not mocked): a minimal project laid out with the actual ``pyproject.toml`` / ``hatch_build.py``, a From 354ee50a51823183935faf07c307fc58d3836c8e Mon Sep 17 00:00:00 2001 From: MiroCillik Date: Fri, 21 Aug 2026 16:28:55 +0200 Subject: [PATCH 3/5] fix(AI-3750): correct the install docs workaround and harden the build regression test Follow-up on review of #623. Four issues, none in the one-line `exclude` fix itself (which is load-bearing and verified). - README: `UV_PYTHON=3.12 curl ... | sh` assigns the var to `curl`, not `sh`, so neither the script nor the `uv` it invokes ever saw it -- the documented workaround for `does not satisfy Python>=3.12` was inert. Show the `| UV_PYTHON=3.12 sh` and `export` forms instead. - pyproject/test docstrings: the root cause was misattributed. Hatchling never runs `git check-ignore`; it parses the `.gitignore` *file* found by `locate_file(root, ".gitignore", boundary=".git")`, where `.git` is the boundary that STOPS the upward search rather than a prerequisite. A `git+` install ships a tracked `.gitignore` and excludes fine; the shape that actually failed is the sdist, whose `include` list omits it. - The regression test asserted "no `.git`", which guards the wrong invariant: with neither `.git` nor `.gitignore` in the fixture, hatchling searched every ancestor of `tmp_path`, so a `TMPDIR` under any checkout let an ancestor `.gitignore` supply the exclusion and the test passed with the fix reverted. Create an empty `.git` (the boundary) and assert no local `.gitignore`. Verified: reverting `exclude` now fails both with a normal TMPDIR and with TMPDIR beneath an ancestor `.gitignore`. - The module inherited `KBAGENT_SKIP_UI_BUILD` (exported in parts of CI), which makes `_bundle_ui` ship an empty `_ui_dist/`; 5 tests then failed on a green build with messages pointing elsewhere. Clear it in an autouse fixture. Also raise the wheel-build timeout to 300s for cold-cache Windows runners. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 +- pyproject.toml | 19 ++++++++++-------- tests/test_build_hook.py | 43 ++++++++++++++++++++++++++++++---------- 3 files changed, 45 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 416c6158..93ad8bf0 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ curl -LsSf https://raw.githubusercontent.com/keboola/cli/main/install.sh | sh This installs a **prebuilt wheel** from the latest GitHub release -- a few-seconds download, no source build. Building from `git+` instead recompiles the bundled React SPA via npm on every install, which takes minutes on WSL ([#353](https://github.com/keboola/cli/issues/353)). The script bundles the `[server]` extras by default (set `KBAGENT_NO_SERVER=1` for a CLI-only install) and needs only `curl` + [`uv`](https://docs.astral.sh/uv/). -Requires **Python >=3.12**. `uv` normally fetches a matching interpreter on its own even if your default Python is older, but if it doesn't (offline, or Python downloads disabled) the install fails with `does not satisfy Python>=3.12` -- prefix the command with `UV_PYTHON=3.12` (a standard `uv` env var, works with the `curl | sh` one-liner too) to pin one explicitly. +Requires **Python >=3.12**. `uv` normally fetches a matching interpreter on its own even if your default Python is older, but if it doesn't (offline, or Python downloads disabled) the install fails with `does not satisfy Python>=3.12` -- set `UV_PYTHON=3.12` (a standard `uv` env var) to pin one explicitly. It has to reach `sh`, not `curl` -- `curl -LsSf https://raw.githubusercontent.com/keboola/cli/main/install.sh | UV_PYTHON=3.12 sh`, or `export UV_PYTHON=3.12` beforehand. Prefixing the whole pipeline (`UV_PYTHON=3.12 curl ... | sh`) assigns it to `curl`, where it has no effect. Prefer to build from source, or pin a specific ref? diff --git a/pyproject.toml b/pyproject.toml index 88b7e214..eb2c8f4e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -49,14 +49,17 @@ packages = ["src/keboola_agent_cli"] # # ``force-include`` (below) is meant to be the ONLY way ``_ui_dist/`` enters # the wheel. Hatchling normally also skips it via .gitignore-based exclusion, -# but that exclusion depends on a ``.git`` directory being present to run -# `git check-ignore` against -- absent one (e.g. a VCS-url install that hands -# hatchling a plain exported source tree, no `.git`), the default package -# globbing is not excluded and picks up `_ui_dist/` too, colliding with -# force-include and aborting the build with "A second file is being added to -# the wheel archive at the same path". `exclude` here is a -# plain glob, evaluated unconditionally regardless of VCS state, so the -# duplicate can't happen either way. +# but that exclusion has to FIND a ``.gitignore`` first: it parses the file +# located by ``locate_file(root, ".gitignore", boundary=".git")`` and hands the +# patterns to pathspec. It never shells out to `git check-ignore`, and a `.git` +# directory is the boundary that STOPS that upward search rather than a +# prerequisite for it. So when no ``.gitignore`` is reachable -- notably an +# sdist build, because ``[tool.hatch.build.targets.sdist].include`` below does +# not ship ``.gitignore`` -- nothing excludes ``_ui_dist/``, the default +# package globbing picks it up alongside force-include, and the build aborts +# with "A second file is being added to the wheel archive at the same path". +# `exclude` here is a plain glob, evaluated unconditionally regardless of VCS +# state, so the duplicate can't happen either way. exclude = ["src/keboola_agent_cli/_ui_dist"] [tool.hatch.build.targets.wheel.force-include] diff --git a/tests/test_build_hook.py b/tests/test_build_hook.py index b775b568..bab167c9 100644 --- a/tests/test_build_hook.py +++ b/tests/test_build_hook.py @@ -14,11 +14,13 @@ ``force-include`` then failed the whole build. We assert every code path leaves ``_ui_dist/`` existing on disk. -Also covers a gap where the CI wheel build always runs inside a real `git` -checkout, so it never exercised the no-``.git`` case (a VCS-url install that -hands hatchling a plain exported source tree). Without a ``.git`` dir, -hatchling's gitignore-based default-exclusion can't run, so the (gitignored) -``_ui_dist/`` the hook just populated got picked up by BOTH the default +Also covers a gap where the CI wheel build always runs inside a checkout that +has a ``.gitignore``, so it never exercised the case where hatchling cannot +reach one -- notably an sdist build, since the sdist ``include`` list does not +ship ``.gitignore``. Hatchling's default exclusion parses the ``.gitignore`` +*file* it locates by walking up from the project root (stopping at a ``.git`` +boundary); with no such file in reach, nothing excludes the (gitignored) +``_ui_dist/`` the hook just populated, so it got picked up by BOTH the default package globbing and ``force-include``, and the wheel build aborted with "A second file is being added to the wheel archive at the same path". See ``TestForceIncludeNoDuplicate`` below for the end-to-end regression test. @@ -48,6 +50,21 @@ sys.modules["hatch_build"] = hatch_build _spec.loader.exec_module(hatch_build) + +@pytest.fixture(autouse=True) +def _neutralize_skip_ui_build(monkeypatch: pytest.MonkeyPatch) -> None: + """Clear the ambient ``KBAGENT_SKIP_UI_BUILD`` knob for every test here. + + It is a documented flag exported by parts of CI (see + ``.github/workflows/ci.yml``). Left set, ``_bundle_ui`` ships an EMPTY + ``_ui_dist/``, so every test that asserts real bundle contents -- including + the end-to-end wheel build, which inherits this environment through + ``uv build`` -- fails for a reason unrelated to what it covers. Tests that + exercise the skip path opt in explicitly with ``monkeypatch.setenv``. + """ + monkeypatch.delenv(hatch_build.SKIP_UI_BUILD_ENV, raising=False) + + # The CI wheel-content assertion helper lives in ``scripts/`` -- load it the # same way so its logic is regression-tested in normal (ubuntu) CI, not only by # the Windows wheel-build job that calls it as a subprocess. @@ -300,13 +317,13 @@ def test_missing_wheel_is_an_error(self, tmp_path: Path) -> None: class TestForceIncludeNoDuplicate: - """Building without a ``.git`` dir must not duplicate ``_ui_dist/``. + """Building with no reachable ``.gitignore`` must not duplicate ``_ui_dist/``. Reproduces the real failure end-to-end (not mocked): a minimal project laid out with the actual ``pyproject.toml`` / ``hatch_build.py``, a prebuilt SPA dist on disk (so the hook populates ``_ui_dist/`` with a real - file), and deliberately NO ``.git`` directory -- the exact shape of a - VCS-url install where the build backend never sees repo history. + file), and deliberately no ``.gitignore`` for hatchling to find -- the + shape of an sdist build, whose ``include`` list omits ``.gitignore``. """ def test_wheel_builds_without_git_directory(self, tmp_path: Path) -> None: @@ -329,14 +346,20 @@ def test_wheel_builds_without_git_directory(self, tmp_path: Path) -> None: dist.mkdir(parents=True) (dist / "index.html").write_text("app", encoding="utf-8") - assert not (project / ".git").exists() + # An empty ``.git`` dir is the boundary that halts hatchling's upward + # ``.gitignore`` search, pinning the no-exclusion state regardless of + # what sits above ``tmp_path``. Without it, a ``TMPDIR`` located inside + # any checkout lets an ancestor ``.gitignore`` supply the exclusion and + # this test passes even with the ``exclude`` fix reverted. + (project / ".git").mkdir() + assert not (project / ".gitignore").exists() result = subprocess.run( ["uv", "build", "--wheel", "-o", str(tmp_path / "out")], cwd=project, capture_output=True, text=True, - timeout=120, + timeout=300, ) assert result.returncode == 0, result.stderr From 401fe80f2cdecb7e3fa6e321ba9a735423149e59 Mon Sep 17 00:00:00 2001 From: MiroCillik Date: Sun, 23 Aug 2026 20:10:40 +0200 Subject: [PATCH 4/5] fix(AI-3750): correct the hook docstring, guard the sdist, and fix the test name Second review round on #623. All five findings were reproduced before fixing. - scripts/hatch_build.py: step 3 of the module docstring said the dist is copied into `_ui_dist/` "so hatchling's normal package collection picks it up". This PR makes that false -- collection now explicitly skips the dir and `force-include` is the only path in. Someone trusting the old wording could drop either half and ship a UI-less wheel or reintroduce the duplicate; only the Windows `check_wheel_ui --expect-ui` step catches the former. - pyproject.toml: give the sdist the symmetric guard. Reproduced the leak: `include` has `src/`, so with `_ui_dist/` already on disk (an earlier editable install) and no reachable `.gitignore`, `uv build --sdist` ships src/keboola_agent_cli/_ui_dist/index.html -- generated assets as source. A clean tree was never affected: the hook is wheel-scoped, so an sdist build never creates the dir. Downstream impact is nil either way because `_bundle_ui` rmtree's `_ui_dist` before every build, so a stale copy cannot reach a wheel; this is sdist hygiene, not a correctness fix. - tests: `test_wheel_builds_without_git_directory` now deliberately CREATES `.git` (the boundary that stops hatchling's upward `.gitignore` search), so the name said the opposite of the body and invited someone to "fix" the body into a false negative. Renamed to match what it covers. - tests: the autouse fixture's docstring claimed KBAGENT_SKIP_UI_BUILD is "exported by parts of CI". It is not -- ci.yml scopes it to one step's `env:`, the only occurrence in .github/ or the Makefile. The fixture still earns its place (a developer's exported shell flag breaks 5 tests here, measured); only the rationale was wrong. - README: the workaround hard-pinned `UV_PYTHON=3.12` while requires-python is >=3.12, so on a 3.13-only machine with downloads disabled -- exactly the stated precondition -- it failed where no pin would have worked. Verified end to end: sdist no longer carries `_ui_dist`; sdist -> wheel (the originally broken shape, no `.gitignore` in the tarball) builds with exactly one `_ui_dist/index.html` carrying fresh content; reverting the wheel `exclude` still fails the regression test both with a normal TMPDIR and with TMPDIR beneath an ancestor `.gitignore`. Full suite 5692 passed, 172 skipped; every `make check` gate passes except the pre-existing `changelog-check` staleness, which fails identically on main. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 +- pyproject.toml | 8 ++++++++ scripts/hatch_build.py | 11 ++++++++--- tests/test_build_hook.py | 15 ++++++++------- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 93ad8bf0..6ba4b855 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ curl -LsSf https://raw.githubusercontent.com/keboola/cli/main/install.sh | sh This installs a **prebuilt wheel** from the latest GitHub release -- a few-seconds download, no source build. Building from `git+` instead recompiles the bundled React SPA via npm on every install, which takes minutes on WSL ([#353](https://github.com/keboola/cli/issues/353)). The script bundles the `[server]` extras by default (set `KBAGENT_NO_SERVER=1` for a CLI-only install) and needs only `curl` + [`uv`](https://docs.astral.sh/uv/). -Requires **Python >=3.12**. `uv` normally fetches a matching interpreter on its own even if your default Python is older, but if it doesn't (offline, or Python downloads disabled) the install fails with `does not satisfy Python>=3.12` -- set `UV_PYTHON=3.12` (a standard `uv` env var) to pin one explicitly. It has to reach `sh`, not `curl` -- `curl -LsSf https://raw.githubusercontent.com/keboola/cli/main/install.sh | UV_PYTHON=3.12 sh`, or `export UV_PYTHON=3.12` beforehand. Prefixing the whole pipeline (`UV_PYTHON=3.12 curl ... | sh`) assigns it to `curl`, where it has no effect. +Requires **Python >=3.12**. `uv` normally fetches a matching interpreter on its own even if your default Python is older, but if it doesn't (offline, or Python downloads disabled) the install fails with `does not satisfy Python>=3.12` -- set `UV_PYTHON` (a standard `uv` env var) to a 3.12-or-newer interpreter you actually have -- `UV_PYTHON=3.12`, `UV_PYTHON=3.13`, or a full path. It has to reach `sh`, not `curl` -- `curl -LsSf https://raw.githubusercontent.com/keboola/cli/main/install.sh | UV_PYTHON=3.12 sh`, or `export UV_PYTHON=3.12` beforehand. Prefixing the whole pipeline (`UV_PYTHON=3.12 curl ... | sh`) assigns it to `curl`, where it has no effect. Prefer to build from source, or pin a specific ref? diff --git a/pyproject.toml b/pyproject.toml index eb2c8f4e..d3312cae 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,6 +94,14 @@ exclude = [ "web/frontend/dist", "web/frontend/tsconfig.tsbuildinfo", "web/backend", + # Generated SPA output must never ship as *source*. ``include`` above has + # ``src/``, and the same VCS-state dependence described on the wheel + # target's ``exclude`` applies here: with no reachable ``.gitignore`` and a + # ``_ui_dist/`` already on disk (an earlier editable install), the sdist + # would carry a prebuilt SPA. Harmless downstream -- ``_bundle_ui`` + # rmtree's the dir before every build, so a stale copy can never reach a + # wheel -- but it bloats the sdist with bytes that are not source. + "src/keboola_agent_cli/_ui_dist", ] [tool.pytest.ini_options] diff --git a/scripts/hatch_build.py b/scripts/hatch_build.py index 49fd648f..67f2f2cf 100644 --- a/scripts/hatch_build.py +++ b/scripts/hatch_build.py @@ -15,9 +15,14 @@ 2. **Building it on the fly** if missing AND ``npm`` is available -- covers the ``uv tool install git+...`` happy path on machines that already have Node 20+ for other reasons. -3. **Copying** the dist into ``src/keboola_agent_cli/_ui_dist/`` so - hatchling's normal package collection picks it up (the dir is in - ``.gitignore`` to avoid checking in generated assets). +3. **Copying** the dist into ``src/keboola_agent_cli/_ui_dist/``, which + reaches the wheel *only* through ``force-include``. The wheel target + deliberately ``exclude``s that (gitignored) dir from hatchling's normal + package collection, because having both paths pick it up aborts the build + with "A second file is being added to the wheel archive at the same path". + Dropping either half breaks a build: no ``force-include`` ships a UI-less + wheel, no ``exclude`` reintroduces the duplicate. See the comment on + ``[tool.hatch.build.targets.wheel].exclude`` in ``pyproject.toml``. If neither a prebuilt dist nor ``npm`` is available, the hook logs a warning and lets the wheel build proceed without the UI. The CLI will diff --git a/tests/test_build_hook.py b/tests/test_build_hook.py index bab167c9..63e0d759 100644 --- a/tests/test_build_hook.py +++ b/tests/test_build_hook.py @@ -55,12 +55,13 @@ def _neutralize_skip_ui_build(monkeypatch: pytest.MonkeyPatch) -> None: """Clear the ambient ``KBAGENT_SKIP_UI_BUILD`` knob for every test here. - It is a documented flag exported by parts of CI (see - ``.github/workflows/ci.yml``). Left set, ``_bundle_ui`` ships an EMPTY - ``_ui_dist/``, so every test that asserts real bundle contents -- including - the end-to-end wheel build, which inherits this environment through - ``uv build`` -- fails for a reason unrelated to what it covers. Tests that - exercise the skip path opt in explicitly with ``monkeypatch.setenv``. + Not a CI concern: ``ci.yml`` scopes that flag to a single step's ``env:``, + so it never reaches the pytest step. This guards the developer who + exported it in their shell. Left set, ``_bundle_ui`` ships an EMPTY + ``_ui_dist/``, and 5 tests in this module then fail with messages about + bundle contents instead of naming the env var -- including the end-to-end + wheel build, which inherits this environment through ``uv build``. Tests + that exercise the skip path opt in explicitly with ``monkeypatch.setenv``. """ monkeypatch.delenv(hatch_build.SKIP_UI_BUILD_ENV, raising=False) @@ -326,7 +327,7 @@ class TestForceIncludeNoDuplicate: shape of an sdist build, whose ``include`` list omits ``.gitignore``. """ - def test_wheel_builds_without_git_directory(self, tmp_path: Path) -> None: + def test_wheel_builds_without_reachable_gitignore(self, tmp_path: Path) -> None: if shutil.which("uv") is None: pytest.skip("uv not on PATH") From 134d1eec83118cf0471090d7c5881d40e7644240 Mon Sep 17 00:00:00 2001 From: MiroCillik Date: Sun, 23 Aug 2026 20:24:54 +0200 Subject: [PATCH 5/5] fix(AI-3750): replace the root-cause explanation with the bisected one @padak's second review was right that the sdist justification is false -- hatchling force-includes the located VCS exclusion file, so `.gitignore` IS in the tarball (verified: keboola_cli-0.86.0/.gitignore). But his replacement framing, and the original `.git`/`git check-ignore` one, and my `no reachable .gitignore` one are all wrong too. Bisected instead of reasoned: clean clone of main, `.git` AND `.gitignore` present, prebuilt SPA on disk, wheel `exclude` removed: hatchling 1.27.0 builds OK hatchling 1.28.0 builds OK hatchling 1.29.0 builds OK hatchling 1.30.0 FAILS (duplicate) hatchling 1.31.0 FAILS hatchling 1.32.0 FAILS So VCS state is not the trigger: a directory-shaped `force-include` collides with `packages` collection whenever `_ui_dist/` exists at collection time, from hatchling 1.30.0 on, and `requires = ["hatchling"]` is unpinned. On 1.27.0 deleting `.gitignore` outright still builds fine, which rules the gitignore mechanism out as the explanation entirely. Corrected in all four places that carried a mechanism claim: the wheel `exclude` comment, the sdist `exclude` comment, the test module docstring and the class docstring. The comments now say what was measured and warn against relying on hatchling's gitignore exclusion, which is version- and layout-dependent (an ancestor `.gitignore` suppresses the duplicate in a minimal fixture; this repo's own root `.gitignore` does not). `scripts/hatch_build.py` needed no change -- its wording was already version-neutral. Renamed the test once more: `test_wheel_build_does_not_duplicate_ui_dist`, which states the invariant rather than a mechanism that turned out to be the wrong one. The `.git` boundary in the fixture stays and is still load-bearing for hermeticity (re-verified: removing it lets an ancestor `.gitignore` pass the test with the fix reverted). Also verified the fix is version-independent: exactly one `_ui_dist/index.html` on hatchling 1.27, 1.29, 1.30 and 1.32. Full suite 5692 passed, 172 skipped; ruff, typecheck, loc/version/ command-sync gates green. Co-Authored-By: Claude Opus 5 (1M context) --- pyproject.toml | 42 +++++++++++++++++++++++----------------- tests/test_build_hook.py | 42 ++++++++++++++++++++++------------------ 2 files changed, 47 insertions(+), 37 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d3312cae..88be469d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,18 +48,24 @@ packages = ["src/keboola_agent_cli"] # user needing the ``web/`` source tree. # # ``force-include`` (below) is meant to be the ONLY way ``_ui_dist/`` enters -# the wheel. Hatchling normally also skips it via .gitignore-based exclusion, -# but that exclusion has to FIND a ``.gitignore`` first: it parses the file -# located by ``locate_file(root, ".gitignore", boundary=".git")`` and hands the -# patterns to pathspec. It never shells out to `git check-ignore`, and a `.git` -# directory is the boundary that STOPS that upward search rather than a -# prerequisite for it. So when no ``.gitignore`` is reachable -- notably an -# sdist build, because ``[tool.hatch.build.targets.sdist].include`` below does -# not ship ``.gitignore`` -- nothing excludes ``_ui_dist/``, the default -# package globbing picks it up alongside force-include, and the build aborts -# with "A second file is being added to the wheel archive at the same path". -# `exclude` here is a plain glob, evaluated unconditionally regardless of VCS -# state, so the duplicate can't happen either way. +# the wheel. Without this ``exclude``, the ``packages`` collection above picks +# the dir up as well and the build aborts with "A second file is being added to +# the wheel archive at the same path". +# +# Do NOT count on hatchling's .gitignore-based exclusion to prevent that; it is +# version-dependent and it does not save this repo. Bisected on a clean +# checkout WITH both ``.git`` and ``.gitignore`` present and a prebuilt SPA on +# disk: hatchling <= 1.29.0 builds, >= 1.30.0 fails (1.30 / 1.31 / 1.32 all +# reproduce), and ``requires = ["hatchling"]`` above is unpinned, so every +# build takes the latest. Whether that exclusion fires at all also depends on +# where the ``.gitignore`` sits relative to the project root and the ``.git`` +# boundary hatchling stops its upward search at -- an ancestor ``.gitignore`` +# suppressed the duplicate in a minimal fixture, this repo's own root +# ``.gitignore`` does not. +# +# ``exclude`` is a plain glob, evaluated unconditionally, so it holds across +# both VCS state and hatchling version: verified shipping exactly one +# ``_ui_dist/index.html`` on hatchling 1.27, 1.29, 1.30 and 1.32. exclude = ["src/keboola_agent_cli/_ui_dist"] [tool.hatch.build.targets.wheel.force-include] @@ -95,12 +101,12 @@ exclude = [ "web/frontend/tsconfig.tsbuildinfo", "web/backend", # Generated SPA output must never ship as *source*. ``include`` above has - # ``src/``, and the same VCS-state dependence described on the wheel - # target's ``exclude`` applies here: with no reachable ``.gitignore`` and a - # ``_ui_dist/`` already on disk (an earlier editable install), the sdist - # would carry a prebuilt SPA. Harmless downstream -- ``_bundle_ui`` - # rmtree's the dir before every build, so a stale copy can never reach a - # wheel -- but it bloats the sdist with bytes that are not source. + # ``src/``, so with a ``_ui_dist/`` already on disk (an earlier editable + # install) and hatchling's .gitignore exclusion not applying -- as + # unreliable here as it is for the wheel target above -- the sdist carries + # a prebuilt SPA. Harmless downstream, since ``_bundle_ui`` rmtree's the + # dir before every build and a stale copy can never reach a wheel, but it + # ships bytes that are not source. "src/keboola_agent_cli/_ui_dist", ] diff --git a/tests/test_build_hook.py b/tests/test_build_hook.py index 63e0d759..4db755c7 100644 --- a/tests/test_build_hook.py +++ b/tests/test_build_hook.py @@ -14,15 +14,13 @@ ``force-include`` then failed the whole build. We assert every code path leaves ``_ui_dist/`` existing on disk. -Also covers a gap where the CI wheel build always runs inside a checkout that -has a ``.gitignore``, so it never exercised the case where hatchling cannot -reach one -- notably an sdist build, since the sdist ``include`` list does not -ship ``.gitignore``. Hatchling's default exclusion parses the ``.gitignore`` -*file* it locates by walking up from the project root (stopping at a ``.git`` -boundary); with no such file in reach, nothing excludes the (gitignored) -``_ui_dist/`` the hook just populated, so it got picked up by BOTH the default -package globbing and ``force-include``, and the wheel build aborted with "A -second file is being added to the wheel archive at the same path". See +Also covers the duplicate-path failure that the wheel ``exclude`` in +``pyproject.toml`` guards against: with ``_ui_dist/`` populated at collection +time, hatchling's ``packages`` selection picks it up alongside the +``force-include`` and the build aborts with "A second file is being added to +the wheel archive at the same path". That reproduces on an ordinary checkout +(``.git`` and ``.gitignore`` both present) from hatchling 1.30.0 onwards -- +1.29.0 and earlier build fine -- so it is not tied to any VCS layout. See ``TestForceIncludeNoDuplicate`` below for the end-to-end regression test. """ @@ -318,16 +316,22 @@ def test_missing_wheel_is_an_error(self, tmp_path: Path) -> None: class TestForceIncludeNoDuplicate: - """Building with no reachable ``.gitignore`` must not duplicate ``_ui_dist/``. + """A populated ``_ui_dist/`` must not land in the wheel twice. Reproduces the real failure end-to-end (not mocked): a minimal project - laid out with the actual ``pyproject.toml`` / ``hatch_build.py``, a - prebuilt SPA dist on disk (so the hook populates ``_ui_dist/`` with a real - file), and deliberately no ``.gitignore`` for hatchling to find -- the - shape of an sdist build, whose ``include`` list omits ``.gitignore``. + laid out with the actual ``pyproject.toml`` / ``hatch_build.py`` and a + prebuilt SPA dist on disk, so the hook populates ``_ui_dist/`` with a real + file. + + The fixture also puts hatchling's .gitignore-based exclusion deliberately + out of reach. That exclusion is *not* what ``exclude`` stands in for -- + the duplicate reproduces in a normal checkout too on hatchling >= 1.30 -- + but pinning it here stops the assertion from passing for an incidental + reason, e.g. under a ``TMPDIR`` that happens to sit below some other + ``.gitignore``. """ - def test_wheel_builds_without_reachable_gitignore(self, tmp_path: Path) -> None: + def test_wheel_build_does_not_duplicate_ui_dist(self, tmp_path: Path) -> None: if shutil.which("uv") is None: pytest.skip("uv not on PATH") @@ -348,10 +352,10 @@ def test_wheel_builds_without_reachable_gitignore(self, tmp_path: Path) -> None: (dist / "index.html").write_text("app", encoding="utf-8") # An empty ``.git`` dir is the boundary that halts hatchling's upward - # ``.gitignore`` search, pinning the no-exclusion state regardless of - # what sits above ``tmp_path``. Without it, a ``TMPDIR`` located inside - # any checkout lets an ancestor ``.gitignore`` supply the exclusion and - # this test passes even with the ``exclude`` fix reverted. + # ``.gitignore`` search, so no ancestor ``.gitignore`` can quietly + # supply an exclusion. Verified load-bearing: without it, a ``TMPDIR`` + # below any tree carrying a matching ``.gitignore`` makes this test + # pass even with the ``exclude`` fix reverted. (project / ".git").mkdir() assert not (project / ".gitignore").exists()