From 40effa1aa14af1a4bbe0d68018b7512364e89d85 Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Sun, 26 Jul 2026 00:43:59 -0600 Subject: [PATCH] fix(build): rebuild the native binary when Rust sources change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `solstone-core` is a maturin package whose Rust sources live outside its package directory — maturin is pointed at ../../core. Nothing that watches the build watched there, so two gates were both open: - uv's default cache key for a path dependency is that package's own pyproject.toml, so `uv sync` reported "no changes" after a Rust-only edit and left the previously built binary installed; - `.installed`'s prerequisites did not move either, so make short-circuited and never called uv at all. PYTEST is $(VENV_BIN)/pytest, not `uv run pytest`, so nothing else triggered a sync. Any environment whose .venv predated a native change kept executing the older binary while `make ci` reported green. test_make_skills_idempotent is the only test that copies and runs .venv/bin/solstone-core, so it was the only one to notice — a pre-port binary has no native `skills` command, routes to the removed Python delegation, and exits 78. Declare the real build inputs as uv cache-keys so a sync rebuilds, and add a content-hashed .rust-core-hash stamp so `make install` runs a sync at all after a Rust-only change. Both halves are required; cache-keys alone never fires because make short-circuits first. Also stop swallowing the binary's own explanation in the idempotency test. check=True raises CalledProcessError, whose pytest summary carries the exit status and nothing else, discarding the one sentence that named the cause. --- .gitignore | 1 + Makefile | 19 +++++++- packages/solstone-core/pyproject.toml | 20 +++++++++ tests/test_journal_skill.py | 63 ++++++++++++--------------- 4 files changed, 68 insertions(+), 35 deletions(-) diff --git a/.gitignore b/.gitignore index f3484ccd8..3248c1cb4 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,7 @@ tests/fixtures/journal/chronicle/*/health/*.jsonl /config*.py .installed .python-version-hash +.rust-core-hash /journal/* /identity.md .agents/ diff --git a/Makefile b/Makefile index ccd3a95f6..4bc18d399 100644 --- a/Makefile +++ b/Makefile @@ -72,8 +72,25 @@ USER_BIN := $(HOME)/.local/bin python3 -c "import sys; print(sys.version_info[:2])" > "$$tmp_file"; \ if [ ! -f $@ ] || ! cmp -s "$$tmp_file" $@; then mv "$$tmp_file" $@; else rm -f "$$tmp_file"; fi +# The native `solstone-core` binary is built by maturin from Rust sources that +# live OUTSIDE its package directory — packages/solstone-core/pyproject.toml +# points maturin at ../../core. So none of `.installed`'s other prerequisites +# move when native code changes: `.installed` stays satisfied, `uv sync` never +# runs, and .venv/bin/solstone-core keeps serving whatever was built at first +# install. Every Python test and check that shells out to that binary then +# exercises stale native bytes while reporting green. This stamp puts the Rust +# tree into the prerequisite chain; the `cache-keys` block in +# packages/solstone-core/pyproject.toml is what makes the `uv sync` this +# triggers actually rebuild. Hashed by content, not mtime, so a checkout or a +# touch that changes nothing does not force a reinstall. core/target/ is +# deliberately absent — the build writes it, so keying on it never settles. +.rust-core-hash: FORCE + @tmp_file=$$(mktemp); \ + python3 -c 'import hashlib, pathlib; root = pathlib.Path("core"); pats = ("Cargo.toml", "Cargo.lock", "crates/**/Cargo.toml", "crates/**/*.rs", "fixtures/**/*"); paths = sorted({p for pat in pats for p in root.glob(pat) if p.is_file()}); print(hashlib.sha256(b"".join(str(p).encode() + b"\0" + p.read_bytes() + b"\0" for p in paths)).hexdigest())' > "$$tmp_file"; \ + if [ ! -f $@ ] || ! cmp -s "$$tmp_file" $@; then mv "$$tmp_file" $@; else rm -f "$$tmp_file"; fi + # Marker file to track installation -.installed: pyproject.toml packages/*/pyproject.toml uv.lock .python-version-hash +.installed: pyproject.toml packages/*/pyproject.toml uv.lock .python-version-hash .rust-core-hash python3 scripts/render_packaging.py $(MAKE) preflight @echo "Installing package with uv..." diff --git a/packages/solstone-core/pyproject.toml b/packages/solstone-core/pyproject.toml index b301c1b74..0fbb9db98 100644 --- a/packages/solstone-core/pyproject.toml +++ b/packages/solstone-core/pyproject.toml @@ -14,3 +14,23 @@ bindings = "bin" manifest-path = "../../core/crates/solstone-core/Cargo.toml" profile = "release" strip = true + +# This package's build inputs live outside its directory (see manifest-path). +# uv's default cache key for a path dependency is this pyproject.toml alone, so +# a Rust-only change left `uv sync` reporting "no changes" and the environment +# kept serving the previously built binary — anything that then invoked +# .venv/bin/solstone-core was exercising stale native code. Declaring the real +# inputs makes uv rebuild when they move. `[tool.uv]` is never published, so +# wheel metadata is unaffected. Keep core/target/ out of the list: the build +# writes it, and keying on it would leave every sync permanently dirty. The +# `.rust-core-hash` stamp in the Makefile is the other half — it is what makes +# `make install` run a `uv sync` at all after a Rust-only change. +[tool.uv] +cache-keys = [ + { file = "pyproject.toml" }, + { file = "../../core/Cargo.toml" }, + { file = "../../core/Cargo.lock" }, + { file = "../../core/crates/**/Cargo.toml" }, + { file = "../../core/crates/**/*.rs" }, + { file = "../../core/fixtures/**" }, +] diff --git a/tests/test_journal_skill.py b/tests/test_journal_skill.py index 550e98fe2..0db312bab 100644 --- a/tests/test_journal_skill.py +++ b/tests/test_journal_skill.py @@ -98,44 +98,39 @@ def test_make_skills_idempotent(tmp_path): } env = {"HOME": str(tmp_path / "home")} - first_run = subprocess.run( - [ - str(native), - "__solstone_identity=sol", - "skills", - "install", - "--project", - str(temp_root), - "--agent", - "all", - ], - cwd=temp_root, - env=env, - check=True, - capture_output=True, - text=True, - ) + + def install() -> subprocess.CompletedProcess[str]: + # Deliberately not check=True: a CalledProcessError reports only the + # exit status in pytest's summary and swallows the binary's own + # explanation, which is the whole diagnosis. Assert instead, and put + # stderr in the failure message. + run = subprocess.run( + [ + str(native), + "__solstone_identity=sol", + "skills", + "install", + "--project", + str(temp_root), + "--agent", + "all", + ], + cwd=temp_root, + env=env, + capture_output=True, + text=True, + ) + assert run.returncode == 0, ( + f"sol skills install exited {run.returncode}: {run.stderr.strip()!r}" + ) + return run + + first_run = install() assert first_run.stderr == "" first = link_state(temp_root) - second_run = subprocess.run( - [ - str(native), - "__solstone_identity=sol", - "skills", - "install", - "--project", - str(temp_root), - "--agent", - "all", - ], - cwd=temp_root, - env=env, - check=True, - capture_output=True, - text=True, - ) + second_run = install() assert second_run.stderr == "" second = link_state(temp_root) -- 2.51.2