Fix MCP agent split-pane targeting, default direction, and guidance#287
Fix MCP agent split-pane targeting, default direction, and guidance#287danshapiro wants to merge 63 commits intomainfrom
Conversation
# Conflicts: # src/components/TabsView.tsx # test/unit/client/components/TabsView.test.tsx
…ting Code fixes: - Fix split-pane with explicit target: route through resolvePaneTarget() (consistent with send-keys, capture-pane, etc.) so bare numeric indices resolve in the caller's tab, not the user's active viewport - Change default split direction from horizontal to vertical (REST API, CLI flag -v -> -h) Instruction updates: - Add decision guide table to HELP_TEXT (split-pane vs new-tab, editor vs terminal+cat, browser vs terminal+curl) - Add 'Choosing the right action' section to MCP INSTRUCTIONS covering tabs-vs-panes, specialized pane types, literal mode, and targeting - Reorder playbooks to show split-pane before new-tab - Fix split-pane description from 'active pane' to 'your own pane' - Update SKILL.md with matching guidance and new -h flag
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 525caf7b03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (msg.code === 'INVALID_SESSION_ID') { | ||
| // Session is gone on the server (e.g. server restarted). Mark it as lost | ||
| // so AgentChatView can detect this and trigger immediate recovery instead | ||
| // of waiting for the 5-second timeout. | ||
| // so AgentChatView can detect this and trigger immediate recovery. | ||
| dispatch(markSessionLost({ sessionId: msg.sessionId as string })) | ||
| } else { |
There was a problem hiding this comment.
Treat RESTORE_NOT_FOUND as recoverable lost session
sdk.attach now emits sdk.error with code: "RESTORE_NOT_FOUND" when a persisted session cannot be restored (server/ws-handler.ts), but this branch only marks sessions lost for INVALID_SESSION_ID. In the reload/stale-session path, that leaves historyLoaded false and only records lastError, so AgentChatView remains stuck in “Restoring session...” with input disabled instead of recreating the session. Please classify restore-not-found restore errors as recoverable (or trigger equivalent recovery) so users are not trapped after restart/history cleanup.
Useful? React with 👍 / 👎.
Summary
Agents using the Freshell MCP tool were making three systematic mistakes:
Code fix
split-panewith explicit target bypassed MCP-level resolution. When an agent passed a target (e.g. bare numeric index"0"), the MCP tool sent it raw to the REST API, where the server resolved it in the user's active viewport tab — not the caller's tab. Now routes throughresolvePaneTarget()like all other pane-targeting actions (send-keys,capture-pane,screenshot).Default direction change
-v(vertical) replaced with-h(horizontal).Agent instruction updates
split-pane(editor/browser) beforenew-tabTest plan
split-panewitheditor:param for file opens