Skip to content

feat: the MCP server, companion and Buckaroo refuse to work across different source - #294

Open
paddymul wants to merge 5 commits into
mainfrom
feat/revision-check
Open

paddymul wants to merge 5 commits into
mainfrom
feat/revision-check

Conversation

@paddymul

@paddymul paddymul commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Problem

The MCP server (tallyman mcp, one per Claude Code session), the companion (tallyman run, one per data dir) and the CLI start independently. One can run code from before a pull, a checkout or an uncommitted edit while another runs the new code, all against the same catalogs. #132 made each one report its git describe --dirty revision, but nothing compares them, and -dirty names every set of uncommitted edits the same.

On this machine an MCP server started from the old /Users/paddy/tallyman clone at 22f6a3e (before #189) was running against the same ~/.tallyman-notebooks as a companion on 4996808. Nothing flagged it.

Fix

  • Revision. version.checkout_revision(root) is git describe --always --dirty --abbrev=7. For a dirty tree it appends the first 6 hex digits of the sha1 of git diff --no-ext-diff --no-color --binary HEAD, giving <sha>-dirty.<hash>. Two processes on one commit with different edits now differ. git_revision() still pins it once per process. packages/app/vite.config.ts computes the same string, so the SPA's existing drift warning still matches. After a build, the bundle had 97b9805-dirty.f3c30d, the same string Python reported.
  • Owner record. claim_data_dir writes revision and source (the checkout path) into server.lock.
  • MCP. Every tool registration is wrapped by _with_source_check, outside the checkpoint wrapper. Before the tool runs it reads the owner record. If a companion holds the data dir and is_this_revision(owner["revision"]) is false (a different revision, a missing one, or unknown), it raises fastmcp.exceptions.ToolError.
    • The message gives each process's pid, revision and checkout, marks a side stale when its checkout has moved since it started, and says how to restart each one.
    • With no companion on the data dir there is nothing to compare, and tools run.
  • Companion. /internal/notify now takes a revision and returns 409, logged at error level, when it differs or is absent. This is the only way the companion notices an MCP server or CLI from before this change, since those have no check of their own. The MCP _notify and the CLI reset-to send it. The SSE event leaves it out.
  • Buckaroo. Buckaroo isn't built from this repo, so its check is package-version equality. BuckarooManager.start() compares the version from /health with buckaroo.__version__ in the companion process. On a mismatch it stops the server, logs an error and raises BuckarooUnavailable. This happens when the venv is synced to another buckaroo under a running companion and the server is then respawned.

Uncommitted edits count. An MCP server started before an edit will refuse every tool once the companion is restarted after that edit, until /mcp → tallyman → Reconnect.

A refusal from the end-to-end run below:

tallyman refused project_list: this MCP server and the companion on its data dir run different source.
  MCP server (pid 72537): 97b9805-dirty.f3c30d from /Users/paddy/code/tallyman2-revision-check
  companion (pid 72502, port 17901): no revision (it was started from source older than the revision check)
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).

Tests

  • Red: 97b9805. CI failed these 7 in the fast suite and nothing else (7 failed, 1124 passed, 1 skipped):
    • test_uncommitted_edits_change_the_revision (ImportError)
    • test_the_server_record_names_the_revision_it_runs (KeyError)
    • test_mcp_tools_refuse_to_run_against_a_companion_on_another_revision (DID NOT RAISE)
    • test_mcp_tools_refuse_a_companion_that_records_no_revision (DID NOT RAISE)
    • test_companion_refuses_a_notify_from_another_revision (200, not 409)
    • test_mcp_notifies_and_links_the_companion_of_its_own_data_dir (payload without revision)
    • test_cli_reset_notifies_the_companion_of_its_own_data_dir (KeyError)
  • Red, integration. The integration job needs the fast job, so it was skipped on the red commit. The two integration tests failed locally only, with DID NOT RAISE:
    • test_integration_start_refuses_a_buckaroo_other_than_the_companions
    • test_stdio_tool_call_against_a_companion_on_another_revision_is_a_tool_error
  • Fix: 8bf1d84. It also updates 18 existing test posts to /internal/notify to send the revision, and one exact-payload assertion.
  • CI on 8bf1d84: fast suite 1131 passed, 1 skipped; integration 9 passed, including the two new tests; ruff clean; perf report passes.
  • Merged with main on 2026-10-02. main (4e505b8, including the buckaroo 0.15.9 bump) is merged in. Main's own lint has failed since f074726, so this branch also carries fix(scripts): wrap the three lines over 120 columns in rebuild_nfl_salaries_catalog.py #302's one commit (204dcae) so that CI's lint passes and the test jobs run. That commit leaves this diff once fix(scripts): wrap the three lines over 120 columns in rebuild_nfl_salaries_catalog.py #302 is merged.
  • CI on b12facb: fast suite 1131 passed, 1 skipped; integration 9 passed; ruff clean; perf report passes. Locally on the merged tree: fast suite 1131 passed, integration 9 passed.
  • Local:
    • Fast suite: 1123 passed, 6 skipped, 1 failed. The failure is test_fouc.py::test_unknown_api_path_404s: 503 because this worktree had no built packages/app/dist. It passes after pnpm build.
    • Integration: 9 passed.
    • uvx ruff check is clean.
  • End to end, against real processes on a scratch data dir and port 17901 (~/.tallyman-notebooks and :7860 untouched):
    1. Companion (with Buckaroo) and MCP server both from this branch. The owner record carries 97b9805-dirty.f3c30d and the checkout. Buckaroo passed the version check and started. project_list over stdio returned normally.
    2. Companion from the base checkout at 4996808 (code from before this change), MCP server from this branch. project_list came back as the tool error above.
    3. Companion from this branch, tallyman reset-to 0 from the 4996808 checkout. The CLI printed the companion at http://127.0.0.1:17901 refused the reload: notify refused: the client runs no revision ..., and the companion logged the refusal.

Found along the way, not fixed here

  • macOS-only checkpoint test failures. On macOS, 12 checkpoint-count tests in test_reset_to_revision.py and test_recalc_surfaces.py fail locally in about 2 of 5 runs, on main as well (checked against 719a3e8).
    • The tests' _git/_steps helpers fork git through subprocess.run.
    • The forked child segfaults before exec, inside PROJ's pthread_atfork child handler: SQLiteHandleCache clear → sqlite3Close → sqlite3_log → os_log. This is in the macOS crash reports.
    • The checkpoints themselves land, because run_git uses posix_spawn.
    • It only starts after the first test that runs a query in-process. What decides 2 runs in 5 is still unknown.
    • Fix: the helpers should use git_util.run_git.
  • catalog_list over stdio. It fails fastmcp's client-side output-schema validation: it is annotated list, but _tag_project returns a dict.
  • .mcp.json. It sets cwd to /Users/paddy/tallyman, the old clone.

🤖 Generated with Claude Code

paddymul and others added 2 commits September 29, 2026 12:19
…fferent 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 <noreply@anthropic.com>
…fferent source

- The revision names uncommitted edits: a dirty checkout is `<sha>-dirty.<hash of git diff HEAD>`, 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 <noreply@anthropic.com>
@paddymul
paddymul marked this pull request as ready for review September 29, 2026 18:25
paddymul and others added 2 commits October 2, 2026 10:11
…laries_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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant