Add pluggable engines and experimental tmux operations - #754
Open
tony wants to merge 229 commits into
Open
Conversation
why: Every tmux command forks the binary inline, so an alternative transport -- control mode, a recording, an in-memory fake -- cannot be substituted without copying the library, which is what the downstream work had to do. Connection flags were built in three places that disagreed, so config_file= and colors= reached tmux on some paths and not others. what: - Route dispatch through a TmuxEngine protocol, defaulting to a subprocess engine that forks exactly as before - Accept engine= on Server, and let an engine that names no server of its own adopt the server's connection rather than the ambient one - Derive one ServerConnection for cmd(), raise_if_dead() and fetch_objs() - Read the result's process field defensively, so an engine may return any structurally compatible result rather than only ours - Mark intentional command boundaries with CommandSeparator, so a ";" passed as data can never become one - Report a connection's tmux version behind SupportsTmuxVersion, for callers that render version-gated argv - Keep cmd() returning tmux_cmd, and arguments reaching tmux unchanged, so the default path behaves as it did
why: A custom tmux_bin names a program, not a server. An engine built with one and no -L/-S was treated as already knowing its server, so it was left unbound and every command reached whichever tmux server a flagless dispatch finds -- the silent ambient dispatch adoption exists to prevent. what: - Add ServerConnection.names_server, asking whether a connection carries connection flags of its own; the engine side of adoption reads it instead of is_unconfigured, which keeps its server-side meaning of "carries nothing at all" - Bind the server's flags onto such an engine while preserving the binary it was built with - Document the binary-is-not-a-server rule on Server.engine and in the CHANGES deliverable prose - Cover both adoption directions plus the two cases that already held, so a single-predicate regression cannot pass
why: The engine captures tmux's stderr instead of letting it reach the terminal, and the raise then discarded it, so a caller was left with an exit code and no way to recover what tmux said -- strictly less than the message the terminal used to show. what: - Pass the captured stdout and stderr to CalledProcessError, matching what CompletedProcess.check_returncode raises - Say so in the docstring and prove it in the doctest - Assert the socket name reaches the exception, which holds across both wordings tmux uses for an unreachable server
why: TmuxEngine and SupportsCommandLine are runtime_checkable Protocols, so isinstance() checks attribute names only -- never signatures, never async-ness. An engine with `async def run` satisfied them, was accepted by Server(engine=...), and failed on the first command with `AttributeError: 'coroutine' object has no attribute 'cmd'`, naming neither the engine nor the mismatch. An `async def command_line` failed the same way, one line earlier, whenever DEBUG logging was on. what: - Guard every engine capability in one place, _guard_sync(), reached through the typed _dispatch_run() and _dispatch_command_line() wrappers, so a mistyped call site is a mypy error rather than a runtime AttributeError - Collapse raise_if_dead onto self.cmd(), deleting the second dispatch site rather than guarding it twice - Reject a declared-async member before calling it, so the common shape never creates a coroutine; test the returned value too, since a plain def can still hand one back - Close a coroutine that did get created -- safe while unstarted, and suppressed against BaseException so a hostile awaitable cannot replace the diagnostic. Never cancel a Task or Future: one bound to another thread's loop would not receive it, and one shared with another awaiter would lose its result - Let AsyncEngineMismatch escape the list-accessor leniency; a misconfigured engine is not a tmux failure and must not read as "no sessions" - Add exc.AsyncEngineMismatch, naming the engine and the method, and document it on cmd() for Server, Session, Window and Pane - Show the failure as a runnable example in docs/topics/engines.md An eagerly-started Task (3.12+) has already run its body before run() returns; the guard reports it but cannot undo it. Nothing dispatches run_batch in-tree, so it gets no guard.
why: A caller measuring engine traffic cannot tell a request that ran one tmux command from one that inlined several into a single dispatch. The distinction lives in the argv -- a CommandSeparator marks a real boundary while a literal ";" is data -- and every engine already agrees on it through is_command_separator. Leaving the arithmetic to each observer invites them to count the encoded argv instead, where the separator has been flattened to a plain string and the inlining is invisible. what: - Add command_count beside is_command_separator, returning the separators plus one, with doctests covering a group and a literal semicolon - Export it from libtmux.engines
why: Counting or tracing tmux traffic meant patching subprocess.Popen from outside, which under-reports any engine that does not fork and cannot see a command group at all. TmuxEngine is a protocol, so the honest place for observation is a decorator that satisfies the same protocol: a program that wants none of it constructs none of it and runs the code it ran before, with no guard on the hot path. SQLAlchemy pays a boolean check per call for its event registry and Django builds a context mapping even with no wrapper registered; both are shaped by having concrete connection classes. A protocol lets the wrapper substitute for the engine wherever one is accepted, and cost nothing where it is not. what: - Add Sink, a before/after/error observer surface matching the hooks OpenTelemetry and Sentry already target on SQLAlchemy, so an exporter written against one reads naturally here - Add CountingSink, reporting requests, tmux commands, the commands that rode inside another request's argv, and elapsed time - Count on request.args rather than the encoded argv, where an engine has flattened the separator and the inlining no longer shows - Add InstrumentedEngine, which forwards anything the protocol does not cover to the engine it wraps - Pin the substitution property as a test: the wrapper is a TmuxEngine
why: The module ships in libtmux.engines but the API page enumerated only base, connection, and subprocess, so autodoc rendered no page for it and a reader following the engines docs would not learn observation exists. what: - Give libtmux.engines.instrumentation its own section and automodule entry - Record the deliverable in CHANGES, including why observation is composed rather than installed and what command_count distinguishes
why: Isolating a tmux server, naming a shape, and reducing samples to percentiles are the same job whichever transport is being measured, and none of it needs an engine -- only the classic Server the library already ships. Keeping one copy per benchmark invites them to drift, and the isolation half is exactly where drift costs: it encodes two tmux behaviours that were each found the hard way. new_server pins a keepalive session because killing a cell's session otherwise drops the server to zero, and under tmux's exit-empty default the next build can reach the socket mid-shutdown. reap_stale_scratch leaves a directory alone while a tmux still answers on it, because stealing a concurrent run's servers would be worse than leaking one directory. what: - Add scripts/bench/primitives.py with server isolation, shape parsing, the classic build, and the percentile summary, alongside the reasoning for the two behaviours above - Return the reaped count rather than printing it, so the module needs no console and its caller keeps its own reporting - Cover the shape, the statistics, the keepalive, and the reaper's refusal to touch a live directory
why: The seam's claim is that the default path is unchanged and that observation costs nothing when nobody asks for it. Both are asserted in prose and neither has a number behind it. A baseline is also what any later transport has to be compared against, and there is currently nothing below the engines that reports one. what: - Add scripts/bench/current_api.py, timing topology construction, the classic hierarchy read, and per-request dispatch through the seam - Count requests separately from the tmux commands they carry, so a command group reads as two commands in one dispatch rather than as a saving; a change that merely moves cost between them stays visible as movement - Report the sessions a shape built apart from the sessions the server holds, since the isolation keepalive is one of the latter and not one of the former - Drive it through the shared primitives, so this benchmark and the engine benchmark above it measure with one ruler
…size why: test_new_session_shell_env forwarded dict(os.environ) to new-session, which becomes one -e KEY=VAL argument per variable. tmux sums every argument and refuses past MAX_IMSGSIZE, so the test's outcome was a function of whoever ran it: green on a lean CI runner, "command too long" under a rich interactive shell, where the environment alone can exceed 16 KiB before libtmux appends its format string. The test asserts nothing about the environment's contents, so the size was incidental. what: - Pass a small explicit environment, so the assertion is about what the test claims: window_command survives alongside environment= - Cover the ceiling deliberately instead, with a payload built to exceed it and an assertion that the refusal is loud and creates no session
`--cov=./` measures every Python file in the tree, so the benchmark and operations scripts under `scripts/` counted toward the library's coverage figure. They are development tooling: their uncovered branches are argument parsing, teardown paths, and failure reporting that exist to be read, not to be exercised by the suite. Counting them made the reported number a statement about the tooling's exercise rate rather than the library's. The two patterns mirror the `tests/` entries directly above them, covering `scripts/*.py` and one level of package beneath it, which is as deep as the tree goes. Verified by running one suite under both configurations: `scripts/bench/primitives.py` contributes 70 statements without the omit and disappears with it, taking the measured total from 3039 to 2969.
The benchmark declared `dependencies = ["libtmux"]` with no `[tool.uv.sources]` entry, so running it the way its shebang says to -- `uv run --script scripts/bench/current_api.py` -- installed the released libtmux into the ephemeral environment instead of this working tree. Since it imports `libtmux.engines`, which no release carries, the documented invocation ended at `ModuleNotFoundError` rather than at a wrong number. A script that did import cleanly would have been worse: it would have reported the index's timings as the working tree's. Its existing tests could not see this. They load the module through `spec_from_file_location` into the development environment, where `import libtmux` succeeds regardless of what the inline block says, so the block was never exercised by anything. `tests/test_script_metadata.py` exercises the block directly, across every script under `scripts/` rather than this one. It compares the pin by resolving it against the script's own directory, so a script that moves between directories fails unless its `..` count follows -- the failure mode a rename invites. A scan-found-nothing guard keeps the parametrized checks from reporting green on an empty set. Verified: the documented invocation now builds and reports; removing the pin and mis-spelling its depth each fail with the specific reason.
…nchmark `new_server` promised a "fresh isolated server" and gave it a unique socket, which isolates it from other servers and from nothing else. tmux reads `~/.tmux.conf` when a server starts, so every measurement quietly carried whatever the machine set -- a `history-limit`, a hook, a slow `default-shell` -- and two machines could disagree about libtmux's cost for reasons that had nothing to do with libtmux. Demonstrated with a `HOME` holding a config that sets `history-limit 4242`: before, the benchmark's server reported 4242; after, it does not. That is the test, and it fails without the fix. `config_file=os.devnull` is the same spelling the engine grid arrived at independently for its own copy of this function, so the two now differ only in whether the module globals they read carry a leading underscore.
… directory `reap_stale_scratch` decided a scratch directory was abandoned by looking for a tmux running out of it. A directory exists from the moment a run imports this module, but its first server appears only at the first `new_server`, so during that window every live run looked abandoned and was deleted -- socket directory and all -- by any other run that happened to reap. Under `pytest -n auto` the workers import at different moments, so the window is always open somewhere. The failure surfaced far from its cause, as `new-session: error creating /tmp/ltbench-XXXX/YYYY.sock (No such file or directory)` in a worker whose directory had been removed underneath it, and as `assert 'keepalive' in []` where the list accessor's empty-on-error contract turned the missing socket into a silent empty list rather than a raised error. Liveness is now the owning process, recorded in `owner.pid` when the directory is created and checked with `os.kill(pid, 0)`. `PermissionError` counts as alive: another user's process is running, merely not ours to signal. A directory naming no owner -- one from before this file recorded them, or one caught between creation and its claim -- is judged by age instead, and only then does the tmux probe run. Every unknown resolves toward keeping the directory, so the failure mode stays a leak rather than a theft. The test that claimed to pin this only ever checked the reaper's own directory, which is skipped by identity, so it could not have failed for the reason it named; it is renamed to say what it actually pins. The four cases that needed a rule are covered directly, and removing the owner check turns exactly the two sparing tests red while the two reaping tests stay green.
`tests/` root is the library's namespace -- `tests/test_server.py` covers
`src/libtmux/server.py`, `tests/experimental/engines/` covers
`src/libtmux/experimental/engines/` -- and a top-level directory names the
repository tree it tests, which is what `tests/docs/` already does for `docs/`.
Tests for `scripts/` were sitting in the library's half of that namespace under
a `test_bench_`/`test_script_` prefix, so the prefix was doing a directory's
job and claiming the wrong tree while doing it.
tests/test_bench_primitives.py -> tests/scripts/bench/test_primitives.py
tests/test_bench_current_api.py -> tests/scripts/bench/test_current_api.py
tests/test_script_metadata.py -> tests/scripts/test_metadata.py
Each of these finds the repository root by counting directories up from its own
file, so moving them deeper silently pointed every lookup at the wrong place --
the same counting mistake as a PEP 723 `path = ".."` that no longer reaches the
root. The counts move with the files.
`tests/*/test_*.py` reaches one directory below `tests/`, and `*` does not cross a separator, so every test file nested deeper was measured as though it were library code. Sixty-eight already were. Because those files execute, they report near-total coverage and lift the figure rather than lowering it: the suite reports 84% with them counted and 78% without, so 5,530 statements of test code were worth six points of the library's score. `**` matches any number of nested directories, so one pattern replaces the two that enumerated levels, and a new directory cannot silently fall outside it. The same collapse applies to the `scripts/` entries added alongside them, which had the same one-level ceiling. This is the third place in this tree where a path's depth was written down and then stopped matching when a file moved -- the others being a PEP 723 `path = ".."` and the `parents[1]` lookups in the tests themselves. Each failed silently rather than raising. The reported figure drops six points because it stops averaging the library with its own test suite.
why: Operationalizes the typed-operations/engines architecture
(issues 688, 689) with the pure substrate that was absent from every
prototype branch: an inert, statically-typed operation value that
renders tmux commands, carries its result type, and serializes without
a live tmux server. Engines stay transport-agnostic over it. None of
this touches or changes existing public APIs.
what:
- Add libtmux.experimental.{ops,engines} packages (experimental, not
under the versioning policy)
- ops: frozen Operation[ResultT] with class-level metadata as the
single source of truth; pure render() with declarative version gating
(LooseVersion); build_result() adapting raw output to typed results
- ops: typed Result base + raise_for_status() (CPython/requests
precedent), SplitWindowResult/CapturePaneResult payloads
- ops: closed Target sum (PaneId/WindowId/SessionId/ClientName/NameRef/
IndexRef/Special/SlotRef) with fail-closed validation
- ops: fail-closed OperationRegistry keyed by kind, with OpSpec views
and predicate listing; stdlib dict serialization with round-trips
- ops: four seed operations (split-window, capture-pane, send-keys,
select-layout) registered via @register
- engines: TmuxEngine/AsyncTmuxEngine protocols, CommandRequest/
CommandResult, EngineSpec; run()/arun() execute bridge sharing one
render/build path (sync vs await is the only divergence)
- tests: 111 pure, fixture-parametrizable unit tests + doctests, all
runnable without a tmux server
why: Proves the operation/result contract is transport-agnostic -- the same typed result whether produced by a real tmux subprocess or an in-memory simulator -- and provides the offline engine that lets ops doctests and tests run without a tmux server (issue 689 phases 2-3). what: - engines.subprocess: classic SubprocessEngine mirroring tmux_cmd (has-session stderr fold, backslashreplace, trailing-blank strip; tmux failure returned as data, only missing binary raises), with for_server() deriving -L/-S/-f/-2 flags from a live Server - engines.concrete: deterministic in-memory engine (fabricated pane/ window/session ids, canned capture lines) for tests and docs - engines.registry: name-keyed engine registry (register/create/ available), seeded with subprocess + concrete - tests/experimental/contract: engine-agnostic operation contract run offline via concrete, plus classic-vs-concrete parity against a real tmux server (same result type + argv, payload may differ)
why: Completes the sync/async-symmetric execution story plus the deferred-execution and documentation mechanisms from issue 689 (phase 5 + docs), still without touching any existing API. what: - engines.asyncio: real AsyncSubprocessEngine on create_subprocess_exec (terminates the child on cancellation; not a thread wrapper), mirroring the classic engine's output handling so it returns the same typed result - ops.plan: LazyPlan records operations without touching tmux and resolves SlotRef forward refs at execute time via a sans-I/O generator; sync execute() and async aexecute() share one resolution core (run vs await arun is the only divergence); whole-plan serialization round-trips - ops.catalog: registry-driven CatalogEntry list (scope, version gates, effects, safety, result type, summary) -- the single source a docs domain renders, so runtime and docs cannot drift - tests: lazy resolution sync+async, plan serialization, catalog coverage, async-vs-sync classic parity against a real tmux server
why: Proves control mode is just another engine returning the same typed result (issue 689 phase 4) -- an operation run over a persistent tmux -C connection is indistinguishable, at the result level, from one run via fork-per-call subprocess. what: - engines.control_mode: ControlModeEngine over one persistent tmux -C connection; run_batch pipelines commands and parses each command's %begin/%end/%error block into a CommandResult; selectors-based nonblocking reads with timeout; startup-ACK discard; lifecycle via close()/context manager (lock-guarded teardown) - engines.control_mode: I/O-free ControlModeParser, unit-testable without tmux, adapted from the chain runner + protocol-engines parser - register control_mode in the engine registry and export it - tests: pure parser tests + real-tmux contract (split creates a real pane, batched commands, control-vs-concrete parity)
why: Demonstrates the "mode lives in the type" model from issue 689 -- EagerPane.split() returns a live EagerPane while LazyPane.split() returns a deferred LazyPane, each a single statically-known return type, both backed by the same SplitWindow operation. One Pane class with a runtime-bound engine could not type these return values distinctly. what: - facade.pane.EagerPane: executes immediately, returns live handles (split -> EagerPane), typed results for capture/send_keys - facade.pane.LazyPane: records into a LazyPlan, returns deferred handles (split -> LazyPane bound to the new pane's SlotRef), chainable - seed of the wider Server/Session/Window/Pane/Client x mode matrix - tests: eager live handles, lazy deferral + forward-ref resolution, and same-operation-backs-both-facades parity
why: Closes the two async gaps from issue 689: control mode and concrete had no async sibling. The async control engine is the one async engine that earns its place -- it adds an event stream subprocess cannot -- and prior libtmux/mux control-mode work (surfaced across agent histories via agentgrep, plus the asyncio-2 branches) shaped its correlation design. what: - engines.async_control_mode: AsyncControlModeEngine over a persistent tmux -C (create_subprocess_exec + one reader task). FIFO future correlation with skip-when-empty so unsolicited %begin blocks (hook- triggered commands and the startup ACK) never desync results; the startup ACK is consumed synchronously in start() to close the correlation race our whole-block parser would otherwise have. DEAD state fails pending commands on reader EOF/error. Cancellation via asyncio.wait_for (3.10 floor: no asyncio.timeout/TaskGroup). Bounded subscribe() notification stream with drop-counting. for_server() helper - engines.control_mode: ControlModeParser now surfaces bare %-notification lines via notifications() (additive; the sync engine ignores them) - engines.concrete: AsyncConcreteEngine sibling over shared simulation; removes the async test shim - ControlNotification typed event value - tests: parser notification/drain; async control vs real tmux (split, pipelined batch, concrete parity, live event stream, lifecycle)
why: Many tmux commands print nothing (rename-window, kill-pane, select-window, ...). tmux returns CMD_RETURN_NORMAL on success or calls cmdq_error on failure, framed in control mode as %end vs %error (see tmux cmd-queue.c) -- they never cmdq_print. They still need a typed result that records success/failure without inventing a payload. what: - results.AckResult: a typed acknowledgement (no payload) whose raise_for_status() still surfaces the error path; documents the tmux success/error mapping - retarget send-keys and select-layout to AckResult (both print nothing) - add no-output ops: rename-window (mutating), kill-window and kill-pane (destructive) -- exercising AckResult across scopes and safety tiers - export AckResult and the new ops; refresh the catalog doctest - tests: render + AckResult success/failure across the no-output ops and destructive safety metadata; update classic/control parity assertions
why: A neo-like read model is useful, but neo.Obj is one flat ~200-field class fused to the query/dispatch pipeline. The experimental namespace lets us try a decoupled, immutable, serializable snapshot layer without any risk to the shipped ORM APIs. what: - libtmux.experimental.models: frozen PaneSnapshot / WindowSnapshot / SessionSnapshot / ServerSnapshot, each a typed core plus the full raw tmux-format tail in .fields (nothing tmux reported is lost) - from_format() builds one node from a format mapping; ServerSnapshot.from_pane_rows() groups a flat "list-panes -a -F" row set into an ordered session/window/pane tree - to_dict()/from_dict() round-trip the whole tree as plain data, with no live objects - pure tests (no tmux): value coercion, tree grouping/order, round-trip
why: The list/show read commands overlap neo's reader. Rather than touch the ORM, add a parallel typed read surface in experimental.ops that yields immutable models snapshots. The render version must thread into result parsing first, because the -F template is version-gated and the parser must split against the same fields it was rendered with. what: - operation: thread `version` through build_result -> _make_result so payload parsing matches the version-gated render (backward compatible; existing overrides accept and ignore it); execute.run/arun pass it - ops._read: re-export neo.get_output_format / parse_output and formats.FORMAT_SEPARATOR as the single source of truth (no copies) - list-panes / list-windows / list-sessions ops (readonly, chainable=False) render the same -F template neo builds and parse rows into models snapshots - ListPanesResult/.../ store JSON-friendly rows and derive typed views (.panes/.server/.windows/.sessions) via properties, so results serialize and round-trip with no special-casing - tests: -F parity with neo, snapshot-tree build, serialize round-trip, and live list-panes/sessions/windows against a real tmux server
why: The operation catalog is registry-derived data, so rendering it in docs keeps the operation reference from drifting from the code -- and the docs gate then exercises catalog() on every build. what: - docs/_ext/tmuxop.py: an in-repo Sphinx directive `tmuxop-catalog` that walks libtmux.experimental.ops.catalog() and emits a table, with :scope:/:safety:/:primitive-only: filters; warns (not raises) on empty - conf.py: add docs/_ext to sys.path and 'tmuxop' to extra_extensions - docs/experimental.md: an experimental ops/engines overview embedding the catalog (full + readonly + destructive views), in the index toctree
why: The sync control engine skipped tmux's startup ACK with a fragile one-shot flags==0 heuristic and had no defense against hook-emitted %begin/%end blocks, so a stray block could desync request->result alignment. The async engine already handles this; backport the approach. what: - consume the startup ACK synchronously at connect (_consume_startup), dropping the one-shot _startup_ack_pending heuristic, so the startup block can never be conflated with a command's result block - drain buffered unsolicited blocks before each batch (_drain_unsolicited), so a hook-triggered command's block left over from a prior call is not mis-attributed to the next command - drain notifications during reads to keep the parser buffer bounded - regression test: many sequential commands stay aligned (first result is real; each call drains before reading its own block) A hook firing mid-pipelined-batch still needs per-command number correlation to disambiguate; single-command run() is robust.
why: The chainable-commands prototype folds independent commands into one "tmux a ; b" dispatch. Our typed-op model is a better host for it -- the Operation already carries a `chainable` classvar and the result Status already reserves `skipped` for exactly the chain-drop case. So yes, lazy mode can adopt the prototype's chainability. what: - mark output/creation ops non-chainable (capture-pane, split-window; list-* already were) so a fold never drops captured data or an id - ops._chain: render_chain (join chainable ops with standalone ';', escaping a trailing-';' arg), ensure_chainable (fail closed), and attribute -- splitting one merged ';'-chain result into a typed result per op (success -> all complete; failure -> first failed, rest skipped, matching tmux cmd-queue.c cmdq_remove_group); plus OpChain with >>/then - Operation.__rshift__/then compose into an OpChain; result_with_status() builds a result with an explicit status (skipped/failed attribution) - LazyPlan.execute/aexecute gain fold=False (opt-in): maximal runs of chainable, resolved ops dispatch once via engine.run; the sans-I/O _drive yields _Single or _Chain so sync and async share the core; add_chain() records an OpChain - tests: >> composition, render_chain, fold=one dispatch, fold-off=N dispatches, failure attribution, creators stay unfolded, add_chain
why: Extend the mode-in-the-type facades beyond the pane seed so a typed return value distinguishes eager/lazy/async across scopes -- and add the few creation ops the cross-scope navigation needs. what: - ops: NewWindow / NewSession (CreateResult, capture the new id), KillSession, RenameSession; generalize binding capture via Result.created_id (base None; SplitWindowResult -> new_pane_id; CreateResult -> new_id) so lazy plans bind window/session creations too - facade: eager Server -> Session -> Window -> Pane navigation (EagerServer/EagerSession/EagerWindow); LazyWindow (records into a plan); AsyncPane / AsyncWindow (await arun) -- all over the same ops. Control mode stays an engine choice, not a separate facade family - EagerServer.for_server() binds the classic engine to a live Server - tests: offline navigation across scopes/modes (concrete engine), and a live eager Server -> Session -> Window -> Pane build against real tmux with cleanup
why: The native binary peer-protocol engine is the strongest proof the
operation/result contract is transport-agnostic -- the same typed
CommandResult whether produced by a subprocess, tmux -C, or by speaking
tmux's imsg protocol directly. Research confirmed it is pure-stdlib and
CI-verifiable; the prototype it is ported from only ever tested against a
fake socketpair server, never real tmux.
what:
- port engines/imsg/{types,v8,base}.py from libtmux-protocol-engines:
ImsgEngine over AF_UNIX + sendmsg/recvmsg + SCM_RIGHTS fd-passing, and
ProtocolV8Codec (=IIII header, IMSG_FD_MARK high bit of len,
peerid=PROTOCOL_VERSION 8, IDENTIFY -> COMMAND -> WRITE_* -> EXIT
handshake); posix_spawn local fallback for attach / start-server /
no-server-running
- adapt to the experimental tuple CommandResult (drop the process field);
add imsg.exc (ImsgError / ImsgProtocolError / UnsupportedProtocolVersion)
and select the v8 codec directly; keep the version-mismatch retry
- register as the opt-in "imsg" engine; import-safe everywhere (AF_UNIX
is only touched at runtime; tests skip without it)
- tests: v8 codec round-trip + MSG_COMMAND framing (no tmux), plus the
live parity test the prototype lacked -- ImsgEngine vs SubprocessEngine
return identical stdout/returncode for read-only commands against a
real tmux server (runs across the CI tmux matrix)
why: Deleting a control-owned session can race tmux's deferred global notifications and terminate supported tmux servers. what: - Attach control clients to safe existing sessions without updating the environment - Bootstrap empty or unsafe servers through tracked subprocess execution - Serialize lifecycle transitions without blocking dependent fallback commands - Cover live hook, concurrency, cancellation, and cross-version paths
why: CI type-checks against Python 3.10, where Task.cancelling is not available in the asyncio stubs. what: - Replace the version-specific cancellation probe with a portable task wait - Make the shared startup future's nullable type explicit - Align lifecycle test overrides and terminal paths with their contracts
why: Whole-file backups could be overwritten or unwound out of order, and malformed state could abort configuration recovery. what: - Preserve first backups and enforce per-config LIFO ordering - Checkpoint state before config writes and each successful revert - Fail closed on missing backups, corrupt state, and malformed configs - Cover repeat swaps, partial failures, and recovery diagnostics
why: Experimental operations, engines, and agent tools could diverge from their documented constructor, result, and command-boundary contracts. what: - Make Attributes the checked source for generated constructor API prose - Expose only tmux-supported targets across Python, MCP, and payloads - Preserve create results and tmux 3.7 composed-operation invariants - Gate executable MCP payloads across every executable tool path - Preserve literal arguments across every experimental tmux transport - Add the direct YAML dependency and tested failure guidance
why: Show how typed Python chains become folded tmux command sequences while one asynchronous control-mode client carries the work. what: - Add a tabbed live tutorial for forward references and planner folding - Replace simulated plan examples with real tmux execution - Test visible compiled commands against actual control-mode dispatches - Add API destinations and navigation for plan concepts
why: First-party record fields must render with complete semantic prose, while gp-sphinx a36 now owns documented-member deduplication. what: - Expand the runtime contract across supported record declarations - Define inheritance, ordering, and exemption rules in AGENTS.md - Remove the obsolete no-undoc-members workaround and source test
why: CI must type-check the untyped doctest dependency boundary and the heterogeneous results produced by a compiled operation plan. what: - Describe the consumed doctest finder interface with a protocol - Narrow the terminal plan result before reading message text
why: Operation's ten class variables were documented as a hand-written definition list under Notes, because an Attributes entry for a class variable used to be dropped from the build. gp-sphinx renders one now, with the annotation and value alongside the prose, so the workaround costs a reader the type and default it cannot state. what: - Move kind, command, scope, result_cls, chainable, primitive, safety, effects, min_version, and flag_version_map into the Attributes section, dropping the Notes list Requires a gp-sphinx release carrying the class-variable rendering; under the pinned 0.0.1a36 these descriptions do not render.
why: Engine users need transport-specific examples that expose real output and boundaries without treating mock results as server evidence. what: - Add one executable first-success example per concrete engine - Map every engine to a focused live or offline tutorial - Test tutorial ownership and transport boundaries
why: gp-sphinx removes tabs.js after rendering, leaving pages with a missing asset reference even though inline tabs operate through CSS. what: - Filter only tabs.js from Sphinx page contexts at late priority - Verify final tab markup and assets with a one-page Sphinx build
why: ruff's default rule set, adopted on master, enforces PLR0402. `import a.b as b` and `from a import b` bind the same name, and the from-form is the one the rest of the suite uses. what: - Rewrite the engines and ops submodule imports in the docs tests https://docs.astral.sh/ruff/rules/manual-from-import/
why: ruff's default rule set enforces FURB188. The conditional slice and `str.removeprefix` are equivalent for a single-character prefix, and the method says what the code is doing. what: - Replace the guarded slice in `ControlNotification.parse` https://docs.astral.sh/ruff/rules/slice-to-remove-prefix-or-suffix/
why: ruff's default rule set enforces PYI025. Bare `Set` reads as the `set` builtin at the use site, but it is the abstract collection; the annotation accepts any set-like, not just `set`. what: - Import `collections.abc.Set` as `AbstractSet` and use it in `get_objects`' docnames annotation https://docs.astral.sh/ruff/rules/unaliased-collections-abc-set-import/
why: ruff's default rule set enforces FLY002, which wants an f-string in
place of a static join. An f-string is the wrong shape here: tmux format
specifiers are `#{...}`, so every brace would need doubling, and the
eight fields would collapse onto one line. Binding the tuple keeps one
field per line and leaves the join non-static.
what:
- Extract the tmux format specifiers into `_DONE_FIELDS` and join that
`_DONE_FORMAT` is unchanged.
https://docs.astral.sh/ruff/rules/static-join-to-f-string/
why: ruff's default rule set enforces ISC004. Implicit concatenation inside a list literal reads like a missing comma between elements, which is how a segment silently merges into its neighbour. Explicit parens make each element's extent unambiguous. what: - Wrap the four multi-line instruction segments in parentheses The rendered instructions are byte-identical, with and without events. https://docs.astral.sh/ruff/rules/implicit-string-concatenation-in-collection-literal/
why: ruff's default rule set enforces PYI034. Annotating `__enter__`, `__aenter__`, and `__new__` with the concrete class loses the subclass: `with SubEngine() as e` inferred the base, so subclass-only attributes read as errors and the wrong type propagated to callers. what: - Return `Self` from `ControlModeEngine.__enter__`, `AsyncControlModeEngine.__aenter__`, and `CommandSeparator.__new__` - Same for the forged separator in the engine base tests - Import `Self` from `typing_extensions` under `TYPE_CHECKING`, matching the rest of the package's 3.10 backport pattern https://docs.astral.sh/ruff/rules/non-self-return-type/
why: ruff's default rule set enforces FURB192. Sorting a whole name set to read its first element states the intent less directly than asking for the minimum. what: - Use `min()` for the two suggested-server picks in the doctor output Ruff marks the fix unsafe because `sorted(x)[0]` raises `IndexError` on an empty sequence where `min(x)` raises `ValueError`. Both call sites sit behind an emptiness guard on the line above, so neither can be reached with an empty set. https://docs.astral.sh/ruff/rules/sorted-min-max/
why: ruff's default rule set enables BLE001, which fires at nine sites where catching everything is the contract rather than a mistake. Each handler records the failure for a caller instead of swallowing it, so narrowing the except clause would lose the failure mode it exists to report. what: - Scope per-file ignores, with the reason, to the async control-mode supervisor, the MCP event drainer, the error-result middleware, the schema fallbacks, and the safety-gate tier assertions The supervisor and reader tasks re-raise `CancelledError` before the catch-all, so cancellation still propagates. https://docs.astral.sh/ruff/rules/blind-except/
why: ruff's EXE001 fires on CI but never locally: it short-circuits on WSL, where every file reports executable, so a Linux runner sees a shebang on a mode-644 file that this machine cannot. Both scripts also declared PEP 723 dependencies their `python3` shebang could not satisfy, so `./scripts/mcp_swap.py` died on a missing import. what: - Set mode 100755 on `scripts/bench_engines.py` and `scripts/mcp_swap.py` - Point both shebangs at `uv run --script`, which resolves the inline dependency block `uv run scripts/<name>.py`, the form the docs use, is unaffected. https://docs.astral.sh/ruff/rules/shebang-not-executable/
why: `use-local` could only point the agents at this working copy, so reviewing a branch meant checking it out first. The writer was also careless with files it does not own: it replaced a symlinked config with a regular file, dropped its permission bits, re-escaped non-ASCII text it never read, appended a newline Claude never wrote, and recorded `config_path` as recovery identity — so a link repointed after a swap sent `revert` into someone else's file. what: - Add `use-local --pr N`, writing `uvx --from git+<remote>@refs/pull/N/head <entry>`; the ref lives on the base repo so fork PRs work unchanged and nothing is checked out for `revert` to clean up - Probe that spec with one MCP `initialize` round trip before any config is touched, so a bad ref fails once here instead of inside every agent; `--no-preflight` skips it - Resolve symlinks and carry the target's mode through `atomic_write`, and record the resolved `SwapEntry.target_path` for `revert` to use - Take the file's own trailing-newline convention in `dump_config_bytes` and stop escaping characters outside the entry being swapped - Compare argv exactly in `_points_at`, so `--entry` is not swallowed by the already-local short-circuit, and label a PR before the pin branch - Cover pull-request targeting, JSON writer fidelity and symlinked configs, and guard that `fake_home` lists every registered CLI The `libtmux-engine-mcp-dev` state slug, the entry-derived server name, and the recovery-stack guards this repo added stay as they were.
why: Two more agent CLIs are installed here and neither could be swapped. Both keep their MCP config in JSONC, which the JSON writer would have reserialized -- stripping every comment out of a file the user wrote by hand. opencode also disagrees with the other six about how one entry is spelled: its container key is `mcp`, argv goes into a single `command` array, and the environment table is called `environment` (an `env` key is dropped in silence, and a scalar `command` is a decode error that stops opencode starting at all). what: - Add a JSONC codec that edits by text splice, so comments, trailing commas, indent width and a missing final newline all survive a swap; values still come from stdlib `json`, so escape semantics are the standard library's - Move the per-CLI branching onto `CLIInfo.container` and `CLIInfo.dialect`, so `get_server`, `set_server`, `delete_server` and `_all_server_specs` no longer carry a chain of CLI-name tests - Register `opencode` ($XDG_CONFIG_HOME/opencode/opencode.jsonc) and `pi` (~/.pi/agent/mcp.json), normalising opencode's array command back to the portable scalar-plus-args spec so the already-local check still fires - Ignore a relative XDG_CONFIG_HOME, which the spec requires and which otherwise made the recorded backup path depend on the working directory - Say in `detect` that pi has no MCP client of its own: the file is read by the third-party pi-mcp-adapter, so without it the swap does nothing - Derive the detect column width from the longest CLI name - Cover both CLIs end to end plus JSONC fidelity, the one-delimiter member removal, key escaping, and comment survival on insert
why: engine-ops replays onto engine-seam-minimal without touching a line of core dispatch, so the only reconciliation left is shared bookkeeping. what: - Merge both changelog entries under one unreleased release block - Regenerate uv.lock against master's gp-sphinx floor
why: The experimental engines carried their own CommandRequest, CommandResult and ServerConnection, so an engine written against them returned values Core's object API could not read, and the two copies could drift apart on the flags they emit. what: - Import the request, result, separator and engine protocols from libtmux.engines instead of redefining them - Reduce the connection module to Core's ServerConnection - Keep what only the experimental transports need: the argv encoders, control-mode rendering, EngineSpec and the async protocol
`scripts/bench/` holds the benchmarks and the primitives they share; the engine grid sat one directory above them under the older `bench_*.py` spelling. Two conventions for one kind of file makes the directory a worse index of itself, and the older one only had to survive on this branch to keep saying the convention was optional. The move lands where the file is introduced rather than where it stops changing, so no branch in the stack ever shows the old spelling. The commits above that rewrite this file replay onto the new path; only the usage block in the module docstring is touched by both, which is a single region to reconcile. The PEP 723 pin reaches the repository root at `../..` from one directory deeper, and `tests/scripts/test_metadata.py` checks that by resolving it rather than by matching text, so the wrong depth cannot land. `RESULTS.md` and the benchmarking skill name the path in prose. One usage line outgrew the line limit and is shortened rather than wrapped, since it is a command to paste. Verified: the grid runs through its own shebang from the new location and `contract` reports parity with exit 0.
`scripts/mcp_swap.py` is a development script, so its test belongs beside the other script tests rather than in the library's half of the `tests/` namespace. Its repository-root lookup counts directories, so the count moves with it.
The grid decided a scratch directory was abandoned by looking for a tmux running out of it. A directory exists from the moment a run starts, but its first server only from the first `new_server`, so during that window a live run looks abandoned. Running the grid deleted the socket directory of any benchmark running beside it, mid-run. `scripts/bench/primitives.py` already records an owner and reads it. That protects nothing against a reaper which does not look, and both sweep the same `ltbench-*` namespace, so this copy read the owner file too -- the same filename, so the two spare each other rather than each protecting only itself. The duplication is the real defect and the branch above removes it, folding this function into the shared one. Until then this copy has to be correct on its own, because a stacked branch is still a branch someone can run. Verified against a directory whose `owner.pid` names a live process: it survived a run of the grid, where before it did not, and a directory naming no owner and older than the grace period was still reaped.
why: tmux 3.2a through 3.5 can move a one-pane window successfully without returning the requested format, leaving break_pane to index an empty result. Silent nonzero results were accepted too. what: - Reject nonzero break-pane results - Refresh the moved pane when tmux returns no destination - Fail with a post-mutation warning if identity cannot be recovered
# Conflicts: # uv.lock
Why: send_keys can return before the shell renders its next prompt, making the end-boundary assertion race even in isolation. What: wait for the complete captured command and prompt before asserting the end selector results.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #754 +/- ##
===========================================
+ Coverage 52.45% 73.23% +20.78%
===========================================
Files 26 171 +145
Lines 3729 11478 +7749
Branches 747 1824 +1077
===========================================
+ Hits 1956 8406 +6450
- Misses 1469 2417 +948
- Partials 304 655 +351 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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
TmuxEngineandServerConnectionwhile leaving the default subprocess path unchanged.Review note
This master-targeted proposal combines the execution seam currently reviewed in #739 with its dependent operations branch in #742. The
improvements-00head is currently identical to #742.Compatibility
Server.raise_if_dead()captures tmux's message on the raised exception instead of echoing it.tmux_cmd.processbecomes read-only and reports when the selected engine owns no process.Test plan
git diff --check.masterbranch.