diff --git a/packages/app/vite.config.ts b/packages/app/vite.config.ts index 40278d7..7b41824 100644 --- a/packages/app/vite.config.ts +++ b/packages/app/vite.config.ts @@ -1,14 +1,23 @@ import { defineConfig } from "vite"; import react from "@vitejs/plugin-react"; import { execSync } from "node:child_process"; +import { createHash } from "node:crypto"; // Bake the git revision into the bundle so the running SPA knows which source it // was built from and can flag drift against the companion's /api/version (the // stale-dist failure mode behind #132). Best-effort: "unknown" if git isn't -// available at build time. +// available at build time. The same string tallyman_core.version.checkout_revision +// computes: a dirty tree also names its edits with a short hash of `git diff HEAD`. function gitRevision(): string { try { - return execSync("git describe --always --dirty --abbrev=7", { encoding: "utf8" }).trim() || "unknown"; + const rev = execSync("git describe --always --dirty --abbrev=7", { encoding: "utf8" }).trim(); + if (!rev) return "unknown"; + if (!rev.endsWith("-dirty")) return rev; + const diff = execSync("git diff --no-ext-diff --no-color --binary HEAD", { + encoding: "utf8", + maxBuffer: 1 << 30, + }).trim(); + return `${rev}.${createHash("sha1").update(diff).digest("hex").slice(0, 6)}`; } catch { return "unknown"; } diff --git a/scripts/rebuild_nfl_salaries_catalog.py b/scripts/rebuild_nfl_salaries_catalog.py index 1dc5828..7c68129 100644 --- a/scripts/rebuild_nfl_salaries_catalog.py +++ b/scripts/rebuild_nfl_salaries_catalog.py @@ -114,8 +114,18 @@ class StockMainStyling(DefaultMainStyling): {"field": "player", "type": "nominal", "title": "Player"}, {"field": "year_signed", "type": "quantitative", "title": "Signing year", "format": "d"}, {"field": "team", "type": "nominal", "title": "Team"}, - {"field": "epa_per_value_million", "type": "quantitative", "title": "EPA/$1M (true value)", "format": ".1f"}, - {"field": "total_epa_during_contract", "type": "quantitative", "title": "Total EPA", "format": ".1f"}, + { + "field": "epa_per_value_million", + "type": "quantitative", + "title": "EPA/$1M (true value)", + "format": ".1f", + }, + { + "field": "total_epa_during_contract", + "type": "quantitative", + "title": "Total EPA", + "format": ".1f", + }, {"field": "value", "type": "quantitative", "title": "Contract value ($)", "format": "$,.0f"}, {"field": "contract_years", "type": "quantitative", "title": "Contract years"}, ], @@ -196,7 +206,8 @@ class Step: " .filter(ibis._.rn == 0)\n" ")\n" "\n" - "total_epa = ibis.coalesce(most_recent_contract.passing_epa, 0) + ibis.coalesce(most_recent_contract.rushing_epa, 0)\n" + "total_epa = ibis.coalesce(most_recent_contract.passing_epa, 0)" + " + ibis.coalesce(most_recent_contract.rushing_epa, 0)\n" "expr = most_recent_contract.select(\n" " player_display_name=most_recent_contract.player_display_name,\n" " season=most_recent_contract.season,\n" diff --git a/src/tallyman_cli/main.py b/src/tallyman_cli/main.py index 3956c09..95b904d 100644 --- a/src/tallyman_cli/main.py +++ b/src/tallyman_cli/main.py @@ -84,12 +84,14 @@ def _notify_companion_reset(project: str) -> None: import httpx from tallyman_core.server_lock import companion_url, resolved_home + from tallyman_core.version import git_revision url = companion_url() if url is None: return # no server on this data dir, so no browsers or Buckaroo sessions to reload - # `home` names this data dir, so a companion serving another one refuses the notify (#183). - payload = {"kind": "project_reset", "project": project, "home": str(resolved_home())} + # `home` names this data dir, so a companion serving another one refuses the notify (#183); `revision` names this + # process's source, so a companion running other source refuses it too. + payload = {"kind": "project_reset", "project": project, "home": str(resolved_home()), "revision": git_revision()} try: resp = httpx.post(f"{url}/internal/notify", json=payload, timeout=2.0) if resp.status_code == 409: diff --git a/src/tallyman_companion/app.py b/src/tallyman_companion/app.py index f030e1e..aa0e1a9 100644 --- a/src/tallyman_companion/app.py +++ b/src/tallyman_companion/app.py @@ -43,7 +43,7 @@ from tallyman_core.paths import entries_dir, project_dir, validate_project_name from tallyman_core.server_lock import is_this_data_dir, resolved_home from tallyman_core.telemetry import read_spans, record_span -from tallyman_core.version import git_revision, version_info +from tallyman_core.version import REPO_ROOT, describe_source, git_revision, is_this_revision, version_info from tallyman_xorq import ( full_diff, list_entries, @@ -72,6 +72,9 @@ class NotifyPayload(BaseModel): # The sender's data dir (TALLYMAN_HOME). A notify from a client of another data dir is refused (#183); absent → not # checked. home: str | None = None + # The sender's git revision. A notify from a client running other source is refused, and so is one without it: a + # client that sends none was started from source older than the check. + revision: str | None = None def _refuse_another_data_dir(what: str, home: str | None) -> None: @@ -90,6 +93,25 @@ def _refuse_another_data_dir(what: str, home: str | None) -> None: ) +def _refuse_other_source(what: str, revision: str | None) -> None: + """409 when a client runs other source than this companion (``tallyman_core.version``). + + An MCP server or CLI on another revision has already written to the catalogs by the time it notifies, and one + started from source older than the check has no check of its own, so this refusal and its log line are how it is + noticed. It must not reload or publish anything here either. + """ + if is_this_revision(revision): + return + message = ( + f"{what} refused: the client runs {revision or 'no revision (source older than the revision check)'} but this " + f"companion runs {describe_source(git_revision(), str(REPO_ROOT))}. Restart whichever is stale: an MCP server " + "with /mcp, then tallyman, then Reconnect; the companion with restart-tallyman. A `tallyman` CLI command runs " + "the source of the checkout it is run from, so run it from the companion's." + ) + log.error(message) + raise HTTPException(409, message) + + def _broadcaster() -> tuple[asyncio.Queue, list]: return asyncio.Queue(), [] @@ -1856,6 +1878,7 @@ async def api_recalc(project: str, payload: dict | None = None): @app.post("/internal/notify") async def notify(payload: NotifyPayload): _refuse_another_data_dir("notify", payload.home) + _refuse_other_source("notify", payload.revision) # The payload may name its project (CLI `reset-to --project` can target # a non-active project); only fall back to the active one when it doesn't. project_name = payload.project or _require_project() @@ -1895,7 +1918,7 @@ async def notify(payload: NotifyPayload): extra = payload.extra or {} event = _recalc_sse_event(extra.get("remap", {}), extra.get("step")) else: - event = payload.model_dump(exclude={"home"}) + event = payload.model_dump(exclude={"home", "revision"}) await publish(event) return {"ok": True, "subscribers": len(subscribers), "project": project_name} diff --git a/src/tallyman_companion/buckaroo_lifecycle.py b/src/tallyman_companion/buckaroo_lifecycle.py index 402bec1..e146eb8 100644 --- a/src/tallyman_companion/buckaroo_lifecycle.py +++ b/src/tallyman_companion/buckaroo_lifecycle.py @@ -245,6 +245,7 @@ def start(self) -> None: r = self._client.get(f"{self.base_url}/health", timeout=1.0) if r.status_code == 200: health = r.json() + self._refuse_other_buckaroo(health.get("version")) started_at = health.get("started") log.info("buckaroo ready on %s (started=%s)", self.base_url, started_at) # If this Buckaroo started fresh (different start time from @@ -260,6 +261,26 @@ def start(self) -> None: self.stop() raise BuckarooUnavailable("buckaroo did not respond to /health in time") + def _refuse_other_buckaroo(self, version: str | None) -> None: + """Stop the server and raise ``BuckarooUnavailable`` when it runs another buckaroo than this companion. + + The companion runs buckaroo in-process too (diffs, klass validation), and the server is spawned from the same + venv, so they agree at startup. The server is spawned again whenever it dies, though, and a venv synced to + another buckaroo in between (a pull that bumped the pin) gives the new server a version the companion, and the + SPA bundle built with it, were not built against. + """ + import buckaroo # noqa: PLC0415 + + if version == buckaroo.__version__: + return + self.stop() + message = ( + f"the Buckaroo server runs buckaroo {version} but this companion runs buckaroo {buckaroo.__version__}: the " + "venv changed under the running companion. Restart the companion (restart-tallyman)." + ) + log.error(message) + raise BuckarooUnavailable(message) + def _drain_stdout(self) -> None: if self.proc is None or self.proc.stdout is None: return diff --git a/src/tallyman_core/server_lock.py b/src/tallyman_core/server_lock.py index c4882f5..9bb568b 100644 --- a/src/tallyman_core/server_lock.py +++ b/src/tallyman_core/server_lock.py @@ -9,9 +9,11 @@ serves. The kernel drops the lock when the process exits, however it exits (SIGKILL included), so there is no stale lock to clean up. ``flock`` rather than ``fcntl.lockf``: a POSIX record lock is dropped when the process closes *any* descriptor of the file, which ``read_owner`` does in the server's own process. The file also holds an owner record -(JSON: pid, host, port, bind host, start time, data dir, argv). It names the holder in a refusal, and it tells a -client of this data dir which port its companion serves on (``companion_url``). The record is believed only while the -lock is held; a dead server's record stays in the file and ``read_owner`` returns None for it. +(JSON: pid, host, port, bind host, start time, data dir, argv, and the git revision and checkout it runs from). It +names the holder in a refusal, it tells a client of this data dir which port its companion serves on +(``companion_url``), and it tells an MCP server whether the companion runs the same source (``tallyman_core.version``). +The record is believed only while the lock is held; a dead server's record stays in the file and ``read_owner`` +returns None for it. The port is decided once, by ``tallyman run --port``, and this record is how it reaches the clients of the data dir (the MCP server, ``reset-to``), which are separate processes started independently of the server. There is no @@ -36,6 +38,7 @@ from tallyman_core.net import client_host from tallyman_core.paths import tallyman_home +from tallyman_core.version import REPO_ROOT, git_revision LOCK_FILENAME = "server.lock" @@ -95,6 +98,8 @@ def claim_data_dir(*, port: int, bind_host: str, home: Path | str | None = None) "started_at": datetime.datetime.now(datetime.UTC).isoformat(timespec="seconds"), "data_dir": str(data_dir), "argv": sys.argv, + "revision": git_revision(), + "source": str(REPO_ROOT), } os.ftruncate(fd, 0) os.pwrite(fd, json.dumps(record).encode(), 0) diff --git a/src/tallyman_core/version.py b/src/tallyman_core/version.py index 5fa660f..10229ad 100644 --- a/src/tallyman_core/version.py +++ b/src/tallyman_core/version.py @@ -1,4 +1,4 @@ -"""Git-revision stamping so each running component reports the source it's on. +"""Git-revision stamping, and the check that every process on a data dir runs the same source. tallyman has no releases yet, so the version *is* the git revision. Every long-lived process (the companion, the MCP server) and the built JS bundle @@ -7,44 +7,84 @@ (the class of bug behind #132). The companion exposes its revision at ``GET /api/version`` and on an ``X-Tallyman-Revision`` response header; the SPA bakes its build-time revision and warns when the two disagree. + +The MCP server (one per Claude Code session), the companion and the CLI are started independently, so one of them can +run code from before a pull, a checkout or an edit while another runs the new code, against the same catalogs. They +refuse to work together across revisions: the companion writes its revision into the owner record of the data dir it +claims (``server_lock``), an MCP tool refuses to run against a companion on another one, and the companion refuses a +notify from a client on another one. """ from __future__ import annotations import functools +import hashlib from pathlib import Path from tallyman_core.git_util import run_git # version.py lives at /src/tallyman_core/version.py. -_REPO_ROOT = Path(__file__).resolve().parents[2] +REPO_ROOT = Path(__file__).resolve().parents[2] + + +def checkout_revision(root: Path | str) -> str: + """The revision of the checkout at *root*, as it is on disk now. + + ``git describe --always --dirty`` yields the abbreviated commit, suffixed ``-dirty`` when there are *tracked* + working-tree changes (untracked files are ignored, so a stray scratch file doesn't flag drift). The suffix is the + same for every set of changes, so a dirty checkout also names its changes with a short hash of ``git diff HEAD``: + ``-dirty.``. Two processes started from one commit with different edits then report different + revisions. ``packages/app/vite.config.ts`` computes the same string for the SPA bundle. + + Routed through ``git_util.run_git`` (fork-safe ``posix_spawn``, the sanctioned primitive — no bare ``subprocess`` + git in src/). Returns ``"unknown"`` when git or the repo isn't available — versioning must never break a start. + """ + try: + rc, out, _ = run_git(["describe", "--always", "--dirty", "--abbrev=7"], cwd=root, timeout=2.0) + if rc != 0 or not out: + return "unknown" + if not out.endswith("-dirty"): + return out + rc, diff, _ = run_git(["diff", "--no-ext-diff", "--no-color", "--binary", "HEAD"], cwd=root, timeout=5.0) + except OSError: + return "unknown" + if rc != 0: + return "unknown" + return f"{out}.{hashlib.sha1(diff.encode()).hexdigest()[:6]}" @functools.lru_cache(maxsize=1) def git_revision() -> str: - """Short git revision of the checkout this process was launched from. + """Revision of the checkout this process was launched from (``checkout_revision``). Pinned on first call (``lru_cache``) so it reflects the code the process is actually running, not wherever ``HEAD`` moves later in the process's life. - ``git describe --always --dirty`` yields the abbreviated commit, suffixed - ``-dirty`` when there are *tracked* working-tree changes (untracked files are - ignored, so a stray scratch file doesn't flag drift). Routed through - ``git_util.run_git`` (fork-safe ``posix_spawn``, the sanctioned primitive — - no bare ``subprocess`` git in src/). Returns ``"unknown"`` when git or the - repo isn't available — versioning must never break a start. """ - try: - rc, out, _ = run_git( - ["describe", "--always", "--dirty", "--abbrev=7"], - cwd=_REPO_ROOT, - timeout=2.0, - ) - except OSError: - return "unknown" - return out if (rc == 0 and out) else "unknown" + return checkout_revision(REPO_ROOT) def version_info() -> dict: """Structured version payload for ``/api/version``, headers, and logs.""" rev = git_revision() - return {"revision": rev, "dirty": rev.endswith("-dirty")} + return {"revision": rev, "dirty": "-dirty" in rev, "source": str(REPO_ROOT)} + + +def is_this_revision(revision: str | None) -> bool: + """Whether another process reporting *revision* runs the source this one does. An unknown revision never does.""" + return revision == git_revision() and revision != "unknown" + + +def describe_source(revision: str | None, source: str | None) -> str: + """`` from `` for a refusal message, noting when that checkout has since moved on. + + The note says which side is stale: a process whose checkout is no longer at the revision it started from is + running old code. + """ + if not revision: + return "no revision (it was started from source older than the revision check)" + text = f"{revision} from {source or 'an unknown checkout'}" + if source and Path(source).is_dir(): + now = checkout_revision(source) + if now not in (revision, "unknown"): + text += f" (stale: that checkout is now at {now})" + return text diff --git a/src/tallyman_mcp/server.py b/src/tallyman_mcp/server.py index 5fa14ac..c70488a 100644 --- a/src/tallyman_mcp/server.py +++ b/src/tallyman_mcp/server.py @@ -15,6 +15,7 @@ import httpx from fastmcp import FastMCP +from fastmcp.exceptions import ToolError from tallyman_core import ( AliasExists, @@ -59,7 +60,8 @@ from tallyman_core.errors import RECORD_HOOKS from tallyman_core.events import record_event from tallyman_core.paths import errors_path -from tallyman_core.server_lock import companion_url, resolved_home +from tallyman_core.server_lock import companion_url, read_owner, resolved_home +from tallyman_core.version import REPO_ROOT, describe_source, git_revision, is_this_revision from tallyman_xorq import BuildError, build_and_persist, full_diff, list_entries, staleness from tallyman_xorq.dependents import references_own_alias from tallyman_xorq.recalc import classify_orphans, recalc @@ -347,16 +349,51 @@ def wrapped(*args, **kwargs): return wrapped +def _refuse_other_source(tool: str) -> None: + """Raise ``ToolError`` when the companion of this data dir runs other source than this MCP server. + + Both are long-lived and started independently, so after a pull, a checkout or an edit one of them runs the old + code against the catalogs the other writes with the new code. Checked on every call, not once at startup: the + companion is restarted under an MCP server that lives as long as its Claude Code session. With no server on the + data dir (or one between claiming it and writing its record) there is nothing to compare. + """ + try: + owner = read_owner() + except OSError: + return + if not owner or is_this_revision(owner.get("revision")): + return + raise ToolError( + f"tallyman refused {tool}: this MCP server and the companion on its data dir run different source.\n" + f" MCP server (pid {os.getpid()}): {describe_source(git_revision(), str(REPO_ROOT))}\n" + f" companion (pid {owner.get('pid')}, port {owner.get('port')}): " + f"{describe_source(owner.get('revision'), owner.get('source'))}\n" + "Restart whichever is stale and try again: the MCP server with /mcp, then tallyman, then Reconnect; the " + "companion with restart-tallyman (or stop `tallyman run` and start it again)." + ) + + +def _with_source_check(fn): + """Refuse the call before *fn* runs when the companion runs other source (``_refuse_other_source``).""" + + @functools.wraps(fn) + def wrapped(*args, **kwargs): + _refuse_other_source(fn.__name__) + return fn(*args, **kwargs) + + return wrapped + + _original_tool = mcp.tool def _checkpointing_tool(*dargs, **dkwargs): - """``mcp.tool`` wrapped so registration applies the checkpoint hook.""" + """``mcp.tool`` wrapped so registration applies the source check and the checkpoint hook.""" register = _original_tool(*dargs, **dkwargs) def _register(fn): _CHECKPOINTED_TOOLS.add(fn.__name__) - return register(_with_checkpoint(fn)) + return register(_with_source_check(_with_checkpoint(fn))) return _register @@ -368,7 +405,8 @@ def _notify(kind: str, content_hash: str | None = None, **extra) -> None: """Best-effort POST to the companion's /internal/notify. Never raise. ``home`` names this data dir, so a companion serving another one refuses the notify (409) instead of publishing - this project's events to its own browsers (#183). + this project's events to its own browsers (#183). ``revision`` names this process's source, so a companion running + other source refuses it too. """ base = companion_url() if base is None: @@ -378,7 +416,13 @@ def _notify(kind: str, content_hash: str | None = None, **extra) -> None: with httpx.Client(timeout=2.0) as client: resp = client.post( url, - json={"kind": kind, "hash": content_hash, "extra": extra or None, "home": str(resolved_home())}, + json={ + "kind": kind, + "hash": content_hash, + "extra": extra or None, + "home": str(resolved_home()), + "revision": git_revision(), + }, ) if resp.status_code >= 400: print(f"[tallyman_mcp] notify {kind} refused by {url}: {resp.status_code} {resp.text}", file=sys.stderr) @@ -2139,9 +2183,7 @@ def main() -> None: level=os.environ.get("TALLYMAN_LOG_LEVEL", "INFO"), format="[%(name)s] %(message)s", ) - from tallyman_core.version import git_revision # noqa: PLC0415 - - log.info("tallyman_mcp revision %s", git_revision()) + log.info("tallyman_mcp revision %s from %s", git_revision(), REPO_ROOT) mcp.run() diff --git a/tests/test_buckaroo.py b/tests/test_buckaroo.py index 8d53f95..496da39 100644 --- a/tests/test_buckaroo.py +++ b/tests/test_buckaroo.py @@ -665,6 +665,25 @@ def test_integration_spawn_and_load(project: str, orders_src: str, isolated_home assert not mgr.is_running +@pytest.mark.integration +def test_integration_start_refuses_a_buckaroo_other_than_the_companions(monkeypatch): + """The companion runs buckaroo in-process (diffs, klass validation) and spawns the Buckaroo server from its venv, + again whenever the server dies. A venv synced to another buckaroo after the companion started gives the respawned + server another version than the companion's own: refused, and the server stopped, rather than served.""" + import buckaroo + + from tallyman_companion.buckaroo_lifecycle import BuckarooUnavailable + + monkeypatch.setattr(buckaroo, "__version__", "0.0.0") # the companion's buckaroo; the subprocess reports its own + mgr = BuckarooManager(port=0, startup_timeout=15.0) + try: + with pytest.raises(BuckarooUnavailable, match=r"0\.0\.0"): + mgr.start() + assert not mgr.is_running + finally: + mgr.stop() + + @pytest.mark.integration def test_integration_load_expr_returns_nonzero_rows(project: str, orders_src: str): """Smoking-gun integration test for the placeholder-expansion fix. diff --git a/tests/test_companion.py b/tests/test_companion.py index 311e595..bc6cadf 100644 --- a/tests/test_companion.py +++ b/tests/test_companion.py @@ -4,6 +4,7 @@ from fastapi.testclient import TestClient from tallyman_core import entry_dir +from tallyman_core.version import git_revision from tallyman_xorq import build_and_persist @@ -269,7 +270,7 @@ def test_read_endpoints_reject_malformed_content_hash(fresh_companion_app, proje def test_internal_notify_returns_subscriber_count(fresh_companion_app): c = TestClient(fresh_companion_app) - r = c.post("/internal/notify", json={"kind": "new_entry", "hash": "abc"}) + r = c.post("/internal/notify", json={"revision": git_revision(), "kind": "new_entry", "hash": "abc"}) assert r.status_code == 200 body = r.json() assert body["ok"] is True diff --git a/tests/test_mcp_stdio.py b/tests/test_mcp_stdio.py index f091fb1..3714f44 100644 --- a/tests/test_mcp_stdio.py +++ b/tests/test_mcp_stdio.py @@ -121,3 +121,24 @@ async def go(): assert diff["before"]["version"] == 1 assert diff["after"]["version"] == 2 assert diff["keyed_summary"] is not None + + +@pytest.mark.integration +@needs_uv +def test_stdio_tool_call_against_a_companion_on_another_revision_is_a_tool_error( + isolated_home: Path, project: str, running_server: dict +): + """What Claude Code sees: the refusal arrives as a tool error carrying the message, not a masked failure.""" + import json + + from fastmcp.exceptions import ToolError + + lock = isolated_home / "server.lock" + lock.write_text(json.dumps({**json.loads(lock.read_text()), "revision": "0000000"})) + + async def go(): + async with _client(project, isolated_home) as client: + await client.call_tool("project_list", {}) + + with pytest.raises(ToolError, match="0000000"): + asyncio.run(go()) diff --git a/tests/test_project_switcher.py b/tests/test_project_switcher.py index 1fc3b8b..5cc5bd7 100644 --- a/tests/test_project_switcher.py +++ b/tests/test_project_switcher.py @@ -22,6 +22,7 @@ from tallyman_companion import create_app from tallyman_core import ensure_project, resolve_project, set_active_project from tallyman_core.paths import active_project_file_path, project_dir +from tallyman_core.version import git_revision # --------------------------------------------------------------------------- # GET /api/projects @@ -286,7 +287,7 @@ def test_internal_notify_tags_with_active_project(isolated_home: Path): set_active_project("alpha") app = create_app() c = TestClient(app) - r = c.post("/internal/notify", json={"kind": "test_event"}) + r = c.post("/internal/notify", json={"revision": git_revision(), "kind": "test_event"}) assert r.status_code == 200 body = r.json() # Response echoes the project it tagged with so callers can verify. diff --git a/tests/test_reset_to_revision.py b/tests/test_reset_to_revision.py index 0bd1134..beea973 100644 --- a/tests/test_reset_to_revision.py +++ b/tests/test_reset_to_revision.py @@ -23,6 +23,7 @@ from tallyman_core import charts, git_util, notebook, paths from tallyman_core import post_processing as pp from tallyman_core import summary_stats as ss +from tallyman_core.version import git_revision from tallyman_xorq import build_and_persist PP_SRC = "def process(expr):\n return expr\n" @@ -463,7 +464,12 @@ def test_cli_reset_notify_names_the_reset_project(isolated_home, running_server, assert runner.invoke(cli, ["init", "beta", "--no-fixture"]).exit_code == 0 res = runner.invoke(cli, ["reset-to", "0", "--project", "beta"]) assert res.exit_code == 0, res.output - assert sent["json"] == {"kind": "project_reset", "project": "beta", "home": str(isolated_home.resolve())} + assert sent["json"] == { + "kind": "project_reset", + "project": "beta", + "home": str(isolated_home.resolve()), + "revision": git_revision(), + } def test_notify_honors_explicit_project(fresh_companion_app, project): @@ -472,11 +478,11 @@ def test_notify_honors_explicit_project(fresh_companion_app, project): from fastapi.testclient import TestClient c = TestClient(fresh_companion_app) - r = c.post("/internal/notify", json={"kind": "project_reset", "project": "beta"}) + r = c.post("/internal/notify", json={"revision": git_revision(), "kind": "project_reset", "project": "beta"}) assert r.status_code == 200, r.text assert r.json()["project"] == "beta" - r2 = c.post("/internal/notify", json={"kind": "project_reset"}) # no project: active + r2 = c.post("/internal/notify", json={"revision": git_revision(), "kind": "project_reset"}) # no project: active assert r2.status_code == 200 assert r2.json()["project"] == project diff --git a/tests/test_result_cache.py b/tests/test_result_cache.py index 15b93ba..f5476cb 100644 --- a/tests/test_result_cache.py +++ b/tests/test_result_cache.py @@ -4,6 +4,7 @@ from fastapi.testclient import TestClient from tallyman_core.paths import catalog_dir, entry_dir +from tallyman_core.version import git_revision from tallyman_mcp.server import catalog_create, catalog_revise from tallyman_xorq.build import list_entries from tallyman_xorq.result_cache import cache_worthy @@ -1014,7 +1015,7 @@ def test_notify_project_reset_evicts_the_compare_build(fresh_companion_app, proj # The notify signal clears it, so the next /api/diff_data builds afresh. c = TestClient(fresh_companion_app) - r = c.post("/internal/notify", json={"kind": "project_reset", "project": project}) + r = c.post("/internal/notify", json={"revision": git_revision(), "kind": "project_reset", "project": project}) assert r.status_code == 200, r.text assert _build_compare_expr.cache_info().currsize == 0 @@ -1038,7 +1039,7 @@ def test_notify_project_reset_clears_result_plan_memo(fresh_companion_app, proje assert cached_result_expr.cache_info().currsize >= 1 c = TestClient(fresh_companion_app) - r = c.post("/internal/notify", json={"kind": "project_reset", "project": project}) + r = c.post("/internal/notify", json={"revision": git_revision(), "kind": "project_reset", "project": project}) assert r.status_code == 200, r.text assert cached_result_expr.cache_info().currsize == 0 diff --git a/tests/test_serve.py b/tests/test_serve.py index 3277334..3faf4da 100644 --- a/tests/test_serve.py +++ b/tests/test_serve.py @@ -7,6 +7,7 @@ from tallyman_companion import create_app from tallyman_core import entry_dir, project_dir, set_alias +from tallyman_core.version import git_revision from tallyman_xorq import build_and_persist @@ -22,7 +23,7 @@ def test_read_only_mode_rejects_notify(fresh_companion_app, project: str): # The plain fixture is edit-mode; build a new app explicitly in read-only. app = create_app(project, read_only=True) c = TestClient(app) - r = c.post("/internal/notify", json={"kind": "new_entry", "hash": "abc"}) + r = c.post("/internal/notify", json={"revision": git_revision(), "kind": "new_entry", "hash": "abc"}) assert r.status_code == 403 @@ -73,7 +74,7 @@ def test_notify_post_processing_changed_reloads_sessions(project: str): stub = _TrackingBuckaroo() app = create_app(project, buckaroo=stub) c = TestClient(app) - r = c.post("/internal/notify", json={"kind": "post_processing_changed"}) + r = c.post("/internal/notify", json={"revision": git_revision(), "kind": "post_processing_changed"}) assert r.status_code == 200 assert stub.reload_calls == [project] @@ -82,7 +83,7 @@ def test_notify_summary_stat_changed_reloads_sessions(project: str): stub = _TrackingBuckaroo() app = create_app(project, buckaroo=stub) c = TestClient(app) - r = c.post("/internal/notify", json={"kind": "summary_stat_changed"}) + r = c.post("/internal/notify", json={"revision": git_revision(), "kind": "summary_stat_changed"}) assert r.status_code == 200 assert stub.reload_calls == [project] @@ -91,7 +92,7 @@ def test_notify_other_kind_does_not_reload_sessions(project: str): stub = _TrackingBuckaroo() app = create_app(project, buckaroo=stub) c = TestClient(app) - r = c.post("/internal/notify", json={"kind": "new_entry", "hash": "abc"}) + r = c.post("/internal/notify", json={"revision": git_revision(), "kind": "new_entry", "hash": "abc"}) assert r.status_code == 200 assert stub.reload_calls == [] @@ -109,7 +110,10 @@ def test_notify_recalc_reloads_sessions_and_invalidates_caches(project: str, mon stub = _TrackingBuckaroo() app = create_app(project, buckaroo=stub) c = TestClient(app) - r = c.post("/internal/notify", json={"kind": "recalc", "extra": {"remap": {"old": "new"}, "step": 3}}) + r = c.post( + "/internal/notify", + json={"revision": git_revision(), "kind": "recalc", "extra": {"remap": {"old": "new"}, "step": 3}}, + ) assert r.status_code == 200 assert stub.reload_calls == [project] # buckaroo reloaded, like project_reset assert invalidated == [True] # LRUs cleared, like project_reset @@ -140,7 +144,10 @@ def test_notify_recalc_publishes_normalized_event(project: str, monkeypatch): ) app = create_app(project) c = TestClient(app) - r = c.post("/internal/notify", json={"kind": "recalc", "extra": {"remap": {"o": "n"}, "step": 5}}) + r = c.post( + "/internal/notify", + json={"revision": git_revision(), "kind": "recalc", "extra": {"remap": {"o": "n"}, "step": 5}}, + ) assert r.status_code == 200 assert captured == [({"o": "n"}, 5)] # remap + step pulled from extra, routed through the normalizer diff --git a/tests/test_server_lock.py b/tests/test_server_lock.py index 22e0917..a35b472 100644 --- a/tests/test_server_lock.py +++ b/tests/test_server_lock.py @@ -24,6 +24,8 @@ import pytest +from tallyman_core.version import git_revision + # A process that claims the data dir named by its TALLYMAN_HOME, says so on stdout and holds the claim until its stdin # closes or it is killed. With "spawn" it first starts a child that outlives it, the way a Buckaroo subprocess could. _HOLDER = """ @@ -534,7 +536,13 @@ def post(self, url, json=None): assert posted == [ ( "http://127.0.0.1:17874/internal/notify", - {"kind": "new_entry", "hash": "abc", "extra": None, "home": str(isolated_home.resolve())}, + { + "kind": "new_entry", + "hash": "abc", + "extra": None, + "home": str(isolated_home.resolve()), + "revision": git_revision(), # so a companion on other source refuses it + }, ) ] assert srv._entry_url(project, "abc") == f"http://127.0.0.1:17874/{project}/catalog/abc" @@ -557,6 +565,7 @@ def test_cli_reset_notifies_the_companion_of_its_own_data_dir(isolated_home, mon assert result.exit_code == 0, result.output assert sent["url"] == "http://127.0.0.1:17875/internal/notify" assert sent["json"]["home"] == str(isolated_home.resolve()) + assert sent["json"]["revision"] == git_revision() # --------------------------------------------------------------------------- @@ -572,14 +581,23 @@ def test_notify_from_another_data_dir_is_refused(project, isolated_home, tmp_pat other = tmp_path / "another-data-dir" c = TestClient(create_app(project)) - refused = c.post("/internal/notify", json={"kind": "new_entry", "hash": "abc", "home": str(other)}) + refused = c.post( + "/internal/notify", json={"revision": git_revision(), "kind": "new_entry", "hash": "abc", "home": str(other)} + ) assert refused.status_code == 409, refused.text detail = refused.json()["detail"] assert str(other.resolve()) in detail assert str(isolated_home.resolve()) in detail - assert c.post("/internal/notify", json={"kind": "new_entry", "home": str(isolated_home)}).status_code == 200 - assert c.post("/internal/notify", json={"kind": "new_entry"}).status_code == 200 # no home: still accepted + assert ( + c.post( + "/internal/notify", json={"revision": git_revision(), "kind": "new_entry", "home": str(isolated_home)} + ).status_code + == 200 + ) + assert ( + c.post("/internal/notify", json={"revision": git_revision(), "kind": "new_entry"}).status_code == 200 + ) # no home: still accepted def _next_event(lines) -> str: @@ -617,10 +635,12 @@ def test_a_refused_notify_publishes_no_sse_event(project, isolated_home, tmp_pat lines = stream.iter_lines() assert _next_event(lines) == "hello" # subscribed before anything is posted refused = client.post( - f"{base}/internal/notify", json={"kind": "from_another_data_dir", "home": str(tmp_path / "other")} + f"{base}/internal/notify", + json={"revision": git_revision(), "kind": "from_another_data_dir", "home": str(tmp_path / "other")}, ) accepted = client.post( - f"{base}/internal/notify", json={"kind": "from_this_data_dir", "home": str(isolated_home)} + f"{base}/internal/notify", + json={"revision": git_revision(), "kind": "from_this_data_dir", "home": str(isolated_home)}, ) first = _next_event(lines) @@ -752,7 +772,9 @@ def test_notify_from_this_data_dir_spelled_in_another_case_is_accepted(project, pytest.skip("case-sensitive filesystem: another spelling is another directory") c = TestClient(create_app(project)) - accepted = c.post("/internal/notify", json={"kind": "new_entry", "home": str(respelled)}) + accepted = c.post( + "/internal/notify", json={"revision": git_revision(), "kind": "new_entry", "home": str(respelled)} + ) assert accepted.status_code == 200, accepted.text switched = c.post("/api/projects/switch", json={"name": project, "home": str(respelled)}) assert switched.status_code == 200, switched.text diff --git a/tests/test_version.py b/tests/test_version.py index e181969..c7c2d64 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -1,9 +1,15 @@ from __future__ import annotations +import json +import subprocess +from pathlib import Path + +import pytest from fastapi.testclient import TestClient from tallyman_companion import create_app from tallyman_core.version import git_revision, version_info +from tallyman_xorq import list_entries # tallyman has no releases yet, so the version is the git revision. Each # long-lived component stamps the source it runs from so drift is visible @@ -41,3 +47,123 @@ def test_revision_header_rides_every_response(project, orders_parquet, monkeypat # SPA makes carries the backend's revision for the drift check. r = c.get("/api/projects") assert r.headers.get("x-tallyman-revision") == git_revision() + + +# --------------------------------------------------------------------------- +# every process on a data dir runs the same source +# --------------------------------------------------------------------------- +# The MCP server (one per Claude Code session), the companion (`tallyman run`, one per data dir) and the CLI are +# started independently, so one of them can be running code from before a pull, a checkout or an edit while the others +# run the new code, all against the same catalogs. The companion writes its revision into the owner record of the data +# dir it claims (server_lock), and a client of that data dir refuses to work against a companion on another revision. + + +def _git(*args: str, cwd: Path) -> str: + return subprocess.run(["git", *args], cwd=cwd, check=True, capture_output=True, text=True).stdout.strip() + + +def test_uncommitted_edits_change_the_revision(tmp_path): + """``git describe --dirty`` names every uncommitted state the same (``-dirty``), so an MCP server started + before an edit and a companion started after it would agree while running different code. Two different sets of + edits on one commit have to be two revisions.""" + from tallyman_core.version import checkout_revision + + repo = tmp_path / "checkout" + repo.mkdir() + _git("init", "-q", cwd=repo) + _git("config", "user.email", "t@example.com", cwd=repo) + _git("config", "user.name", "tester", cwd=repo) + (repo / "mod.py").write_text("x = 1\n") + _git("add", "mod.py", cwd=repo) + _git("commit", "-q", "-m", "one", cwd=repo) + sha = _git("rev-parse", "--short=7", "HEAD", cwd=repo) + + assert checkout_revision(repo) == sha + + (repo / "mod.py").write_text("x = 2\n") + first_edit = checkout_revision(repo) + (repo / "mod.py").write_text("x = 3\n") + second_edit = checkout_revision(repo) + + assert first_edit.startswith(f"{sha}-dirty.") + assert second_edit.startswith(f"{sha}-dirty.") + assert first_edit != second_edit + assert checkout_revision(repo) == second_edit # the same edits are the same revision + + (repo / "mod.py").write_text("x = 1\n") + assert checkout_revision(repo) == sha + + +def test_the_server_record_names_the_revision_it_runs(running_server): + from tallyman_core.server_lock import read_owner + + assert running_server["revision"] == git_revision() + assert read_owner()["revision"] == git_revision() + + +def _set_companion_revision(home: Path, revision: str | None) -> None: + """Rewrite the owner record of the server holding *home* as a companion on *revision* would have written it + (``None``: a companion from before records carried one). The claim itself is untouched: ``flock`` locks the open + file, not its contents.""" + lock = home / "server.lock" + record = json.loads(lock.read_text()) + if revision is None: + record.pop("revision", None) + else: + record["revision"] = revision + lock.write_text(json.dumps(record)) + + +def test_mcp_tools_refuse_to_run_against_a_companion_on_another_revision( + project, orders_src, isolated_home, running_server +): + from fastmcp.exceptions import ToolError + + import tallyman_mcp.server as srv + + entries_before = list_entries(project) + _set_companion_revision(isolated_home, "0000000") + + with pytest.raises(ToolError) as refused: + srv.catalog_list() + message = str(refused.value) + assert "0000000" in message and git_revision() in message # both sides named + assert "/mcp" in message and "restart-tallyman" in message # and how to restart each + + # A refused build builds nothing. + recipe = ( + "from tallyman_xorq.io import tracked_expr_from_alias\n" + f"t = tracked_expr_from_alias({orders_src!r})\n" + 'expr = t.group_by("region").aggregate(n=t.count())\n' + ) + with pytest.raises(ToolError): + srv.catalog_run(recipe, prompt="must not build") + assert list_entries(project) == entries_before + + +def test_mcp_tools_refuse_a_companion_that_records_no_revision(project, isolated_home, running_server): + """A companion started from source older than this check writes no revision. It is not known to match.""" + from fastmcp.exceptions import ToolError + + import tallyman_mcp.server as srv + + _set_companion_revision(isolated_home, None) + + with pytest.raises(ToolError, match="no revision"): + srv.catalog_list() + + +def test_companion_refuses_a_notify_from_another_revision(project, isolated_home): + """The check the other way round: an MCP server or CLI older than this check has no check of its own, and its + notify is the only thing the companion ever sees from it.""" + c = TestClient(create_app(project)) + + other = c.post("/internal/notify", json={"kind": "new_entry", "revision": "0000000"}) + assert other.status_code == 409, other.text + assert "0000000" in other.json()["detail"] and git_revision() in other.json()["detail"] + + none = c.post("/internal/notify", json={"kind": "new_entry"}) + assert none.status_code == 409, none.text + + same = c.post("/internal/notify", json={"kind": "new_entry", "revision": git_revision()}) + assert same.status_code == 200, same.text