Skip to content

B7: an execution client reaches only a server it may use - #1211

Merged
trissim merged 20 commits into
mainfrom
refactor/B7-server-isolation
Oct 12, 2026
Merged

trissim merged 20 commits into
mainfrom
refactor/B7-server-isolation

Conversation

@trissim

@trissim trissim commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes a correctness bug: an execution client could connect to a server that another process tree started, and so run pipelines on another checkout's code. Receipt: docs/refactor/consolidation-20261009/B7-server-isolation.md.

Mechanism found

  • Every OpenHCS process of one user resolved port n to the same file, ~/.openhcs/ipc/openhcs-zmq-<n>.sock.
  • ZMQClient.connect attached to whatever answered a ping there. The reply's identity was not read.
  • DataControlPortPairAuthority.acquire returns a pair whose socket files are absent and reserves nothing. The parity test kept one such port for 30 workflows, each with its own short-lived server; between workflows another process could take the port, and the next workflow attached to it.
  • A persistent server runs in its own session and nothing stopped it; a non-persistent one was stopped only by its client's disconnect().

Fix (library half: OpenHCSDev/ZMQRuntime#32, zmqruntime 0.6.2)

  • A server reports its launch: code root, the process that started it, and its audience (owner only for a non-persistent server, any session for a persistent one).
  • A client connects to a server it started only if that server reports the launch it was handed. It attaches to another local server only if the code root matches and the audience admits it; otherwise EndpointLaunchMismatchError names the address and the reason.
  • The socket root is derived from the import root (~/.openhcs/<digest>/), so two checkouts do not share addresses by default. OPENHCS_SOCKET_ROOT replaces it for one process tree.
  • A server whose owner is gone stops: owner-only at once, any-session once idle.
  • Tests get a private socket root per pytest process (tests/conftest.py). The harness starts private servers through ZMQExecutionClient.with_private_server; the official30 tests no longer choose an execution port.

Files of in-flight surfaces touched

  • openhcs/agent/services/viewer_endpoint_discovery.py (U4): one expression, the socket directory now reads socket_root().
  • openhcs/runtime/zmq_execution_client.py, zmq_execution_server.py, zmq_execution_server_launcher.py (near K7): the launch argument and code_root.
  • benchmark/openhcs_measured_run.py, well_throughput_scaling.py, matched_cellprofiler_batch.py (near K1/K8): client construction only.

Release order

  1. Merge A client reaches only a server it may use; an orphaned server exits (0.6.2) ZMQRuntime#32 and release zmqruntime 0.6.2.
  2. Point external/zmqruntime here at that merge commit (it points at the PR branch now).
  3. Merge this PR. pyproject.toml already requires zmqruntime>=0.6.2.

Upgrade note: servers started before this change are not found (the socket root moved) and are not stopped by it. Stop them once by hand.

Tests run locally (this head, -n 2, private socket root)

  • Guards tests/unit/test_b7_server_isolation_guards.py: 6 passed.
  • tests/integration/test_execution_server_isolation.py (real launcher) with test_compiled_artifact_inspection.py: 7 passed.
  • Official30 parity under flock .claude/verify/official30.lock: 1 passed in 661.69 s.
  • 55 unit files that touch the changed code: the failures equal origin/main run the same way (45 failed, 26 errors on both; none only on this branch). 87 more ZMQ-related unit files: 919 passed, 2 failed in the parallel run, and both of those pass alone here and on main.
  • test_main (disk, ImageXpress, direct and zmq), test_session_journey, test_measured_cli_session, test_agent_observation_export_zmq, test_source_store_code_mode_zmq: 17 passed, 10 failed. The same 10 fail on origin/main (ZMQRuntimeObservationExportScope has no check_export_path).
  • Structural ratchet (openhcs, scripts, benchmark) and nra-r1, run locally against the merge base: exit 0, empty delta.
  • zmqruntime: 383 passed, 2 skipped, 2 failed (IPC path longer than sun_path under my --basetemp; both pass with a short one).

B7 · PR #1211 · guards 6/6 · production −12 +102 · tests −35 +291

🤖 Generated with Claude Code

trissim and others added 6 commits October 11, 2026 17:07
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 12, 2026 00:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

trissim and others added 11 commits October 11, 2026 21:19
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llow

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…penhcs/ipc

The test pinned the per-user ~/.openhcs/ipc directory that B7 replaced with
OpenHCSZMQConfig.socket_root(). It now derives the directory from that config
and checks that the viewer endpoint, viewer discovery and an execution
endpoint on the same port resolve the same address.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
trissim and others added 3 commits October 12, 2026 03:34
…nbindable paths and failed connects end the wait

The smoke check sets TMPDIR to a deep directory under the worktree. The test
run's socket root came from tempfile.mkdtemp, so every socket path was 136
bytes against a limit of 107 (zmq.IPC_PATH_MAX_LEN). Each server died at bind,
the client reported only 'Failed to connect', and the measured CLI waited for a
compilation that no event would ever end.

- use_private_socket_root() owns the test run's root: a new directory beneath
  checkout_socket_root(), exported as OPENHCS_SOCKET_ROOT for every child.
  tests/conftest.py and the unix_socket_dir fixture use it.
- ZMQExecutionClient.connect raises SocketPathTooLongError (path, length,
  limit) before it starts a server.
- A compile that cannot connect publishes CompilationFailed per dataset, so the
  measured CLI and other waiters end.
- Guards: a child process computes its parent's socket root, code root and
  audience; a private root binds whatever TMPDIR is; the client refuses an
  unbindable path without starting a server.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@trissim
trissim merged commit 907ed4d into main Oct 12, 2026
0 of 3 checks passed
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.

2 participants