Make tmux and MCP transports owned and bounded - #9
Open
tony wants to merge 182 commits into
Open
Conversation
why: jobs.find releases the collection mutex while returning a mutable job pointer. A concurrent settlement then writes fields another get_job call may already be reading. what: - Add the race-detector reproduction as a temporary known failure
why: jobs.find released the collection mutex while returning mutable state, so concurrent settlement raced with get_job reads. Retaining caller-owned output also left state vulnerable to later mutation. what: - Return an owned job value and clone retained output at both boundaries - Enable the race-detector regression
why: WithServer accepted any configured handle even though tmux record IDs are server-local. A record could therefore target an unrelated object whose server reused the same ID. what: - Replace public record rebinding with record-level engine selection - Carry the selected engine through materialized relations - Update the control examples, warnings, tests, and design guidance
why: the caller-pane guidance described an old fail-open policy, and operator documentation named a stale recipe variable, version, and shutdown diagnostic. Those claims disagreed with the shipped server. what: - Describe fail-closed caller-pane elicitation consistently - Correct the recipe variable, version example, and signal wording - Gate operational names against exported runtime values
why: every control notification was appended to a temporary file for the client lifetime, and command-only pools retained one unusable copy per connection. Active sessions could therefore grow disk usage without bound. what: - Replace the disk spool with a 16 MiB ordered in-memory queue - Drain the exact queued prefix before reporting explicit overflow - Keep command replies live after notification overflow - Attach pooled clients with no-output and no notification queue
why: NewServer discarded the owner of detached-job directories, watcher connections and timers, and audit files. Embedded callers could not release those resources without ending their process. what: - Return an Instance that embeds the SDK server and closes owned resources - Stop and join watchers and cancel deferred notifications - Close file-backed audit logs while leaving caller engines caller-owned - Update embedding examples and internal consumers
why: inserting a missing server after a JSONC member with a trailing comma added another separator. The resulting double comma made the client config unreadable. what: - Reuse the blanked source while locating the insertion point - Add a separator only when the final existing member has none - Cover insertion into a trailing-comma object
why: the shallow environment reader stripped only double quotes, then re-quoted the result. Literal strings, escapes, and inline comments changed meaning during a swap. what: - Retain existing environment values as raw TOML tokens - Render only newly supplied values as basic strings - Decode string tokens only on the read-only status path - Cover literal, escaped, and commented environment values
why: restoring the entire backup discarded unrelated configuration edits made after a swap. what: - restore or remove only the tmux server entry - retain backups when ownership checks fail - cover existing, absent, and manually changed entries
why: an unreadable backup path was treated as an existing backup, allowing the live config to change without a usable recovery copy. what: - stop writes when backup inspection fails - report config and backup inspection failures in dry runs - cover symlink-loop failures without touching the config
why: truncating live agent configuration files in place could leave partial contents if a write was interrupted. what: - write backups and configs through synced sibling files - preserve modes and symlinked dotfile layouts - reject non-regular backup paths
why: the subprocess stderr copier could append while preflight read its diagnostic buffer after stdout closed. what: - guard stderr writes and snapshots with a mutex - clone diagnostic text before releasing the lock - reproduce the overlap with a real subprocess under race
why: automatic subprocess fallback could silently violate a caller transport or deployment boundary. what: - add an opt-in rejecting engine fallback policy - return typed errors without starting tmux processes - cover unsupported kinds, exact-byte reads, and reset behavior
why: tmux display formats can execute shell commands through #(), so advertising arbitrary expansion as read-only crossed the stated boundary. what: - withhold display_message at the readonly level - lead tool and generated docs with the shell risk - cover registration and annotation accounting
why: readonly, mutating, and destructive tiers classify tmux operations but cannot independently bound content disclosure, pane control, workspace creation, settings, layout, and destruction. what: - default to metadata-only access with inspect, operate, and all profiles - gate tools, batches, resources, prompts, subscriptions, and completions - publish capability metadata and report active and rejected grants - verify the complete 58-tool capability matrix
why: session names can carry sensitive project or customer text and did not belong in the audit cleartext allowlist. what: - hash sessionName values like other payloads - retain stable tmux ids and closed flags in cleartext - cover the distinction explicitly
why: Ubuntu-only tests miss native PTY and socket behavior, while the existing fuzz targets previously ran only as fixed regression seeds. what: - run every module against Homebrew tmux on macOS - fuzz option and format decoders for bounded intervals - fail if a fuzz target rename would silently disable the gate
why: An interactive shell could redraw its prompt after kill-session -C, while the whole-second window_activity format made the test believe the window was already quiet. what: - gate one pane write until activity monitoring is enabled - leave the pane on a silent long-lived process - wait for the activity flag instead of a lossy timestamp
why: The writing policy repeated its own guidance and made the comment quality bar harder to scan. what: - Reduce the policy to direct, enforceable rules - Preserve caller contracts, directives, examples, and commit conventions
why: Package overviews buried cross-cutting contracts inside feature tours that duplicated declaration documentation. what: - Replace the core overview with task-oriented entry points - Preserve record, identity, transport, plan, and failure contracts - Tighten the MCP overview and correct caller-pane elicitation behavior
why: Every generated accessor repeated snapshot and missing-value rules, obscuring the field-specific token, type, scope, and version. what: - Put shared materialization semantics on FormatValues - Emit one searchable line per generated accessor - Derive accessor inventory checks from the format specification
why: Accessors and setters repeated negative style metadata and raw fallback guidance until the generated API read like boilerplate. what: - Centralize materialization, inheritance, and raw-read guidance - Keep exact option names, value types, variants, scopes, and setter links - Retain sparse replacement and affirmative style contracts
why: Generated JSON and predicate methods repeated schema and sentinel instructions that belong to the filter types and error declaration. what: - Shorten filter, relation, validation, predicate, and JSON documentation - Keep strict decoding, partial receiver state, and relation semantics - Guard against reintroducing per-method schema boilerplate
why: Internal generator comments explained development history and obvious control flow instead of the marker, naming, and inventory constraints. what: - State source-region replacement and marker ordering directly - Condense shared Go-name conversion rules - Preserve the attached-client requirement for format inventory
why: Buffer and wait-channel validators treated one argument containing spaces as several argv elements, although tmux receives it intact. what: - Accept embedded whitespace in buffer and channel names - Keep empty, leading-dash, and backslash protections - Cover buffer round trips and channel signalling against real tmux
why: Example comments repeated fixture setup and narrated code, burying the behavior each example proves. what: - Replace repeated setup prose with isolation and cleanup contracts - Keep notification, capture, forward-reference, and result invariants - Shorten comments in executable examples and their tests
why: Benchmark comments narrated obvious code and buried the conditions that make lane costs comparable. what: - State the shared workload, answer, and display order - Preserve isolation, process-count, and instance-bound invariants - Remove comments that repeat names and control flow
why: Workspace comments repeated control flow and obscured when Build creates resources, pauses, opens a client, and returns partial state. what: - Centralize non-atomic, command-order, and transport behavior on Build - Keep timing, ownership, validation, and failure invariants - Remove setup narration from workspace tests and helpers
why: The package index needs to tell callers where to begin. what: - Rename the task index heading to "Where to start"
why: The plan API lacked a runnable example of inspecting operations before sending them. what: - Add ExampleNewPlan - Show forward references, Preview, and Explain output - Verify the rendered commands and planner grouping
why: The comment explained why the distinction matters, which the call below already shows. what: - Keep the spent-context constraint and drop the justification
why: tmux publishes no pane-close notification, so the evidence this package uses is not obvious from the contract alone. what: - Name the arrangement as the evidence, and an unreadable one as no evidence - Record that a server's own exit reason reaches the loss
why: get_job returns without a commit three ways, and only the leader's deadline named a cause. A zero-timeout poll and a follower whose wait expired both returned Running and OutputUnavailable empty, which is the shape the deadline fix already removed. The poll is the documented way to check a detached job, so it is the path most callers take. what: - Diagnose the zero-timeout poll from the job's marks, which keeps it local - Diagnose a follower whose own wait ended before the collector finished - Share one description between the waiting branches so they cannot drift
why: The pane completion example told the pane to run "tmux", which resolves through the pane's PATH. Where that finds a different release, its client cannot signal this server, the channel is never signalled, and the example waits out its whole budget. On a machine holding several tmux builds it timed out at 60s; it now finishes in about one. what: - Name the server's frozen executable in the command the pane runs
why: tmux ends a control client that falls more than five minutes behind. This package bounded its own queue and reported an overflow, which drops what tmux was willing to hold. tmux has offered per-pane holding since 3.2, and both halves of that model are what asyncio's stream reader implements: bound the buffer, then push back on the producer. what: - Add NotificationOptions.PauseAfter, which arms tmux's per-pane hold - Add NotificationStream.ContinuePane to resume a held pane - Reject a hold below tmux's whole-second resolution, and one asked for without the pane output it governs - Reject a pane identifier tmux's resume parser would discard in silence tmux does not replay what it held, and answers neither a malformed resume nor one for a pane it never held, so both are refused here rather than sent.
why: The two detached jobs gated each other with "tmux wait-for", resolved through the pane's PATH. A pane running the inherited interactive shell can find another release there, whose client answers "server exited unexpectedly" at once instead of waiting, so the second job finished immediately and the test read that as one job's completion ending another's wait. what: - Name the harness server's frozen executable in both gate commands
why: The type is named for one pane and its notifications are not filtered to that pane, so a caller reading the name expects output the stream does not promise. The package's own real fixture has to select by identifier. what: - Record that the stream carries the whole connection, not this pane alone
why: The bookkeeping wrapper was one format string with thirteen positional arguments, six of them the same temporary name repeated in pairs. Transposing any adjacent pair renders a wrapper that still runs and silently publishes the wrong record, and nothing in the call site shows which argument is which. what: - Render each published record from its producer and final path - Derive the temporary name where it is used, rather than at three call sites - Give the wrapper its own function, so the shape reads as the script it is The rendered script is unchanged: an equivalence check against the replaced construction matched byte for byte for plain, quoted, and tab-bearing paths, and failed when two records were transposed.
why: run_command_tools.go held four subjects in 687 lines: the tool, the wrapper's record format, extraction of a command's output from the pane grid, and the shell text the wrapper is made of. Two of them are not the tool's at all -- jobs.go reads the same marks and calls the same extraction -- so the file's name described a third of its contents. what: - Move the record format, its marks, extraction, and the reasons a record is missing into command_records.go - Move the wrapper script and its shell quoting into command_wrapper.go - Leave run_command_tools.go the tool, its delivery, and its waiting No behavior changes; every declaration moved intact.
why: A cursor taken while the pane's cursor sat on the first row carried no leading fingerprint, and its anchor was then relocated by arithmetic alone. tmux renumbers the grid when it trims history, so once the pane overflowed its limit that arithmetic addressed the wrong row and the reply omitted lines while reporting linesMissed false. A gap reported as completeness is the one outcome a caller cannot detect, and a fresh pane is exactly where a caller takes its first cursor. what: - Fingerprint the history above an anchor on the first row, so only a pane that has written nothing reaches the positional path - Refuse that path once history is full, because tmux trims only at the limit and the first row cannot then be told from the oldest row that survived - Read the pane's history limit alongside the rest of its grid state Reproduced against real tmux: 127 of 200 lines were absent with linesMissed false, and the same case now reports the loss.
why: The test gates two detached jobs inside a 15s budget, but its panes ran the inherited interactive shell, whose startup is the largest and most load-sensitive cost in the run. The tmux 3.7b matrix cell spent the whole budget waiting for the first job's closing record; the same test passes in under a second in isolation, ten times over. what: - Give the shared-signal test the harness fixed shell Ten repetitions under 3.7b fall from about 0.75s each to about 0.41s, and stop depending on how long two shells take to start.
why: The guard added with the first-row fix compared history size against the pane's history limit, and tmux does not report those alike: with a limit of 50 a flooded pane settles at 48, so the guard never fired. The tmux 3.6 matrix cell caught it, where the same reply lost two hundred lines and still claimed to be complete. what: - Read from the retained boundary and check the first row against the anchor hash the cursor already carries, which needs no version-specific format - Drop the history limit from the pane state again Checked against tmux 3.2a, 3.6, and 3.7c, and shown to fail on 3.6 without it.
why: The test sends "exit" and waits for the pane to die, but its pane ran the inherited interactive shell, which may still be starting when the keys arrive. The tmux 3.7 matrix cell spent the wait on that; the same test passes five times in a row in isolation. what: - Build the exited-shell test's server with the harness fixed shell Five repetitions fall from about 1.1s each to about 0.14s.
why: Verifying a first-row anchor by one row's hash accepts any blank row, because every blank row hashes alike. A pane whose prompt had not been drawn when its cursor was taken recorded a blank anchor, and a later blank row then proved nothing while reading as proof. The race build surfaced it, where the first read lands earlier. what: - Accept without proof only while nothing has scrolled into history, where the first row cannot have been discarded - Require a non-blank anchor that still reads the same once rows have moved - Take the reproduction's cursor on a fixed-shell pane at its first row, rather than arranging that state with clear Checked under the race build five times over, and on tmux 3.2a, 3.4, 3.6, 3.7c.
why: The macOS temporary root can leave too little room for a tmux Unix socket when the test adds its descriptive directory name. what: - Allocate the real-server fixture directly under the system temp root - Retain explicit cleanup
why: A proof compiled only for another target disappeared from the host index even though the parity audit examines every supported target. what: - Merge proof declarations from each build context - Reject conflicting declarations and cover a Plan 9-only proof
why: macOS resolves temporary directories through a symlink, so expected socket roots used a different spelling from the frozen selection. what: - Canonicalize both ordinary and symlinked fixture roots
why: New platform and fuzz jobs inherited GitHub token permissions instead of declaring their least-privilege boundary. what: - Grant the tests workflow read-only repository contents access
Why: tmux reports physical pane directories on macOS, where /tmp resolves to /private/tmp, so pathUnder missed a caller-provided alias of the same directory. What: Compare resolved path spellings while preserving the sibling-boundary check, with a deterministic symlink regression test.
Why: macOS resolves /tmp as /private/tmp and has a shorter Unix-socket path limit, so fixtures compared aliases and one long t.TempDir socket could not start. What: Assert against each frozen SocketSelection and allocate the discovery lease under a short temporary directory.
Why: The macOS lane could send the key sequence while the workspace shell was still starting, so tmux accepted every key but the pane never displayed them. What: Complete one command round trip before exercising send_keys_batch.
Why: The macOS portability fixture reassigned the sibling path after its only use, so the full linter rejected the test. What: Drop the ineffectual assignment while retaining the canonical discovery-directory assertion.
why: Capturing shell echo can miss typed keys after a redraw even when tmux delivered them. what: - Start a raw key reader and wait for its readiness marker - Assert the exact bytes delivered without appending Enter
why: send_keys_batch launched one tmux client per key even though tmux accepts an ordered sequence, adding process and partial-failure boundaries. what: - Add validated sequence delivery through one tmux command - Route send_keys_batch through the typed sequence API - Cover argv, ownership, invalid input, and live byte delivery
why: A raw stty and dd reader captures only the first input event on macOS, even when tmux sends the complete sequence in one command. what: - Read the batch as one canonical shell line - Include the explicit Enter that the tool contract requires - Bound readiness and output-file waits with the test context
why: maxServers capped only the reply after every socket was allocated and probed, while directory read failures looked like an empty inventory. what: - Apply finite default and ceiling limits before sibling construction - Reserve the configured target and report bounded scan truncation - Propagate directory failures except an absent socket directory - Cover probe bounds and directory errors with regressions
why: shellQuote documentation sat above wrapperScript, so the comment described the wrong declaration. what: - Move a concise POSIX-word invariant to shellQuote
why: reserving the configured target before the bounded scan bypassed the existing name filter. what: - Filter the reserved target by name before admitting it - Count a filtered target as skipped - Cover the target path with an isolated regression
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
2 participants
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
NewServervalidate and freeze execution and socket configuration while binding handles to one tmux daemon.ConnectionAPI for commands, observations, and session-local control.Changes by area
Breaking changes
NewServerreturns(*Server, error).CommandRunner,Engine, andControlPoolgive way to owned connection APIs.Test plan
git diff --check.