feat(client)!: BytesSink + attachBytesSink + PaneScope substrate (tmux-pane-output-i3m.1)#59
Merged
Merged
Conversation
…oses tmux-pane-output-i3m.1
Replaces the old per-pane / all-panes split (`attachPaneSink` /
`attachAllPanesSink`, `PaneByteSink`, `PaneByteMultiplexer`) with a
single scope-based API:
client.attachBytesSink(sink, { scope: sessionScope(id) })
Four scope kinds: server (default), session, window, pane.
**Internal substrate**
- `src/pane-output.ts`: BytesSink interface, PaneScope union + factory
functions, PaneTopologyManager (paneId → {sessionId, windowId}),
SinkRegistry (four scope-bifurcated buckets, snapshot-before-write).
- Topology bootstrap is lazy: fires only when a session/window-scoped
sink is attached, so server/pane consumers pay zero extra cost.
- Topology kept live by window-add / window-close / layout-change /
sessions-changed handlers across all three TmuxClientLike
implementations (TmuxClient, WebSocketTmuxClient, TmuxClientProxy).
**Bug fix included**: `window-add` and `unlinked-window-add` events were
not triggering topology updates. New pane bytes arrived before the
topology table was seeded, causing session/window-scoped sinks to miss
the initial output. Fixed by calling `refreshWindowTopology(windowId)`
on both event types in all client implementations.
**Migrations**
- `src/pane-sink.ts` deleted; all callers migrated to `attachBytesSink`.
- `src/sinks/text-stream.ts` updated to BytesSink (reads `msg.data`).
- All connectors (electron/main, electron/renderer, websocket/*)
migrate `attachAllPanesSink` → `attachBytesSink` (serverScope).
- `FakeTmuxClient` in pane-terminal bench implements `attachBytesSink`
with the same SinkRegistry + PaneTopologyManager.
**Tests**
- `tests/integration/pane-scope.test.ts` (new): 9 integration tests
covering all four scope kinds, dynamic membership (window-add after
subscription), multi-scope dispatch (no duplication), bootstrap
correctness, and the no-consumer fast path.
- All 554 tests pass (29 files).
There was a problem hiding this comment.
Pull request overview
This PR replaces the prior per-pane vs all-panes byte subscription APIs with a single scope-based subscription surface (attachBytesSink) backed by a new SinkRegistry + PaneTopologyManager, and updates all client/bridge implementations and tests accordingly. It also fixes a topology update gap so newly added windows/panes correctly participate in session/window-scoped routing.
Changes:
- Introduce
BytesSink+PaneScopeand implement scope-bifurcated dispatch viaSinkRegistry, with topology tracked byPaneTopologyManager. - Replace
attachPaneSink/attachAllPanesSinkacross core client, WebSocket, Electron, pane-terminal stream, demos, and tests withattachBytesSink(..., { scope }). - Add integration coverage for all four scope kinds and topology bootstrap / dynamic membership behavior.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/web-contents-sink.test.ts | Updates unit test sink contract usage to BytesSink (msg.data) instead of raw byte arrays. |
| tests/unit/text-stream-sink.test.ts | Updates text-stream sink tests to write PaneOutputMessage into the sink. |
| tests/unit/pane-sink.test.ts | Migrates behavioral tests from attachPaneSink/attachAllPanesSink to scope-based attachBytesSink. |
| tests/unit/pane-sink-registry.test.ts | Replaces registry tests with SinkRegistry + PaneTopologyManager scope/topology coverage. |
| tests/unit/electron-bridge.test.ts | Updates Electron bridge tests to use attachBytesSink + paneScope. |
| tests/integration/websocket-bridge.test.ts | Migrates WS bridge integration test to server-scope attachBytesSink. |
| tests/integration/pane-scope.test.ts | Adds new integration suite covering server/session/window/pane scopes against a real tmux server. |
| tests/integration/client.test.ts | Updates notification coverage test to use attachBytesSink instead of all-panes mux API. |
| src/sinks/text-stream.ts | Adapts createTextStreamSink to accept PaneOutputMessage via BytesSink. |
| src/pane-sink.ts | Removes the old PaneByteSink/PaneByteMultiplexer + PaneSinkRegistry implementation. |
| src/pane-output.ts | Adds new BytesSink, PaneScope, SinkRegistry, PaneTopologyManager, and parsePaneListLine. |
| src/index.ts | Updates public exports to the new pane-output API surface. |
| src/emitter.ts | Updates documentation to reflect bytes flowing through SinkRegistry/attachBytesSink. |
| src/connectors/websocket/sink.ts | Updates per-pane WS forwarder to attach via attachBytesSink + paneScope. |
| src/connectors/websocket/server.ts | Updates WS server connection byte forwarding to use BytesSink + attachBytesSink. |
| src/connectors/websocket/client.ts | Migrates WS client to SinkRegistry + PaneTopologyManager and adds lazy topology bootstrap/updates. |
| src/connectors/electron/renderer.ts | Migrates renderer proxy to SinkRegistry + PaneTopologyManager and adds lazy topology bootstrap/updates. |
| src/connectors/electron/main.ts | Migrates main-process bridge byte forwarding to BytesSink + attachBytesSink. |
| src/client.ts | Replaces pane sink APIs with attachBytesSink, adds lazy topology bootstrap + topology event handling. |
| packages/pane-terminal/src/stream/pane-stream.ts | Switches PaneStream byte attachment to attachBytesSink with paneScope. |
| packages/pane-terminal/src/bench/fake-tmux-client.ts | Updates fake client to attachBytesSink and adds seedTopology for scoped tests. |
| IMPL.md | Updates implementation notes to the new scope-based API and topology model. |
| examples/web-multiplexer/web/pane-stream-bridge.ts | Updates demo bridge client to attachBytesSink + scoped routing and topology refresh logic. |
| CHANGELOG.md | Documents the breaking API change and the new scope/topology components. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- BytesSink JSDoc: reference createTextStreamSink (existing API) instead
of attachLineSink (not yet shipped) for the UTF-8 text lines use case
- Re-export createTextStreamSink from src/index.ts — it was part of the
public API before this branch and was silently dropped
- WS sink: pass msg directly to encodePaneOutput instead of constructing
{ type: "output", ... } — extended-output type and age field were lost
…nded-output The IPC envelope was a lossy subset of PaneOutputMessage — it carried only (paneId, data) and hardcoded type:"output" on the renderer side, silently dropping the extended-output discriminator and age field. [LAW:one-source-of-truth] The envelope IS a PaneOutputMessage; making it a type alias collapses two representations into one. main.ts sends msg directly; renderer.ts calls sink.write(envelope) as-is — no reconstruction, no loss.
…ng synchronous ones JavaScript processes synchronous notification handlers before Promise microtasks in the same I/O frame. When window-close and %end (for an in-flight list-panes) arrive in the same TCP segment, removeWindow fires synchronously, then seed()/updateWindow() fire as microtasks — re-adding the closed window's panes. TopologyEpochTracker (added to pane-output.ts) provides per-bootstrap and per-window generation counters. Each async query captures its gen before execute(); window-close calls invalidateWindow() to advance the counter. The microtask checks isBootstrapCurrent/isWindowRefreshCurrent before applying — if the gen advanced, the result is stale and discarded. [LAW:one-type-per-behavior] One tracker type, four clients. [LAW:dataflow-not-control-flow] The generation IS a value proving the result is authoritative — data deciding, not control-flow skipping.
…r docs - Export TopologyEpochTracker and parsePaneListLine from src/index.ts so TmuxClientLike implementors (like pane-stream-bridge) can depend on the public API rather than reaching into src/ directly - Update pane-stream-bridge to import from @promptctl/tmux-control-mode-js - CHANGELOG: remove "internal" label from PaneTopologyManager (it IS in public API) and document the three new exports - createTextStreamSink: document that handler must not throw — it runs inside BytesSink.write and a throwing handler breaks co-attached sinks [LAW:no-defensive-null-guards]: silent try/catch would hide the error; the contract is the enforcer, not a swallow
…aint - TopologyEpochTracker.invalidateWindow: delete the entry instead of incrementing. The window is gone; isWindowRefreshCurrent correctly returns false for a missing key. Prevents unbounded Map growth on long-running clients cycling through many transient windows. - createTextStreamSink: document the single-pane constraint — multi-pane scopes interleave bytes from different panes into one decoder, corrupting multi-byte UTF-8 sequences at chunk boundaries.
updateWindow() and removeWindow() deleted table entries based on windowIndex alone. If a pane moved to another window and that window's refresh ran first, table already held the correct new mapping — but the old window's operation would delete it, breaking session/window routing. Fix: check table.get(paneId)?.windowId === windowId before deleting. The table is the authoritative mapping; windowIndex is derived state. A deletion is only safe when both agree. [LAW:one-source-of-truth]: table is the canonical pane→window source. Deletions now verify against table, not just the reverse index.
startBootstrap() now clears windowGens so any in-flight list-panes -t @W
result is correctly discarded when a sessions-changed bootstrap supersedes
it. Previously, an in-flight window refresh could pass isWindowRefreshCurrent
after the bootstrap seeded a fresh table, clobbering the new authoritative
state.
Also fixes the attachWebContentsSink JSDoc: the IPC frame now forwards
the full PaneOutputMessage directly (PaneBytesEnvelope = PaneOutputMessage),
not the old { paneId, data } shape.
After startBootstrap() clears windowGens, per-window counters that restart from 0+1=1 can alias tokens issued to pre-clear refreshes (both capture gen=1 for the same windowId). isWindowRefreshCurrent would then incorrectly accept the stale pre-bootstrap result as current. Fix: replace the per-window (prev??0)+1 increment with a single global windowEpoch that only increases. Every startWindowRefresh call gets a strictly-greater-than-all-prior token. Post-clear refreshes draw from the same sequence and therefore cannot collide with pre-clear tokens. [LAW:types-are-the-program]: the per-window counter type admitted re-use after a clear. The global monotone counter makes collision structurally impossible by construction.
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
attachPaneSink/attachAllPanesSink/PaneByteSink/PaneByteMultiplexerwith a single scope-based entry point:attachBytesSink(sink, { scope }).PaneScopediscriminated union (server / session / window / pane),BytesSinkcontract,SinkRegistry(four scope-bifurcated buckets, snapshot-before-write dispatch), andPaneTopologyManager(paneId → {sessionId, windowId}).window-addandunlinked-window-addwere not triggering topology updates, causing new panes to miss initial session/window-scope routing. Fixed across all threeTmuxClientLikeimplementations.tmux-pane-output-i3m.1.Breaking Changes
attachPaneSink,attachAllPanesSink,PaneByteSink,PaneByteMultiplexer,PaneSinkRegistryremoved.attachPaneSink(id, sink)→attachBytesSink(sink, { scope: paneScope(id) });attachAllPanesSink(mux)→attachBytesSink(mux).Test plan
pnpm run test:all— 554 tests pass (29 files), includes 9 new integration tests inpane-scope.test.tscovering all four scope kinds against a real ephemeral tmux server.pnpm run build— clean TypeScript compilation.