From 97b98052312daf4c075a36c3cab91cff26bd3140 Mon Sep 17 00:00:00 2001 From: Paddy Mullen Date: Tue, 29 Sep 2026 12:19:40 -0400 Subject: [PATCH 1/3] test: the MCP server, companion and Buckaroo refuse to work across different source The MCP server (one per Claude Code session), the companion and the CLI start independently, so one can run code from before a pull, checkout or edit while another runs the new code against the same catalogs. These fail until: - uncommitted edits are part of the revision (two edit states on one commit differ) - the companion's owner record names its revision - an MCP tool refuses to run against a companion on another revision, or one that records none - the companion refuses a notify from another revision, or one that sends none - the MCP and CLI notifies carry their revision - BuckarooManager.start refuses a Buckaroo server whose version is not the companion's buckaroo - over stdio, the refusal is a tool error carrying the message Co-Authored-By: Claude Opus 5.5 --- tests/test_buckaroo.py | 19 ++++++ tests/test_mcp_stdio.py | 21 +++++++ tests/test_server_lock.py | 11 +++- tests/test_version.py | 126 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 1 deletion(-) diff --git a/tests/test_buckaroo.py b/tests/test_buckaroo.py index 8d53f952..496da39c 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_mcp_stdio.py b/tests/test_mcp_stdio.py index f091fb13..3714f441 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_server_lock.py b/tests/test_server_lock.py index 22e09179..0e36464c 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() # --------------------------------------------------------------------------- diff --git a/tests/test_version.py b/tests/test_version.py index e1819699..c7c2d645 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 From 8bf1d849ae7c196576333bb11778cadbffd6edb8 Mon Sep 17 00:00:00 2001 From: Paddy Mullen Date: Tue, 29 Sep 2026 14:17:08 -0400 Subject: [PATCH 2/3] feat: the MCP server, companion and Buckaroo refuse to work across different source - The revision names uncommitted edits: a dirty checkout is `-dirty.`, so two processes on one commit with different edits differ. vite.config.ts bakes the same string into the SPA. - The companion writes its revision and checkout into the owner record of the data dir it claims. - Every MCP tool checks that record before it runs and raises a ToolError naming both sides, which checkout has moved on since its process started, and how to restart each. With no companion on the data dir there is nothing to check. - The MCP and CLI notifies carry their revision. The companion refuses (409, logged as an error) a notify from another revision or with none, which is how an MCP server or CLI from before this check is noticed. - BuckarooManager.start stops the Buckaroo server and raises BuckarooUnavailable when /health reports another buckaroo version than the one the companion runs in-process: a venv synced under a running companion, seen on a respawn. - Tests that post /internal/notify send the revision. Co-Authored-By: Claude Opus 5.5 --- packages/app/vite.config.ts | 13 +++- src/tallyman_cli/main.py | 6 +- src/tallyman_companion/app.py | 27 ++++++- src/tallyman_companion/buckaroo_lifecycle.py | 21 ++++++ src/tallyman_core/server_lock.py | 11 ++- src/tallyman_core/version.py | 78 +++++++++++++++----- src/tallyman_mcp/server.py | 58 +++++++++++++-- tests/test_companion.py | 3 +- tests/test_project_switcher.py | 3 +- tests/test_reset_to_revision.py | 12 ++- tests/test_result_cache.py | 5 +- tests/test_serve.py | 19 +++-- tests/test_server_lock.py | 25 +++++-- 13 files changed, 226 insertions(+), 55 deletions(-) diff --git a/packages/app/vite.config.ts b/packages/app/vite.config.ts index 40278d73..7b418249 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/src/tallyman_cli/main.py b/src/tallyman_cli/main.py index 3956c090..95b904da 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 f030e1ed..aa0e1a91 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 402bec15..e146eb86 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 c4882f50..9bb568b4 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 5fa660fc..10229adb 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 5fa14ac3..c70488a7 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_companion.py b/tests/test_companion.py index 311e595f..bc6cadfa 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_project_switcher.py b/tests/test_project_switcher.py index 1fc3b8be..5cc5bd74 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 0bd11347..beea9736 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 66c6c9ba..d91e8869 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 32773344..3faf4dad 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 0e36464c..a35b4726 100644 --- a/tests/test_server_lock.py +++ b/tests/test_server_lock.py @@ -581,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: @@ -626,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) @@ -761,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 From 204dcaebcf89de958f5b47f74b32fce5607f8ed7 Mon Sep 17 00:00:00 2001 From: Paddy Mullen Date: Fri, 2 Oct 2026 10:13:50 -0400 Subject: [PATCH 3/3] fix(scripts): wrap the three lines over 120 columns in rebuild_nfl_salaries_catalog.py f074726 left three E501 lines, so `ruff check` has failed on main since, and CI skips the test jobs behind it. Two tooltip dicts go one key per line; the recipe line splits into two adjacent string literals, which the parser joins back into the same string. The module's AST is unchanged. Co-Authored-By: Claude Opus 5.5 --- scripts/rebuild_nfl_salaries_catalog.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/scripts/rebuild_nfl_salaries_catalog.py b/scripts/rebuild_nfl_salaries_catalog.py index 1dc58281..7c68129f 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"