Pluggable command engines: batching, async, and tmux-free tests - #738
Open
tony wants to merge 32 commits into
Open
Pluggable command engines: batching, async, and tmux-free tests#738tony wants to merge 32 commits into
tony wants to merge 32 commits into
Conversation
why: Connection flags and process dispatch were written four times across core with four different answers, and there was no way to run a tmux command through anything but a fork of the tmux binary. what: - Add libtmux.engines: TmuxEngine protocol, CommandRequest/CommandResult, ServerConnection, and SubprocessEngine as the default - Add Server(engine=...); an engine naming no server adopts the server's connection so it cannot dispatch to the ambient tmux server - Route Server.cmd, Server.raise_if_dead, neo.fetch_objs and the control-mode client through one ServerConnection - Escape a trailing ";" in command arguments per tmux's cmd_parse grammar; CommandSeparator marks an intentional boundary - Deprecate tmux_cmd.process; keep tmux_cmd as the declared return type - Document in docs/topics/engines.md, docs/api/libtmux.engines.md, MIGRATION and CHANGES
why: The seam shipped a protocol but nothing to plug into it. Simulating tmux is not viable — listing queries ask for 136 version-gated format fields per row, and a fake that answers unknown commands optimistically reports that every session exists while none do. what: - Add RecordingEngine and ReplayEngine: record real tmux answers, serve them back with no server running; tapes round-trip through JSON - Fail closed on an unrecorded command with exc.UnscriptedCommand - Add the recording_server pytest fixture, usable as a spy - Add CommandResult.ok/raise_for_status(), mirrored on tmux_cmd - Give TmuxEngine.run_batch a default body so subclasses implement run - Declare AsyncTmuxEngine for persistent-connection engines - Validate engine= at construction, naming a missing method and rejecting an async engine that satisfies the structural check - Document in docs/topics/engines.md, the API page and CHANGES
why: Version detection was the one dispatch site the seam never covered. neo.fetch_objs resolved the version-gated -F template by running tmux -V through its own engine, so a replay still needed the binary it exists to avoid — and the lenient list accessors turned the resulting TmuxCommandNotFound into an empty result rather than an error. what: - Resolve the tmux version from the engine when it reports one, falling back to tmux -V for engines that cannot - Record the tmux version in the tape; ReplayEngine reports it, so a listing query resolves with no tmux installed - Propagate exc.UnscriptedCommand through Server.sessions and .clients; leniency covers an unreachable tmux, not an untaught engine - Elide long arguments in UnscriptedCommand and name the recorded version, so a miss on a listing query is legible - Describe the format-field set as version-gated rather than counting it
why: A tape keyed by argv kept one answer per command, so a query whose answer changed — list-sessions before and after a session is created — replayed the end state for both. A test asserting that transition passed against the wrong value, silently. what: - Record an ordered sequence of Exchange values instead of a mapping; ReplayEngine serves each command's answers in the order recorded - Repeat an answer that never varied, so a read-only query stays usable more often than it was recorded - Raise once a varying answer is exhausted, naming how many times it was recorded and requested - Build from_dict as a list, not a dict comprehension, which was collapsing duplicates back to the last answer - Accept a mapping for a hand-written tape with one answer per command
why: Swapping an engine meant rebuilding the Server, so every caller that wanted to record or fake a section of code rebuilt one by hand and had no way back. Recording in particular needs the engine already in use, which a constructor argument cannot express. what: - Add Server.using(engine): dispatch through engine for a block, restore on the way out including on exception, nesting in reverse - Validate the engine there exactly as the constructor does - Add Server.recording(): wrap the engine already in use, yield the recorder, restore afterwards - Document both in docs/topics/engines.md and CHANGES
why: Nothing in libtmux calls run_batch, which twice read as dead weight worth deleting. It is the override point a persistent-connection engine uses to pipeline down one connection, so removing it would have to be undone as a breaking protocol change. what: - Record the rationale on TmuxEngine.run_batch
why: Moving Server.cmd() to CommandResult means callers meet tuples where tmux_cmd gave lists. Two patterns break on that: equality against a list (17 sites in-repo, unknown downstream) and isinstance(stderr, list), which gates three error paths that would otherwise stop raising silently. what: - Add Lines, a read-only list subclass that also compares equal to a tuple, and normalize CommandResult's cmd/stdout/stderr to it - Annotate those fields Sequence[str], which is what they now are: indexable, iterable, and not mutable - Update the doctests that showed tuple reprs Proposal branch: Server.cmd() still returns tmux_cmd. This is the enabling step, measured separately so the flip can be judged on its own.
why: An application that picks its tmux transport from a config file or a CLI flag had to import the implementing class, so a packaged engine could not be selected without libtmux knowing about it. what: - Add create_engine/available_engines/register_engine/unregister_engine, with "subprocess" and "replay" registered - Read the libtmux.engines entry-point group on first use, not at import; scanning costs several ms against a ~50ms import and most programs never resolve by name - Skip a distribution whose engine fails to load rather than making every other engine unresolvable - Name the registered engines in the unknown-name error
why: Two result types described the same thing. tmux_cmd could only ever wrap a subprocess, so it raised for .process on an engine that forks nothing, and its output could be mutated after the command had finished. what: - Server.cmd() and the Session/Window/Pane counterparts return CommandResult; stdout/stderr/cmd read as before and stay list-typed - Output is read-only, so mutating a finished command's result raises - .process is the Popen or None, rather than raising when absent - Widen Hooks.from_stdout to Sequence[str]; convert at the boundaries that promise list[str] - tmux_cmd is unchanged and still constructible directly - Document in MIGRATION and CHANGES
why: Returning CommandResult from cmd() left every command building a deprecated tmux_cmd just to read a result off it, so the back-compat class sat on the hot path of the thing replacing it. what: - Add common.dispatch(): the single path from an engine to a result, owning the debug logging and tmux's has-session stdout quirk - Route Server.cmd() and neo.fetch_objs() straight through it - Rebuild tmux_cmd on top of it, so it is a leaf nothing depends on - Drop the tmux_cmd.result property, which existed only to bridge the two and is now unreferenced
why: The seam exists so a transport other than fork-per-command can be plugged in, but nothing proved one could. A persistent `tmux -C` engine needed one helper core did not expose, so the claim was untested. what: - Add render_control_line() and unescape_control_output(): encode a command for tmux's line-oriented control parser, and decode a %output payload back to the bytes a pane wrote - Add a worked example holding one long-lived tmux -C connection, and assert the object API traverses through it — listing queries included
why: AsyncTmuxEngine shipped as a type nothing could consume — Server is synchronous and refuses one, and there was no async dispatch path — so it advertised a capability that did not exist. what: - Add AsyncSubprocessEngine: awaits the tmux binary, output handling identical to the synchronous engine - Add common.adispatch(), the async twin of dispatch(), applying the same logging and has-session adaptation - Server still refuses an async engine and says why
why: run_batch existed as an override point no public API could reach, so the fastest path in the system was unusable. Measured against tmux over repeated runs at 10, 40 and 160 commands, a persistent connection is roughly 200x faster than forking per command and ~10x faster than issuing the same commands one at a time; the fork side varies widely with load. what: - Add common.dispatch_batch(), handing the whole sequence to the engine instead of looping over single dispatches - Extract the has-session adaptation both dispatch paths share - Show pipelining in the control-mode example, and assert a batch round-trips
why: unescape_control_output shipped exercised only by doctests with synthetic input, so nothing showed it handled what tmux actually sends. what: - Assert an attached control client is pushed %output, and that decoding a real payload recovers the bytes the pane wrote - Record the two traps the probe hit: a control connection that never attached sees no output, and select() on a buffered text stream reports nothing to read while Python still holds lines
why: Delivering notifications looked like it needed concurrency. It does not: replies and pushes share one stream, so reading a reply already walks past any notification that landed first. Collecting them instead of discarding them costs a list and a branch. what: - Collect %output lines in the control-mode example rather than skipping - Assert a pane's output can be waited for by polling with cheap commands, each of which drains what tmux pushed - Say plainly that delivery is poll-driven, and that pushing the instant output appears is the part needing a thread
why: Push delivery was the last thing called genuinely concurrent and therefore large. It is one reader thread: the stream already interleaves replies and notifications, so separating them is the whole job. what: - Add an example engine whose reader thread routes reply blocks to a queue and everything else to a callback, leaving run() synchronous - Assert output lands while the caller issues no commands at all, which is what distinguishes this from draining replies - Close by shutting stdin and joining the reader, and assert it exits
why: raise_if_dead called the engine directly, so it was the one tmux command in the library that produced no debug record — invisible to anyone reading the log to find out what libtmux ran. It also skipped the adaptations every other command gets. what: - Dispatch the list-sessions probe like any other command - Assert it logs, and that it still raises CalledProcessError
why: Reconnection was the last part of a persistent-connection engine still assumed to be large. It is a liveness check and a respawn; backoff, in-flight recovery and replaying attach state are the hardening on top. what: - Reconnect lazily in the example engine when the connection has died, and count it - Assert commands and traversal resume after the client is killed - Replace a fixed sleep in the notification example with a readiness round trip, halving the example suite's runtime and the load it adds to a timing-sensitive neighbour
why: The control-mode example opened its connection with `new-session -A -s _control`, which is the easy way and the wrong one: the session it makes is real and outlives the connection, so merely attaching an engine changed what the caller saw in server.sessions. what: - Attach to a session the caller names, matching the push example - Say why in the class docstring, since new-session -A is the obvious thing to reach for - Assert connecting adds no session
why: Splitting __init__ into a respawnable _spawn() left the notification list being created inside _spawn, guarded by hasattr so a reconnect would not wipe it. The guard hid the ordering rather than fixing it. what: - Initialize notifications and reconnects in __init__, before the first connection, so _spawn only opens a connection
why: dispatch_batch reached the engine's batch path but lived in libtmux.common and took an engine and raw argv, so using it meant reaching past the object API. run_batch stayed effectively unused. what: - Add Server.cmd_batch(): one result per command, in order, a tmux-side failure reported on its own result rather than truncating the batch - Say plainly that the speedup depends on the engine — the default forks per command either way; measured against a control-mode engine, forty commands took about an eighth as long batched - Document in docs/topics/engines.md and CHANGES
why: The sync side gained dispatch_batch while the async side had only adispatch, so an async caller wanting to batch had to call run_batch on the engine directly — skipping the has-session adaptation and logging, which is exactly the gap dispatch_batch closed for sync. what: - Add common.adispatch_batch(), the async twin of dispatch_batch - Assert it hands the whole sequence to run_batch once, and adapts each result the way single dispatch does
why: Every capability added to one dispatch path has at some point been forgotten on the other — adispatch shipped without a batch twin, and the async engine never gained tmux_bin. what: - Add AsyncSubprocessEngine.tmux_bin, matching the synchronous engine - Assert the two engines expose the same public surface, so the next divergence fails a test rather than shipping
why: The example claimed a command in flight when the connection dropped is lost. Measured, it is not: if tmux had already written the reply, the pipe buffer holds it and the next read still returns it. The real gap was unmentioned — when the tmux server goes away, writing raises BrokenPipeError, an OSError rather than a LibTmuxException, so a caller guarding against libtmux errors does not catch it. what: - Replace the claim with the measured behavior - Name the server-death case and what a hardened engine owes it
why: TmuxEngine.run said nothing about failure, so a third-party engine author had no target and callers had nothing reliable to catch. The control-mode example proved the cost: when the tmux server went away it leaked BrokenPipeError, an OSError that `except LibTmuxException` misses. what: - Add exc.EngineError for a command that never reached tmux, keeping a tmux-side rejection as data on the result - Reparent TmuxCommandNotFound under it, which widens the hierarchy and leaves existing handlers working - State the contract on TmuxEngine.run - Translate the dead-connection write in the control-mode example
why: A failed attach was reported as an ordinary result. tmux answers `attach-session -t missing` with a %begin/%error block and exits, so the engine read returncode 1 and handed back something indistinguishable from tmux rejecting a command — while the connection was, in fact, dead. Reconnecting against a gone server also spawned a client per attempt, 11 for 10 commands, ~11ms each, every one reported as a failed command. what: - Check the handshake block's status and raise EngineError naming the session and what tmux said - Document _read_block's failure, and that the stream closing mid-block is a transport failure rather than an empty success
why: Pipelining pairs a reply with its command by position alone. That is correct — measured, tmux answers in the order it was asked, including when one command fails — but the engine discarded the id tmux tags each reply with, so a desynchronized stream would have attributed every later reply to the wrong command, silently. what: - Keep the id from %begin and reject a block whose %end or %error carries a different one, as EngineError - Assert a batch's results pair with their commands, failure included
why: The example is what someone copies to write an engine, and it implemented only TmuxEngine. An engine built from it forgoes connection adoption — so Server(socket_name=..., engine=it) can reach the ambient tmux server — and has its tmux version resolved by running the binary rather than being asked. what: - Add connection, with_connection and tmux_version, so the template produces a well-behaved engine - Say in each docstring what omitting it costs, since all three fail quietly - Assert the example satisfies SupportsConnection and SupportsTmuxVersion
why: The guide taught the protocol and then left readers to invent the rest. Two complete engines live in the test suite encoding the traps that cost the most to rediscover, and nothing outside a changelog line mentioned them — so a reader wrote a toy from the sketch and met the traps one at a time. what: - Name both examples and what each demonstrates, next to the section that teaches writing one - List the traps they encode, so the guide is useful even to someone who never opens them
why: The suite's docstring rule failed on the new engine tests, which turns the branch's own lint gate red. what: - Say what each registry, dispatch, and result-compat case proves - Describe what test_result_compat covers rather than when it was written
why: A distribution whose engine will not import was skipped in silence, so a name that should resolve simply was not there, with nothing to explain why. Reporting the failure is also what makes catching a third party's arbitrary exception legitimate. what: - Warn with the traceback and the entry-point name, then carry on - Cover the skip-and-report path with a deliberately broken entry point
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #738 +/- ##
==========================================
+ Coverage 52.37% 54.57% +2.20%
==========================================
Files 26 35 +9
Lines 3729 4405 +676
Branches 747 812 +65
==========================================
+ Hits 1953 2404 +451
- Misses 1472 1679 +207
- Partials 304 322 +18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
why: The changelog convention carries the pull request ref in each deliverable heading, and the number only existed once the pull request was opened. what: - Add the ref to the fifteen headings this branch introduced
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TmuxEngine— an object that takes a rendered argv and returns a structured result.SubprocessEngineis the default and forks the tmux binary exactly as before, so existing code is unaffected.Server(engine=…),Server.using()for a block,Server.recording(), and name resolution throughcreate_engine()with an entry-point group so a packaged engine registers itself.Server.cmd_batch()plusdispatch_batch/adispatch/adispatch_batch. A tmux-side failure lands as data on its own result rather than an exception that truncates the rest of the batch, and an engine holding a persistent connection can write every command before waiting for the first reply.RecordingEngine/ReplayEngine, JSON-serializable tapes, and arecording_serverfixture — the whole object API (sessions,windows,panes) works offline against recorded output, and a replay fails closed withUnscriptedCommandon a command it never saw.tmux_cmdwithCommandResultas the return ofcmd().stdout,stderr,cmd, andreturncoderead as before and the sequences still compare equal to lists, but they are read-only;okandraise_for_status()come with them.-L/-S/-f/-2/-8flags intoServerConnection. Four copies previously disagreed about which flags to emit, which is whyconfig_file=andcolors=were honored on some commands and not others.;in an argument being swallowed by tmux's parser;CommandSeparatornow expresses a deliberate command boundary.Full user-facing detail is in
CHANGES; the guide isdocs/topics/engines.md.Changes by area
Engine layer —
src/libtmux/engines/base.py: theTmuxEngine/AsyncTmuxEngineprotocols,CommandRequest/CommandResult,CommandSeparator, the optional capability protocols (SupportsCommandLine,SupportsConnection,SupportsTmuxVersion), and the control-mode codecsrender_control_line/unescape_control_output.connection.py:ServerConnection— sole owner of the tmux binary path and connection flags, derived from the server's public attributes on each use so reassigningsocket_nametakes effect on the next command, memoizing itsshutil.whichlookup.subprocess.py/asyncio.py: the default forking engine and its awaiting counterpart.record.py:RecordingEngine,ReplayEngine,Tape,Exchange.registry.py: name resolution,available_engines(), entry-point discovery on first use.Core dispatch
common.py:dispatch,dispatch_batch,adispatch,adispatch_batch;tmux_cmdgainsok/raise_for_status()and deprecatesprocess.server.py:engine/connectionproperties,cmd_batch(),using(),recording(), engine validation at construction, andraise_if_dead()routed through dispatch so it captures tmux's message onto the exception instead of letting it hit the terminal.exc.py:EngineErrorfor transport failure withTmuxCommandNotFoundreparented beneath it;UnscriptedCommandfor a replay gap.Docs and worked examples
docs/topics/engines.md,docs/api/libtmux.engines.md: guide and reference.tests/examples/engines/: a control-mode engine written entirely against the public engine API, push notifications, and recording.Design decisions
Protocol, not base class.
TmuxEngineis atyping.Protocol, so any object withrun()andrun_batch()qualifies and a third-party engine takes on no dependency on libtmux's class hierarchy. Subclassing it remains supported purely to inherit arun_batchthat loops, so a stateless engine implements onlyrun.Validate the engine where it is supplied.
Serverrejects a non-engine at construction, naming the missing method, instead of failing with anAttributeErrorinside the first command. An async engine needs its own explicit rejection: aruntime_checkableProtocol compares method names, not signatures, so an async engine satisfies the structural check while returning coroutines.Serverstays synchronous; async callers useadispatch.tmux rejecting a command is data; never reaching tmux is an exception. A nonzero result stays on the result object, which is what lets
cmd_batch()report per-command outcomes instead of stopping at the first failure. Only transport failure — missing binary, closed connection, desynchronized protocol — raises, asEngineError.An engine adopts the server's connection unless it names its own. Otherwise injecting an engine into a socket-scoped
Servercould silently dispatch to the ambient tmux server.A tape stores ordered exchanges, not one answer per command. Serving the final answer for a repeated command would report end state for an earlier step, so a command whose recorded answers run out raises instead of guessing.
Escape arguments rather than keep relying on tmux's
;parse. Passing;as a separator worked only because tmux consumed it as a command boundary — the same behavior that swallowed a;a caller meant as data. Escaping fixes the data case;CommandSeparatormakes the separator case explicit.Before / After
A trailing
;a caller meant as data:Before, the pane received
echo hello. After, it receivesecho hello;.Test plan
uv run mypy src tests— cleanuv run ruff format --check .— cleanjust build-docs— builds, including the new topic and API pages and every{ref}fromCHANGESandMIGRATIONtests/test_engines.py,test_dispatch.py,test_async_engine.py,test_engine_registry.py,test_engine_error.py,test_result_compat.py— engine protocol conformance, dispatch adaptation, async paths, name resolution, transport-failure typing, andCommandResult/list compatibilitytests/examples/engines/test_control_mode_engine.py— a persistent-connection engine driving the object API using only public engine exports, against real tmuxtests/examples/engines/test_recording_engine.py— record against tmux, replay with no server running;verified live:pane.cmd("send-keys", "echo hello;")arrives with the semicolon intactRelated
The engine seam here is the core-facing extraction of the layer explored in #690.