chore: single-source Python version + adapter-convert CI smoke#172
Merged
Conversation
…version Read the pinned Python minor version once (in builder.py from .python-version) and thread it through every runtime Dockerfile as a PYTHON_VERSION build arg. CI workflows also switch from a hardcoded 'uv python install 3.12' to 'uv python install "$(cat .python-version)"'. Bumping the runtime version now needs one edit — .python-version — and propagates to dev, CI, and all four live Docker images (runner, db-service, mock-web, rag-service). Orphan Dockerfiles get the same ARG for consistency in case they're ever revived. Fixes the drift where runner + db_service ran on 3.11 and mock_web + rag on 3.10 while dev / CI / devcontainer were on 3.12 — undercutting the "byte-identical engine" contract.
…ation Add three canonical tests that catch failure modes the existing in-process adapter-convert suite can't see: * Wheel packaging inspection — build the wheel via ``uv build``, unzip, assert every module the CLI reaches at call time (bundle_writer, native, base, adapter_commands, cli.main) is physically present. This is exactly the shape of the GH #37 regression: source file existed on disk, tests passed, PyPI install crashed with ImportError. * Subprocess invocation of ``uv run tolokaforge adapter convert --help`` — proves the console script wires and the CLI imports cleanly under the real Python resolver (no monkeypatch shortcuts). * Import-time symbol check — cheap in-process guard on ``bundle_writer.write_bundle``. Runs under the ``canonical`` marker, so the existing CI test-smoke job picks it up automatically — no workflow-file wiring needed. Closes #37
Contributor
|
Claude finished @CiroGamboa's task —— View job PR Review: single-source Python version + adapter-convert CI smokeReviewed all 14 changed files against Todo:
Rule-by-rule verdict
Strengths
Non-blocking observations (not violations)
VerdictEverything checks out. No Job run · |
… cleanly The previous PR-#172 shape read `.python-version` from the repo root via `_pinned_python_version()` at module import of `tolokaforge.docker.builder`. That works from a source checkout but crashes with `FileNotFoundError` under a wheel install — the wheel lands in `site-packages/tolokaforge/`, and the repo-root dotfile is not part of the package payload. Every consumer that transitively imports the docker submodule (including `tolokaforge docker build` and any code that touches `builder.PYTHON_VERSION`) breaks. Fix (idiomatic hatchling + importlib.resources): - `pyproject.toml` gains a `[tool.hatch.build.targets.wheel.force-include]` block that copies `.python-version` from the repo root into the wheel as `tolokaforge/_python_version.txt` at build time. The repo-root dotfile stays the single source of truth (dev, CI, docker build all read it); the packaged copy is a build artifact. - `_pinned_python_version()` now tries the packaged file first via `importlib.resources.files("tolokaforge").joinpath("_python_version.txt")` and falls back to `repo_root() / ".python-version"` when the packaged file is not present (editable installs, direct source execution). Both branches read the same value on a matching install; the two-branch shape only exists to cover the wheel-install case. Tests: - `test_python_version_pin_ships_in_the_built_wheel` — builds the wheel via `uv build`, inspects it via `zipfile`, asserts `tolokaforge/_python_version.txt` is present and its content matches the repo-root pin. - `test_pinned_python_version_reads_cleanly_in_process` — in-process guard that the resolver returns a non-empty value matching the repo-root pin. Verified: - Fresh wheel install into a Python 3.12 venv: `from tolokaforge.docker.builder import PYTHON_VERSION` returns 3.12. `tolokaforge docker build --help`, `docker status`, and unrelated imports all clean (previously the module-load crash cascaded). - Source install (uv sync from a fresh clone): fallback branch handles it; same value. - Full unit + canonical suite: 2433 pass.
Contributor
|
Claude finished @CiroGamboa's task —— View job PR Review: single-source Python version + adapter-convert CI smokeRe-reviewed all 15 changed files against Todo:
Rule-by-rule verdict
Strengths
Non-blocking observations (not violations)
VerdictEverything checks out. No Job run · |
6 tasks
CiroGamboa
added a commit
that referenced
this pull request
Jul 5, 2026
…kflow (#173) * chore(docker): delete unused Dockerfiles + normalize release-gate workflow Two of the six Dockerfiles under `tolokaforge/docker/dockerfiles/` were never wired into `IMAGE_DEFINITIONS` in `builder.py` — they existed only because the initial open-source release brought them along as drafts. The most recent Claude review of PR #172 flagged them for exactly this reason. `docs/FUTURE_DEVELOPMENT.md` had already noted both as audit candidates. Removing them plus the resolved audit questions and stale doc references. **Deleted:** - `tolokaforge/docker/dockerfiles/json_db.Dockerfile` — bare draft of `db_service.Dockerfile`. Both copy the same source tree (`tolokaforge/env/json_db_service/`) but only `db_service` is production-hardened and wired into `IMAGE_DEFINITIONS`. The `json-db` name that appears in older docs is a *network alias* on the `db-service` container (`tolokaforge/docker/stacks/core.py`), not a separate service. - `tolokaforge/docker/dockerfiles/orchestrator.Dockerfile` — placeholder for a hosted-orchestrator feature that never materialized. `tolokaforge` ships as a CLI; the orchestrator runs on the host, not inside a container. **Documentation swept:** - `docs/DB_SERVICE_API.md` — the "Dockerfile Changes" note now points at `db_service.Dockerfile` (the live sibling). - `docs/FUTURE_DEVELOPMENT.md` — the four audit-question lines covering these two Dockerfiles are removed (question answered by deletion). - `docs/GETTING_STARTED.md`, `docs/TROUBLESHOOTING.md` — the `docker compose up -d json-db mock-web rag-service` command can never have worked (there's no `docker-compose.yml` in the repo — the engine generates compose manifests dynamically via `ServiceStack` classes). Replaced with `uv run tolokaforge docker build` + a note that services auto-start on `tolokaforge run`. **`.github/workflows/release-gate.yml` normalization:** - Bump `astral-sh/setup-uv@v4` (with `version: "latest"`) to `@v7` with `version: "0.9.5"` — matching `ci.yml`. Copied the pin-rationale comment verbatim so future readers see why the version pin matters in-place. Prevents lock-format drift on uv release day. - Replaced the never-worked `docker compose build json-db mock-web rag-service executor` line with `uv run tolokaforge docker build`. `json-db` was a network alias, `executor` doesn't exist anywhere in the codebase, and there's no compose file to back either. The engine's own docker entry point builds every service in `IMAGE_DEFINITIONS` — the same set the removed command was targeting (minus the non-existent ones). **Verification:** - `make lint` clean. - `uv run pytest tests/unit/test_docker_build_context.py -q` — 10 pass (the test that pins which Dockerfiles ship in `IMAGE_DEFINITIONS`; the deletions are invisible to it because it only asserts on the ones that ARE wired). - `uv run pytest tests/ -m "unit or canonical" -q` — 2445 pass. - `make docker-build-core` clean — db-service + runner both build. - Post-flight grep confirms zero lingering references to the deleted files or the broken compose invocation. * review: fold in adjacent workflow + doc cleanup flagged by review GitHub Claude review flagged two additional in-scope items: - `release-gate.yml` still ran `pytest tests/functional/` and `pytest tests/e2e/` — both directories were deleted per `FUTURE_DEVELOPMENT.md` Stage 10 (tests consolidated to `unit / canonical / integration`). The workflow was still failing downstream of the Python-version and compose-command fixes because of these two missing paths. Collapsed the two steps into a single `pytest tests/ -m "unit or canonical"` invocation matching how the repo actually runs tests today. - `docs/TROUBLESHOOTING.md:31` still referenced an `executor container` — the same non-existent name the release-gate compose command was excised for. Replaced with a pointer to the `runner` container + `tolokaforge docker status` for state inspection. Both edits are in the same class of cleanup as the rest of the PR (remove references to deleted / never-existed artifacts). Folding in here so the workflow can genuinely run end-to-end after this lands rather than trading one broken step for another. * docs(future-dev): tick verification-gate checkboxes this PR resolves Follow-up on the review nit — three checkboxes under FUTURE_DEVELOPMENT.md §9.5 "Verification gate" are now satisfied: - No orphaned Dockerfiles — the two orphans (json_db, orchestrator) are deleted in this PR. - builder.py IMAGE_DEFINITIONS matches actual files — every remaining Dockerfile is referenced by exactly one IMAGE_DEFINITIONS entry. - tolokaforge docker build --core succeeds — validated in this PR's verification (make docker-build-core builds runner + db-service cleanly). The other three items in the gate (minimal-runner image, content-hash scope, frozen_mcp_core parity) are separate scopes and stay unchecked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Bundles two independent CI/packaging hygiene items into a single PR so one review cycle unblocks both. Each was previously carried as its own PR (both green, both had passed a full Claude-review round, both awaiting human review); consolidating trims the review overhead without merging together things that aren't cohesive.
What lands in this PR
1. Single-source the Python runtime version from
.python-versionSix engine Dockerfiles,
tolokaforge/docker/builder.py, and five GitHub Actions workflows previously hardcodedpython:3.12-slim/python-version: "3.12"in eight separate places. This meant a Python-version bump was an eight-file coordinated edit with high drift risk.After this change,
.python-versionis the single source.builder.pyreads it and threads it into every Dockerfile as thePYTHON_VERSIONbuild arg; workflows read the same file via a smallsetup-pythonstep. A future3.12 → 3.13bump is a one-line change.Files: 5 workflow YAMLs, 6 Dockerfiles,
tolokaforge/docker/builder.py,tests/unit/test_docker_build_context.py. Total: +67/-28 LOC.2. CI smoke test for
tolokaforge adapter convertpackagingAdds three canonical tests exercising the
adapter convertCLI end-to-end:test_bundle_writer_ships_in_the_built_wheel— inspects the built wheel to confirmbundle_writer.pyis packaged.test_adapter_convert_cli_help_runs_via_subprocess— real subprocess invocation of the installed CLI.test_bundle_writer_module_imports_cleanly— fast import-guard smoke.Prevents the wheel-packaging regression class where
adapter convertbreaks silently because a module fails to make it into the built distribution.Files:
tests/canonical/test_adapter_convert_packaging.py(new, 123 LOC).Why these two together
Both items are:
Bundling them saves a review cycle. The other two PRs from the same batch (an orchestrator DI refactor and a typing-infra stage-1 payload change) stay standalone because they touch different layers and each warrants its own review lens.
Test plan
uv run ruff checkclean on all modified files.uv run pytest tests/canonical/test_adapter_convert_packaging.py tests/unit/test_docker_build_context.py -q— 13 pass.main.native_shared_domain,coding,multi_service,multi_service_advanced,multi_service_postgres) plus two representative recent internal task packs — all produce byte-identicalavg_score_microonmainvs. the branch that has this + the sibling PRs applied.Supersedes
Closes #136 and #137 — content unchanged, single review path.