Skip to content

Preserve transport truth and batch attributed requests - #749

Open
tony wants to merge 8 commits into
engine-ops-on-seamfrom
engine-ops-hardening
Open

Preserve transport truth and batch attributed requests#749
tony wants to merge 8 commits into
engine-ops-on-seamfrom
engine-ops-hardening

Conversation

@tony

@tony tony commented Aug 23, 2026

Copy link
Copy Markdown
Member

Summary

  • Fix four ways the engine transports could lie about what happened: misrouted commands, misattributed results, leaked child processes, and pane output left encoded.
  • Replace semicolon-folded command strings with ordered batches of distinct requests, so a control-mode reply can be matched to the operation that caused it.
  • Fix the MCP adapter closing engines it does not own, and make one close survive repeated cancellation.
  • Add session x window x pane matrix scenarios that verify their live postconditions rather than assuming them.
  • Fix a sync warmup that verified the full session set from inside the per-session build loop.

Changes by area

Transports

  • src/libtmux/_internal/tmux_argv.py, engines/connection.py: protect literal semicolons and reject incompatible engine connections.
  • experimental/engines/control_mode.py: correlate replies without treating guard-shaped output as protocol.
  • experimental/engines/async_control_mode.py, asyncio.py: drain and reap async children through cancellation and reader failures.
  • pane.py, experimental/mcp/events.py: expose decoded pane-output bytes to control-mode consumers.

Planner

  • experimental/ops/: ordered batches of distinct requests in place of folded command strings, with planner partitions, dependency boundaries, and result cardinality validated. In-flight plans and bindings are snapshotted across sync and async execution.
  • Borrowing tmux's server-global marked pane is gone; a plan no longer depends on process-wide state another client can move.

MCP lifespan

  • experimental/mcp/: model engine ownership and close only adapter-owned instances, shielding one close task through repeated cancellation with stable precedence.

Benchmark matrix

  • Report planner, request, engine-call, and process-model quantities separately, so a change that moves cost between them reads as movement rather than as a win.
  • Build every warmup session before topology verification, and clean the set only once it passes.

Design decisions

Batches, not folded strings. Folding several commands into one semicolon-separated string erases control-mode's reply boundaries: the transport returns output with no way to say which command produced which part, so a failure gets reported against the wrong operation. Ordered batches keep one request per result.

Close only what you own. An adapter handed an engine by its caller must not close it — the caller may still be using it. Ownership is now modelled explicitly rather than inferred from who happened to construct it.

Warmup completes before it is checked. Verifying the full expected topology from inside the per-session build loop asserts a postcondition that cannot hold until the loop finishes, so every multi-session scenario failed on its first build.

Test plan

  • uv run ruff check . — lint clean
  • uv run ruff format . — formatting clean, tree unchanged
  • uv run mypy — types clean
  • uv run pytest --reruns 0 — see the note below
  • just build-docs — docs build clean
  • Argv tests — literal semicolons survive rendering; incompatible connections are refused
  • Correlation tests — guard-shaped pane output is not mistaken for protocol
  • Async lifecycle and supervisor tests — children drain and reap under cancellation and reader failure
  • MCP settle tests — concurrent clients, borrowed engines, and close-error propagation
  • Matrix tests — the public CLI regression covers multi-session sync warmup end to end

tests/test_server.py::test_new_session_shell_env fails from a shell whose environment exceeds tmux's command-length limit; it is unrelated to this branch and is fixed higher in the stack.

@tony
tony force-pushed the engine-ops-hardening branch from d6347b9 to 1f7c749 Compare August 23, 2026 01:21
@codecov

codecov Bot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.79502% with 138 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.56%. Comparing base (54d10ec) to head (552ac5e).

Files with missing lines Patch % Lines
src/libtmux/_internal/tmux_argv.py 60.22% 27 Missing and 8 partials ⚠️
...libtmux/experimental/engines/async_control_mode.py 77.53% 21 Missing and 10 partials ⚠️
src/libtmux/engines/connection.py 70.70% 22 Missing and 7 partials ⚠️
src/libtmux/experimental/engines/control_mode.py 74.46% 10 Missing and 2 partials ⚠️
src/libtmux/experimental/engines/asyncio.py 87.34% 5 Missing and 5 partials ⚠️
src/libtmux/experimental/mcp/_lifespan.py 83.01% 5 Missing and 4 partials ⚠️
src/libtmux/server.py 73.33% 6 Missing and 2 partials ⚠️
src/libtmux/exc.py 75.00% 2 Missing ⚠️
src/libtmux/experimental/ops/plan.py 97.22% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##           engine-ops-on-seam     #749      +/-   ##
======================================================
+ Coverage               72.50%   73.56%   +1.05%     
======================================================
  Files                     171      172       +1     
  Lines                   11469    11809     +340     
  Branches                 1821     1898      +77     
======================================================
+ Hits                     8316     8687     +371     
+ Misses                   2490     2449      -41     
- Partials                  663      673      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tony
tony force-pushed the engine-ops-hardening branch from 80c20ae to 8389250 Compare August 23, 2026 13:31
@tony
tony force-pushed the engine-ops-hardening branch from 8389250 to 65304a8 Compare August 23, 2026 15:35
@tony
tony force-pushed the engine-ops-hardening branch from 65304a8 to 9a33450 Compare August 23, 2026 15:58
tony added 8 commits August 23, 2026 12:47
why: Direct argv parsing, pinned server scope, control-mode correlation, and
async subprocess cleanup could misroute commands, misattribute results, leak
process resources, or leave pane output encoded.

what:
- Protect literal semicolons and reject incompatible engine connections
- Correlate control replies without treating guard-shaped output as protocol
- Drain and reap async children through cancellation and reader failures
- Expose decoded pane-output bytes to control-mode consumers
why: Semicolon folding erased control-mode reply boundaries, reported failures
against the wrong operations, and borrowed tmux's server-global marked pane.

what:
- Replace folded command strings with ordered batches of distinct requests
- Validate planner partitions, dependency boundaries, and result cardinality
- Snapshot in-flight plans and bindings across sync and async execution
- Make workspace, fluent, query, docs, and CLI surfaces use batching vocabulary
why: Adapter lifespans did not distinguish borrowed from owned engines, and a
second cancellation or close failure could strand resources or mask the body
exception.

what:
- Model engine ownership and close only adapter-owned instances
- Shield one close task through repeated cancellation with stable precedence
- Align plan preview and execution defaults on the batching planner
- Cover concurrent clients, borrowed engines, and close-error propagation
why: The earlier workload could not measure planner batching and conflated
planner steps, engine calls, tmux requests, and transport process models.

what:
- Add session x window x pane scenarios with verified live postconditions
- Report planner, request, engine-call, and process-model quantities separately
- Mark historical timings stale until regenerated with the current workload
- Add lossless source-scroll and bounded-overload async control demos
why: Sync warmup checked the full expected session set from inside the
per-session build loop, so it validated every requested name after building
only the first one. Every multi-session scenario aborted after its first
build, which took valid hierarchy benchmarks down with it.

what:
- Build every warmup session before topology verification, and clean the
  set only once it passes
- Cover it through the public matrix command, requiring a clean exit, so
  the regression is pinned at the surface a user actually invokes
why: The script carries a `uv run --script` shebang but shipped without the
executable bit, so the shebang was misleading: the one invocation it
advertises could not work. ruff's EXE001 catches exactly this, and CI runs on
Linux where the rule applies -- it is skipped on Windows and WSL, so a
developer working there cannot see it locally.

what:
- Record the file as mode 755, matching the other runnable scripts
… it does not

`scripts/bench/primitives.py` exists so a benchmark need not carry its own
copy of the isolation, naming, and statistics helpers. The grid carried both:
a second hermetic-isolation routine is exactly the case that module was
extracted to prevent, since each copy is free to forget one of the tmux
behaviours the other encodes -- which is how they came to disagree about
`config_file` in the first place.

Delegated because they are equivalent once the private-name prefix is
normalised: `new_server`, `uniq`, `percentile`, `summarize`, `STAT_LABELS`,
and the scratch-directory machinery behind them.

Kept local because they are not the same function:

- `build_classic` applies `BENCH_OPTIONS`, which exists to hold this grid's
  cells steady and means nothing to a benchmark that does not have cells.
- `parse_shape` here accepts the multi-session `SxWxP` scenarios this grid
  sweeps and rejects the rest; the shared one parses the two-axis `WxP` its
  own callers use.

`reap_stale_scratch` returns a count rather than printing one, because a
module with no console cannot decide how a caller reports; the grid does that
with the console it already had.

Verified: `contract` reports parity with exit 0, the script's tests pass, and
a `classic,mock` grid builds and reports percentiles through the shared
helpers.
…ripts

Both cover `scripts/`, so they belong beside the other script tests rather than
in the library's half of the `tests/` namespace. Their repository-root lookups
count directories, and the counts move with them.

One stale path travelled with them. The commit that moved the grid to
`scripts/bench/engines.py` sits a branch below, but a later commit here adds a
test that spawns the grid by path and was written against the old spelling, so
it named a file that no longer exists. It ran `uv run scripts/bench_engines.py`
and failed to spawn.

That is the shape of this whole change: a path written down in one place and
not the other. The mirrored layout is what makes the pairing checkable -- a
test under `tests/scripts/bench/` has exactly one script it can be about.
@tony
tony force-pushed the engine-ops-hardening branch from 9a33450 to 552ac5e Compare August 23, 2026 18:09
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