-
Notifications
You must be signed in to change notification settings - Fork 1.8k
refactor(tmux-subagent): state-first architecture with decision engine #1125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 issues found across 15 files
Confidence score: 3/5
- Potential stale pane mappings in
src/features/tmux-subagent/decision-engine.tscan attempt to close non-existent panes and block new spawns, which is a concrete user-impacting risk. - Unsafe
line.split(",")parsing insrc/features/tmux-subagent/pane-state-querier.tscan misclassify panes when titles contain commas, adding uncertainty to pane management behavior. - Overall risk is moderate due to multiple medium-severity issues in tmux pane lifecycle handling rather than minor housekeeping changes.
- Pay close attention to
src/features/tmux-subagent/decision-engine.ts,src/features/tmux-subagent/pane-state-querier.ts,src/tools/delegate-task/tools.ts- stale mappings, unsafe parsing, and fixed sleeps can cause failures or delays.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/features/tmux-subagent/pane-state-querier.ts">
<violation number="1" location="src/features/tmux-subagent/pane-state-querier.ts:42">
P2: Parsing tmux output with `line.split(",")` is unsafe because `pane_title` can contain commas, which corrupts field positions and can misclassify panes. Parse from the end or use a safer delimiter.</violation>
</file>
<file name="src/features/tmux-subagent/decision-engine.ts">
<violation number="1" location="src/features/tmux-subagent/decision-engine.ts:77">
P2: Filter cached mappings by panes that actually exist in the queried window state before choosing the oldest to close; otherwise a stale mapping can cause a close failure and prevent spawning.</violation>
</file>
<file name="src/tools/delegate-task/tools.ts">
<violation number="1" location="src/tools/delegate-task/tools.ts:865">
P2: Avoid fixed sleeps in tool logic; use a readiness/polling check or remove the delay so execution isn’t gated by an arbitrary timeout.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| if (state.agentPanes.length > 0) { | ||
| const oldest = findOldestSession(sessionMappings) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Filter cached mappings by panes that actually exist in the queried window state before choosing the oldest to close; otherwise a stale mapping can cause a close failure and prevent spawning.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/tmux-subagent/decision-engine.ts, line 77:
<comment>Filter cached mappings by panes that actually exist in the queried window state before choosing the oldest to close; otherwise a stale mapping can cause a close failure and prevent spawning.</comment>
<file context>
@@ -0,0 +1,130 @@
+ }
+
+ if (state.agentPanes.length > 0) {
+ const oldest = findOldestSession(sessionMappings)
+
+ if (oldest) {
</file context>
| }).catch((err) => { | ||
| log("[delegate_task] onSyncSessionCreated callback failed", { error: String(err) }) | ||
| }) | ||
| await new Promise(r => setTimeout(r, 200)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Avoid fixed sleeps in tool logic; use a readiness/polling check or remove the delay so execution isn’t gated by an arbitrary timeout.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/delegate-task/tools.ts, line 865:
<comment>Avoid fixed sleeps in tool logic; use a readiness/polling check or remove the delay so execution isn’t gated by an arbitrary timeout.</comment>
<file context>
@@ -845,6 +852,19 @@ To continue this session: session_id="${task.sessionID}"`
+ }).catch((err) => {
+ log("[delegate_task] onSyncSessionCreated callback failed", { error: String(err) })
+ })
+ await new Promise(r => setTimeout(r, 200))
+ }
+
</file context>
5ce04d6 to
84ee192
Compare
…ngine Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <[email protected]>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <[email protected]>
84ee192 to
dcc4404
Compare
Summary
Major refactor of tmux-subagent to use state-first architecture (Query → Decide → Execute pattern).
Architecture Changes
New Config Options
main_pane_min_width: Minimum width for main pane (default: 120)agent_pane_min_width: Minimum width for agent panes (default: 40)Callback Integration
onSubagentSessionCreatedcallbackonSyncSessionCreatedcallbackTesting
Summary by cubic
Refactored tmux-subagent to a state-first architecture with a Query → Decide → Execute flow, improving reliable pane spawning/closing and capacity handling. Added session-created callbacks so panes spawn immediately for background and sync sessions.
New Features
Refactors
Written for commit dcc4404. Summary will update on new commits.