Skip to content

feat(client)!: BytesSink + attachBytesSink + PaneScope substrate (tmux-pane-output-i3m.1)#59

Merged
brandon-fryslie merged 11 commits into
masterfrom
pane-output-substrate-i3m1
May 27, 2026
Merged

feat(client)!: BytesSink + attachBytesSink + PaneScope substrate (tmux-pane-output-i3m.1)#59
brandon-fryslie merged 11 commits into
masterfrom
pane-output-substrate-i3m1

Conversation

@brandon-fryslie

Copy link
Copy Markdown
Contributor

Summary

  • Replaces attachPaneSink / attachAllPanesSink / PaneByteSink / PaneByteMultiplexer with a single scope-based entry point: attachBytesSink(sink, { scope }).
  • Introduces PaneScope discriminated union (server / session / window / pane), BytesSink contract, SinkRegistry (four scope-bifurcated buckets, snapshot-before-write dispatch), and PaneTopologyManager (paneId → {sessionId, windowId}).
  • Topology bootstrap is lazy — only fires when a session/window-scoped sink is attached.
  • Bug fix: window-add and unlinked-window-add were not triggering topology updates, causing new panes to miss initial session/window-scope routing. Fixed across all three TmuxClientLike implementations.
  • Closes ticket tmux-pane-output-i3m.1.

Breaking Changes

  • attachPaneSink, attachAllPanesSink, PaneByteSink, PaneByteMultiplexer, PaneSinkRegistry removed.
  • Migration: 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 in pane-scope.test.ts covering all four scope kinds against a real ephemeral tmux server.
  • pnpm run build — clean TypeScript compilation.

…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).
Copilot AI review requested due to automatic review settings May 27, 2026 10:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 + PaneScope and implement scope-bifurcated dispatch via SinkRegistry, with topology tracked by PaneTopologyManager.
  • Replace attachPaneSink / attachAllPanesSink across core client, WebSocket, Electron, pane-terminal stream, demos, and tests with attachBytesSink(..., { 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.

Comment thread src/pane-output.ts Outdated
Comment thread src/index.ts
Comment thread src/connectors/websocket/sink.ts Outdated
- 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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

Comment thread src/connectors/electron/renderer.ts
Comment thread src/connectors/electron/main.ts Outdated
…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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 8 comments.

Comment thread src/connectors/websocket/sink.ts
Comment thread src/client.ts
Comment thread src/connectors/websocket/client.ts
Comment thread src/connectors/electron/renderer.ts
Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md Outdated
Comment thread IMPL.md Outdated
Comment thread IMPL.md Outdated
…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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.

Comment thread examples/web-multiplexer/web/pane-stream-bridge.ts Outdated
Comment thread CHANGELOG.md Outdated
Comment thread src/sinks/text-stream.ts
…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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

Comment thread src/pane-output.ts Outdated
Comment thread src/sinks/text-stream.ts
…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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

Comment thread tests/integration/pane-scope.test.ts Outdated
Comment thread tests/integration/pane-scope.test.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

Comment thread src/pane-output.ts Outdated
Comment thread src/pane-output.ts
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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

Comment thread src/pane-output.ts Outdated
Comment thread src/connectors/electron/main.ts
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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Comment thread src/pane-output.ts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Comment thread src/pane-output.ts
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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

@brandon-fryslie brandon-fryslie merged commit c685f82 into master May 27, 2026
1 of 2 checks passed
@brandon-fryslie brandon-fryslie deleted the pane-output-substrate-i3m1 branch May 27, 2026 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants