Skip to content

feat(server): sidecar handshake contract for headless hosts#717

Open
paddymul wants to merge 3 commits into
mainfrom
feat/server-sidecar-handshake
Open

feat(server): sidecar handshake contract for headless hosts#717
paddymul wants to merge 3 commits into
mainfrom
feat/server-sidecar-handshake

Conversation

@paddymul
Copy link
Copy Markdown
Collaborator

@paddymul paddymul commented May 9, 2026

Summary

Adds the small server-side surface that a parent supervisor (Tauri desktop app, Electron, raw subprocess driver) needs to spawn python -m buckaroo.server without polling for readiness, inventing session IDs, or hard-coding ports.

All changes are additive and backward-compatible. The existing python -m buckaroo.server --port=8700 --no-browser invocations are unchanged. The MCP tool, standalone browser-tab flow, and Jupyter widget paths all keep working.

Diff: 6 files, +67 / -9.

What changed

  • --port=0 random binding. tornado.netutil.bind_sockets recovers the OS-assigned port (Application.listen doesn't expose the bound port). On listen, prints BUCKAROO_PORT=<n> to stdout (line-buffered) before any other output — a deterministic readiness signal for parent processes.
  • --stdio-control flag. Server exits when stdin closes. Lets a parent guarantee the sidecar dies when the parent does, more reliable than process-tree teardown across platforms.
  • protocol_version: 1 field on initial_state WS messages. Clients can warn on mismatch when the wire format changes incompatibly.
  • LoadHandler mints a UUID session server-side when the caller omits it; previously errored. Hosts that already pass a session ID keep working unchanged.
  • New buckaroo-server console script entry point (pyproject.toml). Existing buckaroo-table MCP script unchanged. Makes which buckaroo-server discoverable from PATH so a parent can derive the right Python interpreter.
  • must be running inside ipython warning routes to stderr. Previously polluted stdout, fine for interactive Jupyter use but breaks any pipe-captured handshake.
  • mcp_tool.ensure_server passes --port=<SERVER_PORT> explicitly. Decouples the MCP tool from the argparse default.

Why ship this from buckaroo (vs. a downstream package)

The contract is small and inseparable from the WS protocol it depends on. Downstream desktop integrations (Tauri plugin, Electron wrapper, etc.) consume this contract over __main__'s stdout + the existing WS protocol, but the contract itself ships with buckaroo so it's versioned in lockstep with the WS messages.

Test plan

  • pytest tests/unit/ passes (963 passed, 6 skipped, 1 deselected for an unrelated pre-existing flake — test_uvx_no_stdout_pollution has a bug where it closes proc.stdin then calls proc.communicate() causing flush of closed file)
  • paddy-format (project's pre-commit hook) clean on all changed files
  • --no-browser still works (existing flag, unchanged)
  • MCP tool's view_data still passes (verified via test_view_data_call)
  • BUCKAROO_PORT=<n> line appears on stdout before any other output, even when the ipython warning would otherwise leak first
  • --port=0 binds a random port and the handshake reports the actual bound port (verified end-to-end against a Tauri host that parses the line and opens an internal WS to it)

Follow-ups (separate work)

  • Tests for the new behavior (handshake line, --stdio-control, server-mint session, protocol_version).
  • A new tests/unit/server/test_sidecar_contract.py smoke test that spawns a subprocess and asserts the contract: port handshake, line-buffering, sessionId minting.
  • The downstream Tauri plugin + JS adapter (buckaroo-tauri Rust crate, buckaroo-tauri-adapter npm package, example app) — these will land in a separate repo or follow-up PR. Plan + decision log lives in a worktree at .claude/worktrees/tauri-embedding-plan/TAURI_EMBEDDING_PLAN.md.

🤖 Generated with Claude Code

Adds the small server-side surface a parent supervisor (Tauri desktop app,
Electron, raw subprocess driver) needs to spawn `python -m buckaroo.server`
without polling for readiness, inventing session IDs, or hard-coding ports.

All changes are additive and backward-compatible. Existing
`python -m buckaroo.server --port=8700 --no-browser` invocations are
unaffected; the MCP tool, standalone browser-tab flow, and Jupyter widget
paths all keep working.

What changed:

* `--port=0` binds an OS-assigned port via `tornado.netutil.bind_sockets`
  (Application.listen doesn't expose the bound port); on listen, prints
  `BUCKAROO_PORT=<n>` to stdout (line-buffered) before any other output,
  giving parents a deterministic readiness signal.
* New `--stdio-control` flag: server exits when stdin closes. Lets a
  parent guarantee the sidecar dies when it does, more reliable than
  process-tree teardown across platforms.
* `initial_state` WS messages now carry `protocol_version: 1` so clients
  can warn on mismatch when the wire format changes incompatibly.
* `LoadHandler` mints a UUID `session` server-side when the caller omits
  it; previously errored. Hosts that already pass a session ID keep
  working unchanged.
* New `buckaroo-server` console script entry point (existing
  `buckaroo-table` MCP script unchanged) so `which buckaroo-server`
  resolves the right venv from PATH.
* `must be running inside ipython` warning now writes to stderr; previously
  polluted stdout, which is fine for interactive Jupyter use but breaks
  any pipe-captured handshake.
* `mcp_tool.ensure_server` passes `--port=<SERVER_PORT>` explicitly to the
  spawned server so it doesn't depend on the argparse default.

Why this and not a separate package: the contract is small and lives
inside the existing server module. Downstream desktop integrations (a
Tauri plugin, an Electron wrapper) consume this contract over `__main__`'s
stdout + WS protocol, but the contract itself ships with buckaroo so it's
versionable in lockstep with the WS messages it depends on.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

📦 TestPyPI package published

pip install --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo==0.13.5.dev25737317319

or with uv:

uv pip install --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo==0.13.5.dev25737317319

MCP server for Claude Code

claude mcp add buckaroo-table -- uvx --from "buckaroo[mcp]==0.13.5.dev25737317319" --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo-table

📖 Docs preview

🎨 Storybook preview

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 78184cb8f7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread buckaroo/mcp_tool.py
# Pass --port explicitly so we don't depend on the buckaroo.server default
# (the default has shifted historically; the MCP tool's port is governed
# by the BUCKAROO_PORT env var with a SERVER_PORT default).
cmd = [sys.executable, "-m", "buckaroo.server", "--port", str(SERVER_PORT)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the port fix to the packaged MCP entry point

The BUCKAROO_PORT fix is applied to buckaroo/mcp_tool.py, but the installed buckaroo-table console script still points at the top-level buckaroo_mcp_tool:main in pyproject.toml, and that file still starts python -m buckaroo.server without --port. In packaged/uvx MCP usage with BUCKAROO_PORT set to anything other than 8700, the tool will continue polling the requested port while the spawned server binds to the default port, so the startup failure this change is meant to address remains on the actual entry point.

Useful? React with 👍 / 👎.

Comment thread buckaroo/server/__main__.py Outdated
Comment on lines +55 to +56
sockets = tornado.netutil.bind_sockets(args.port, address="127.0.0.1")
bound_port = sockets[0].getsockname()[1]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Propagate the OS-assigned port into app settings

When the new --port 0 path is used and a /load request does not set no_browser, the server binds a real bound_port here but the application was created with make_app(port=args.port), so LoadHandler._handle_browser_window() still reads port=0 from settings and opens/focuses http://localhost:0/s/... instead of the actual listener. This breaks the browser-launch flow for random-port servers even though the stdout handshake reports the correct port.

Useful? React with 👍 / 👎.

Regression tests for the two codex review comments on this PR. Both fail
on main and on this branch as of 78184cb.

1. ``buckaroo-table`` console script entry — ``buckaroo_mcp_tool.ensure_server``
   spawns ``python -m buckaroo.server`` without ``--port``, so callers that
   set BUCKAROO_PORT≠default poll one port while the server binds another.
   Parametrized over both the packaged ``buckaroo_mcp_tool`` and the inner
   ``buckaroo.mcp_tool`` so the fix has to land in both.

2. ``__main__`` ``--port=0`` propagation — when the OS assigns an
   ephemeral port via ``bind_sockets``, the Application's
   ``settings['port']`` still holds the requested ``0``, so
   ``LoadHandler._handle_browser_window`` opens
   ``http://localhost:0/s/<id>``. Test imports a not-yet-existing
   ``bind_and_make_app`` helper that the fix will extract.

Pushing tests first per the TDD rule: every test seen failing on CI
before the matching fix lands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ntract

Three CI failures on this branch, addressed together so green CI lands in
one push:

* ``buckaroo_mcp_tool.py`` (packaged ``buckaroo-table`` entry) now passes
  ``--port=<SERVER_PORT>`` when spawning ``python -m buckaroo.server``,
  mirroring the fix already in ``buckaroo/mcp_tool.py``. Without this,
  callers that set ``BUCKAROO_PORT`` to anything other than the server's
  argparse default polled the requested port while the server bound the
  default — silent breakage for the installed MCP path.

* ``buckaroo/server/__main__.py`` extracts ``bind_and_make_app(port,
  open_browser)``, which binds first and then constructs the Application
  with the *bound* port. When ``--port=0`` requested an OS-assigned port,
  ``settings['port']`` previously held the stale ``0`` and
  ``LoadHandler._handle_browser_window`` opened
  ``http://localhost:0/s/<id>``. Now settings reflect reality.

* ``pw-tests/server.spec.ts`` updates the failing ``400 on missing
  session field`` test to assert the new intended contract:
  omitted ``session`` → 200 + server-minted UUID. The previous test
  pinned the old behavior the PR explicitly removed (Tauri/Electron
  hosts should not have to invent session IDs).

Tests added in c7df4b3 now pass; existing ``test_server.py`` already
covered the ``Missing 'path'`` 400 case so no Python coverage gap.

Co-Authored-By: Claude Opus 4.7 (1M context) <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