fix(tmux): prefer split-or-defer with FIFO deferred attach#1844
fix(tmux): prefer split-or-defer with FIFO deferred attach#1844liu-qingyuan wants to merge 1 commit intocode-yeongyu:devfrom
Conversation
|
All contributors have signed the CLA. Thank you! ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 14 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Tests added and refactor ensures layout split decisions respect config. Cubic's AI review found no issues. Changes follow strict main-pane size rules, avoiding regressions.
Fixes: #1671 What was wrong
What changed
Why this fixes #1671This makes pane spawning follow the layout decision path consistently instead of replacing active panes when geometry is tight, which was a key source of the “layout looks ignored” behavior. Config used (for repro / verification)// Tmux multi-pane view for background subagents
"tmux": {
"enabled": true,
"layout": "main-vertical",
"main_pane_size": 60,
"main_pane_min_width": 120,
"agent_pane_min_width": 40
}This is the config I used to reproduce/verify the issue locally. Note on limitstmux geometry limits (height/width thresholds) still apply. Testing
If possible, could you retest on the latest |
liu-qingyuan
force-pushed
the
fix/tmux-split-defer-fifo
branch
from
c595ab9 to
5343f60
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the tmux subagent’s pane management to prefer safe splits and otherwise defer session attachment, adding FIFO deferred-attach behavior and expanding layout-aware sizing/planning.
Changes:
- Introduces layout-aware split targeting (main-horizontal vs main-vertical) and removes the “replace oldest pane” fallback in favor of deferring attach.
- Adds FIFO deferred attach queueing with a background retry loop and spawn serialization.
- Refactors width/capacity computations and adds/expands tests around layout rules, split-or-defer decisions, and deferred lifecycle.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/tmux/tmux-utils/layout.ts | Adds configurable main-pane width calculation and injectable spawn dependency for layout application. |
| src/shared/tmux/tmux-utils/layout.test.ts | Adds unit tests for applyLayout behavior across main/non-main layouts. |
| src/features/tmux-subagent/types.ts | Extends capacity config with layout + main pane size inputs. |
| src/features/tmux-subagent/tmux-grid-constants.ts | Adds config-driven main pane sizing and width/agent-area computations. |
| src/features/tmux-subagent/grid-planning.ts | Updates capacity/grid planning to accept either config or numeric options and use computed agent area width. |
| src/features/tmux-subagent/pane-split-availability.ts | Makes split feasibility depend on configurable minimum pane width. |
| src/features/tmux-subagent/spawn-target-finder.ts | Adds strict main-layout targeting and deterministic fallback ordering. |
| src/features/tmux-subagent/spawn-action-decider.ts | Switches from replace fallback to defer attach; makes decisions layout-aware. |
| src/features/tmux-subagent/pane-state-querier.ts | Changes pane-state parsing delimiter and updates main-pane selection heuristics. |
| src/features/tmux-subagent/action-executor.ts | Reapplies layout and enforces main-pane width after actions using latest window state. |
| src/features/tmux-subagent/manager.ts | Adds spawn serialization and FIFO deferred attach queue w/ retry loop; threads sourcePaneId through executor calls. |
| src/features/tmux-subagent/manager.test.ts | Updates/expands tests for defer behavior, FIFO order, idempotency, and lifecycle. |
| src/features/tmux-subagent/layout-config.test.ts | Adds layout-specific behavioral tests for split direction/deferral rules. |
| src/features/tmux-subagent/decision-engine.test.ts | Expands tests for min-width and main-pane-size driven split/defer decisions and spawn targeting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function resolveMinPaneWidth(options?: CapacityOptions): number { | ||
| if (typeof options === "number") { | ||
| return Math.max(1, options) | ||
| } |
There was a problem hiding this comment.
resolveMinPaneWidth() ignores CapacityConfig.agentPaneWidth when options is a config object, always falling back to MIN_PANE_WIDTH. This can overestimate grid capacity / slot widths under stricter agentPaneWidth settings and lead to inconsistent planning vs split checks. Consider deriving minPaneWidth from options.agentPaneWidth when options is a CapacityConfig (and only defaulting to MIN_PANE_WIDTH when config is absent).
| } | |
| } | |
| if (options && typeof (options as CapacityConfig).agentPaneWidth === "number") { | |
| return Math.max(1, (options as CapacityConfig).agentPaneWidth) | |
| } |
| private startDeferredAttachLoop(): void { | ||
| if (this.deferredAttachInterval) return | ||
| this.deferredAttachInterval = setInterval(() => { | ||
| void this.enqueueSpawn(async () => { | ||
| await this.tryAttachDeferredSession() | ||
| }) | ||
| }, POLL_INTERVAL_BACKGROUND_MS) |
There was a problem hiding this comment.
startDeferredAttachLoop() uses setInterval to enqueue a new task every tick via enqueueSpawn(). If tryAttachDeferredSession() ever takes longer than POLL_INTERVAL_BACKGROUND_MS (or tmux calls hang), the promise chain can grow without bound, causing memory/backlog buildup. Consider ensuring only one deferred-attach attempt is in flight at a time (e.g., skip scheduling when a run is already pending, or use a self-scheduling setTimeout that queues the next run only after the previous attempt finishes).
| tmux, | ||
| "list-panes", | ||
| "-t", | ||
| sourcePaneId, | ||
| "-F", | ||
| "#{pane_id},#{pane_width},#{pane_height},#{pane_left},#{pane_top},#{pane_title},#{pane_active},#{window_width},#{window_height}", | ||
| "#{pane_id}\t#{pane_width}\t#{pane_height}\t#{pane_left}\t#{pane_top}\t#{pane_title}\t#{pane_active}\t#{window_width}\t#{window_height}", | ||
| ], |
There was a problem hiding this comment.
The list-panes -F format places #{pane_title} mid-record and parsing is done via line.split("\t"). If a pane title contains a tab character, the fields shift and widths/heights/window sizes will be parsed incorrectly. To make parsing robust, consider moving #{pane_title} to the end of the format (or using a delimiter/escaping strategy that cannot occur in titles, and reconstructing the title from remaining fields).
| } | ||
|
|
||
| export interface CapacityConfig { | ||
| layout?: string |
There was a problem hiding this comment.
Indentation in CapacityConfig is inconsistent: layout?: string is tab-indented while surrounding fields use two spaces. Please align indentation to match the rest of this file to avoid churn and formatter diffs.
| layout?: string | |
| layout?: string |
1 participant
Summary
Why
Under constrained pane geometry, replacing active panes can evict in-progress work and reduce reliability.
Defer-and-reattach preserves active panes while still ensuring pending sessions are attached when pane capacity is available.
Testing
bun test src/features/tmux-subagent- Passed (56 pass, 0 fail)bun run build- PassedNotes
Summary by cubic
Prefer splitting panes when safe and defer attaching when not, with a FIFO queue that auto-attaches once capacity returns. After each spawn/close, the manager reapplies the tmux layout and enforces exact main-pane sizing based on config.
New Features
Refactors
Written for commit 5343f60. Summary will update on new commits.