From b4c5f155ed78929bb75dc2c3a314d2773d6034f1 Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Tue, 3 Feb 2026 01:24:01 -0600 Subject: [PATCH 1/8] feat(symphony): add manual contribution credit handler Add symphony:manualCredit IPC handler to allow crediting contributions made outside the Symphony workflow (e.g., manual PRs, external contributors). This enables proper tracking of all contributions regardless of how they were created. - Add symphony:manualCredit handler with full validation - Add preload API for manual credit - Support all contribution fields (tokens, time, merged status, etc.) - Prevent duplicate PR credits - Update contributor stats (streak, repos, totals) - Add comprehensive tests for validation and success cases --- .../main/ipc/handlers/symphony.test.ts | 194 ++++++++++++++++++ src/main/ipc/handlers/symphony.ts | 172 +++++++++++++++- src/main/preload/symphony.ts | 22 ++ 3 files changed, 384 insertions(+), 4 deletions(-) diff --git a/src/__tests__/main/ipc/handlers/symphony.test.ts b/src/__tests__/main/ipc/handlers/symphony.test.ts index 18d34133..578c98d9 100644 --- a/src/__tests__/main/ipc/handlers/symphony.test.ts +++ b/src/__tests__/main/ipc/handlers/symphony.test.ts @@ -5244,4 +5244,198 @@ This is a Symphony task document. expect(result.success).toBe(true); }); }); + + // ============================================================================ + // Manual Credit Tests (symphony:manualCredit) + // ============================================================================ + + describe('symphony:manualCredit', () => { + const getManualCreditHandler = () => handlers.get('symphony:manualCredit'); + + beforeEach(() => { + // Reset state to empty + vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); + vi.mocked(fs.mkdir).mockResolvedValue(undefined); + vi.mocked(fs.writeFile).mockResolvedValue(undefined); + }); + + describe('validation', () => { + it('should reject missing required fields', async () => { + const handler = getManualCreditHandler(); + const result = await handler!({} as any, {}); + + // Handler returns { error: '...' }, wrapper adds success: true + // So validation errors show as { success: true, error: '...' } + expect(result.error).toContain('Missing required fields'); + expect(result.contributionId).toBeUndefined(); + }); + + it('should reject missing repoSlug', async () => { + const handler = getManualCreditHandler(); + const result = await handler!({} as any, { + repoName: 'Test Repo', + issueNumber: 123, + prNumber: 456, + prUrl: 'https://github.com/owner/repo/pull/456', + }); + + expect(result.error).toContain('Missing required fields'); + expect(result.contributionId).toBeUndefined(); + }); + + it('should reject duplicate PR credit', async () => { + // Setup existing state with a contribution + vi.mocked(fs.readFile).mockResolvedValue( + JSON.stringify({ + active: [], + history: [ + { + id: 'existing_contrib', + repoSlug: 'owner/repo', + prNumber: 456, + }, + ], + stats: { + totalContributions: 1, + totalMerged: 0, + totalIssuesResolved: 0, + totalDocumentsProcessed: 0, + totalTasksCompleted: 0, + totalTokensUsed: 0, + totalTimeSpent: 0, + estimatedCostDonated: 0, + repositoriesContributed: ['owner/repo'], + currentStreak: 0, + longestStreak: 0, + }, + }) + ); + + const handler = getManualCreditHandler(); + const result = await handler!({} as any, { + repoSlug: 'owner/repo', + repoName: 'Test Repo', + issueNumber: 123, + issueTitle: 'Test Issue', + prNumber: 456, + prUrl: 'https://github.com/owner/repo/pull/456', + }); + + expect(result.error).toContain('already credited'); + expect(result.contributionId).toBeUndefined(); + }); + }); + + describe('successful credit', () => { + it('should create a completed contribution with minimal params', async () => { + const handler = getManualCreditHandler(); + const result = await handler!({} as any, { + repoSlug: 'owner/repo', + repoName: 'Test Repo', + issueNumber: 123, + issueTitle: 'Test Issue', + prNumber: 456, + prUrl: 'https://github.com/owner/repo/pull/456', + }); + + expect(result.success).toBe(true); + expect(result.contributionId).toMatch(/^manual_123_/); + + // Verify state was written + expect(fs.writeFile).toHaveBeenCalled(); + const writeCall = vi.mocked(fs.writeFile).mock.calls[0]; + const writtenState = JSON.parse(writeCall[1] as string); + + expect(writtenState.history).toHaveLength(1); + expect(writtenState.history[0].repoSlug).toBe('owner/repo'); + expect(writtenState.history[0].prNumber).toBe(456); + expect(writtenState.stats.totalContributions).toBe(1); + }); + + it('should handle wasMerged flag correctly', async () => { + const handler = getManualCreditHandler(); + const result = await handler!({} as any, { + repoSlug: 'owner/repo', + repoName: 'Test Repo', + issueNumber: 123, + issueTitle: 'Test Issue', + prNumber: 456, + prUrl: 'https://github.com/owner/repo/pull/456', + wasMerged: true, + mergedAt: '2026-02-02T23:31:31Z', + }); + + expect(result.success).toBe(true); + + const writeCall = vi.mocked(fs.writeFile).mock.calls[0]; + const writtenState = JSON.parse(writeCall[1] as string); + + expect(writtenState.history[0].wasMerged).toBe(true); + expect(writtenState.history[0].mergedAt).toBe('2026-02-02T23:31:31Z'); + expect(writtenState.stats.totalMerged).toBe(1); + expect(writtenState.stats.totalIssuesResolved).toBe(1); + }); + + it('should add repo to repositoriesContributed if not already present', async () => { + const handler = getManualCreditHandler(); + await handler!({} as any, { + repoSlug: 'new-owner/new-repo', + repoName: 'New Repo', + issueNumber: 1, + issueTitle: 'Issue 1', + prNumber: 1, + prUrl: 'https://github.com/new-owner/new-repo/pull/1', + }); + + const writeCall = vi.mocked(fs.writeFile).mock.calls[0]; + const writtenState = JSON.parse(writeCall[1] as string); + + expect(writtenState.stats.repositoriesContributed).toContain('new-owner/new-repo'); + }); + + it('should accept custom token usage', async () => { + const handler = getManualCreditHandler(); + await handler!({} as any, { + repoSlug: 'owner/repo', + repoName: 'Test Repo', + issueNumber: 123, + issueTitle: 'Test Issue', + prNumber: 456, + prUrl: 'https://github.com/owner/repo/pull/456', + tokenUsage: { + inputTokens: 50000, + outputTokens: 25000, + totalCost: 1.5, + }, + }); + + const writeCall = vi.mocked(fs.writeFile).mock.calls[0]; + const writtenState = JSON.parse(writeCall[1] as string); + + expect(writtenState.history[0].tokenUsage.inputTokens).toBe(50000); + expect(writtenState.history[0].tokenUsage.outputTokens).toBe(25000); + expect(writtenState.history[0].tokenUsage.totalCost).toBe(1.5); + expect(writtenState.stats.totalTokensUsed).toBe(75000); + expect(writtenState.stats.estimatedCostDonated).toBe(1.5); + }); + + it('should set firstContributionAt on first credit', async () => { + const handler = getManualCreditHandler(); + await handler!({} as any, { + repoSlug: 'owner/repo', + repoName: 'Test Repo', + issueNumber: 123, + issueTitle: 'Test Issue', + prNumber: 456, + prUrl: 'https://github.com/owner/repo/pull/456', + }); + + const writeCall = vi.mocked(fs.writeFile).mock.calls[0]; + const writtenState = JSON.parse(writeCall[1] as string); + + expect(writtenState.stats.firstContributionAt).toBeDefined(); + expect(writtenState.stats.lastContributionAt).toBeDefined(); + }); + }); + }); }); diff --git a/src/main/ipc/handlers/symphony.ts b/src/main/ipc/handlers/symphony.ts index 7a64fcea..e72cb4cc 100644 --- a/src/main/ipc/handlers/symphony.ts +++ b/src/main/ipc/handlers/symphony.ts @@ -17,6 +17,7 @@ import { logger } from '../../utils/logger'; import { isWebContentsAvailable } from '../../utils/safe-send'; import { createIpcHandler, CreateHandlerOptions } from '../../utils/ipcHandler'; import { execFileNoThrow } from '../../utils/execFile'; +import { getExpandedEnv } from '../../agents/path-prober'; import { SYMPHONY_REGISTRY_URL, REGISTRY_CACHE_TTL_MS, @@ -575,7 +576,7 @@ async function createBranch( * Check if gh CLI is authenticated. */ async function checkGhAuthentication(): Promise<{ authenticated: boolean; error?: string }> { - const result = await execFileNoThrow('gh', ['auth', 'status']); + const result = await execFileNoThrow('gh', ['auth', 'status'], undefined, getExpandedEnv()); if (result.exitCode !== 0) { // gh auth status outputs to stderr even on success for some info const output = result.stderr + result.stdout; @@ -685,7 +686,8 @@ async function createDraftPR( '--body', body, ], - repoPath + repoPath, + getExpandedEnv() ); if (prResult.exitCode !== 0) { @@ -710,7 +712,12 @@ async function markPRReady( repoPath: string, prNumber: number ): Promise<{ success: boolean; error?: string }> { - const result = await execFileNoThrow('gh', ['pr', 'ready', String(prNumber)], repoPath); + const result = await execFileNoThrow( + 'gh', + ['pr', 'ready', String(prNumber)], + repoPath, + getExpandedEnv() + ); if (result.exitCode !== 0) { return { success: false, error: result.stderr }; @@ -833,7 +840,8 @@ This pull request was created using [Maestro Symphony](https://runmaestro.ai/sym const result = await execFileNoThrow( 'gh', ['pr', 'comment', String(prNumber), '--body', commentBody], - repoPath + repoPath, + getExpandedEnv() ); if (result.exitCode !== 0) { @@ -2579,5 +2587,161 @@ This PR will be updated automatically when the Auto Run completes.`; ) ); + /** + * Manually credit a contribution (for contributions made outside Symphony workflow). + * This allows crediting a user for work done on a PR that wasn't tracked through Symphony. + */ + ipcMain.handle( + 'symphony:manualCredit', + createIpcHandler( + handlerOpts('manualCredit'), + async (params: { + repoSlug: string; + repoName: string; + issueNumber: number; + issueTitle: string; + prNumber: number; + prUrl: string; + startedAt?: string; + completedAt?: string; + wasMerged?: boolean; + mergedAt?: string; + tokenUsage?: { + inputTokens?: number; + outputTokens?: number; + totalCost?: number; + }; + timeSpent?: number; + documentsProcessed?: number; + tasksCompleted?: number; + }): Promise<{ contributionId?: string; error?: string }> => { + const { + repoSlug, + repoName, + issueNumber, + issueTitle, + prNumber, + prUrl, + startedAt, + completedAt, + wasMerged, + mergedAt, + tokenUsage, + timeSpent, + documentsProcessed, + tasksCompleted, + } = params; + + // Validate required fields + if (!repoSlug || !repoName || !issueNumber || !prNumber || !prUrl) { + return { error: 'Missing required fields: repoSlug, repoName, issueNumber, prNumber, prUrl' }; + } + + const state = await readState(app); + + // Check if this PR is already credited + const existingContribution = state.history.find( + (c) => c.repoSlug === repoSlug && c.prNumber === prNumber + ); + if (existingContribution) { + return { error: `PR #${prNumber} is already credited (contribution: ${existingContribution.id})` }; + } + + const now = new Date().toISOString(); + const contributionId = `manual_${issueNumber}_${Date.now()}`; + + const completed: CompletedContribution = { + id: contributionId, + repoSlug, + repoName, + issueNumber, + issueTitle: issueTitle || `Issue #${issueNumber}`, + startedAt: startedAt || now, + completedAt: completedAt || now, + prUrl, + prNumber, + tokenUsage: { + inputTokens: tokenUsage?.inputTokens ?? 0, + outputTokens: tokenUsage?.outputTokens ?? 0, + totalCost: tokenUsage?.totalCost ?? 0, + }, + timeSpent: timeSpent ?? 0, + documentsProcessed: documentsProcessed ?? 0, + tasksCompleted: tasksCompleted ?? 1, + wasMerged: wasMerged ?? false, + mergedAt: mergedAt, + }; + + // Add to history + state.history.push(completed); + + // Update stats + state.stats.totalContributions += 1; + state.stats.totalDocumentsProcessed += completed.documentsProcessed; + state.stats.totalTasksCompleted += completed.tasksCompleted; + state.stats.totalTokensUsed += + completed.tokenUsage.inputTokens + completed.tokenUsage.outputTokens; + state.stats.totalTimeSpent += completed.timeSpent; + state.stats.estimatedCostDonated += completed.tokenUsage.totalCost; + + if (!state.stats.repositoriesContributed.includes(repoSlug)) { + state.stats.repositoriesContributed.push(repoSlug); + } + + if (wasMerged) { + state.stats.totalMerged = (state.stats.totalMerged || 0) + 1; + state.stats.totalIssuesResolved = (state.stats.totalIssuesResolved || 0) + 1; + } + + state.stats.lastContributionAt = completed.completedAt; + if (!state.stats.firstContributionAt) { + state.stats.firstContributionAt = completed.completedAt; + } + + // Update streak + const getWeekNumber = (date: Date): string => { + const d = new Date(Date.UTC(date.getFullYear(), date.getMonth(), date.getDate())); + const dayNum = d.getUTCDay() || 7; + d.setUTCDate(d.getUTCDate() + 4 - dayNum); + const yearStart = new Date(Date.UTC(d.getUTCFullYear(), 0, 1)); + const weekNo = Math.ceil(((d.getTime() - yearStart.getTime()) / 86400000 + 1) / 7); + return `${d.getUTCFullYear()}-W${weekNo}`; + }; + const currentWeek = getWeekNumber(new Date()); + const lastWeek = state.stats.lastContributionDate; + if (lastWeek) { + const oneWeekAgo = new Date(Date.now() - 7 * 24 * 60 * 60 * 1000); + const previousWeek = getWeekNumber(oneWeekAgo); + if (lastWeek === previousWeek || lastWeek === currentWeek) { + if (lastWeek !== currentWeek) { + state.stats.currentStreak += 1; + } + } else { + state.stats.currentStreak = 1; + } + } else { + state.stats.currentStreak = 1; + } + state.stats.lastContributionDate = currentWeek; + if (state.stats.currentStreak > state.stats.longestStreak) { + state.stats.longestStreak = state.stats.currentStreak; + } + + await writeState(app, state); + + logger.info('Manual contribution credited', LOG_CONTEXT, { + contributionId, + repoSlug, + prNumber, + prUrl, + }); + + broadcastSymphonyUpdate(getMainWindow); + + return { contributionId }; + } + ) + ); + logger.info('Symphony handlers registered', LOG_CONTEXT); } diff --git a/src/main/preload/symphony.ts b/src/main/preload/symphony.ts index 65a3d853..896afda5 100644 --- a/src/main/preload/symphony.ts +++ b/src/main/preload/symphony.ts @@ -320,6 +320,28 @@ export function createSymphonyApi() { ): Promise<{ success: boolean; content?: string; error?: string }> => ipcRenderer.invoke('symphony:fetchDocumentContent', { url }), + manualCredit: (params: { + repoSlug: string; + repoName: string; + issueNumber: number; + issueTitle: string; + prNumber: number; + prUrl: string; + startedAt?: string; + completedAt?: string; + wasMerged?: boolean; + mergedAt?: string; + tokenUsage?: { + inputTokens?: number; + outputTokens?: number; + totalCost?: number; + }; + timeSpent?: number; + documentsProcessed?: number; + tasksCompleted?: number; + }): Promise<{ success: boolean; contributionId?: string; error?: string }> => + ipcRenderer.invoke('symphony:manualCredit', params), + // Real-time updates onUpdated: (callback: () => void) => { const handler = () => callback(); From 4ae5d86a0528745f5f1d0ec34cf8b9576da24d4f Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Tue, 3 Feb 2026 07:12:50 -0600 Subject: [PATCH 2/8] fix(tab-bar): ensure full tab visibility when scrolling into view Changed from centering the tab to using scrollIntoView with 'nearest' option. This ensures the entire tab including the close button is visible, rather than potentially cutting off the right edge when near container boundaries. --- .../renderer/components/TabBar.test.tsx | 86 +++++++++++++------ src/renderer/components/TabBar.tsx | 10 +-- 2 files changed, 67 insertions(+), 29 deletions(-) diff --git a/src/__tests__/renderer/components/TabBar.test.tsx b/src/__tests__/renderer/components/TabBar.test.tsx index 94115a13..cae12cf7 100644 --- a/src/__tests__/renderer/components/TabBar.test.tsx +++ b/src/__tests__/renderer/components/TabBar.test.tsx @@ -164,8 +164,9 @@ describe('TabBar', () => { beforeEach(() => { vi.useFakeTimers(); vi.clearAllMocks(); - // Mock scrollTo + // Mock scrollTo and scrollIntoView Element.prototype.scrollTo = vi.fn(); + Element.prototype.scrollIntoView = vi.fn(); // Mock clipboard Object.assign(navigator, { clipboard: { @@ -1430,13 +1431,13 @@ describe('TabBar', () => { }); describe('scroll behavior', () => { - it('scrolls to center active tab when activeTabId changes', async () => { + it('scrolls active tab into view when activeTabId changes', async () => { // Mock requestAnimationFrame const rafSpy = vi.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => { cb(0); return 0; }); - const scrollToSpy = vi.fn(); + const scrollIntoViewSpy = vi.fn(); const tabs = [ createTab({ id: 'tab-1', name: 'Tab 1' }), @@ -1454,9 +1455,11 @@ describe('TabBar', () => { /> ); - // Mock scrollTo on the container - const tabBarContainer = container.firstChild as HTMLElement; - tabBarContainer.scrollTo = scrollToSpy; + // Mock scrollIntoView on the tab elements + const tabElements = container.querySelectorAll('[data-tab-id]'); + tabElements.forEach((el) => { + (el as HTMLElement).scrollIntoView = scrollIntoViewSpy; + }); // Change active tab rerender( @@ -1470,19 +1473,29 @@ describe('TabBar', () => { /> ); - // scrollTo should have been called via requestAnimationFrame - expect(scrollToSpy).toHaveBeenCalled(); + // Re-mock scrollIntoView on tab elements after rerender + const newTabElements = container.querySelectorAll('[data-tab-id]'); + newTabElements.forEach((el) => { + (el as HTMLElement).scrollIntoView = scrollIntoViewSpy; + }); + + // scrollIntoView should have been called via requestAnimationFrame + expect(scrollIntoViewSpy).toHaveBeenCalledWith({ + inline: 'nearest', + behavior: 'smooth', + block: 'nearest', + }); rafSpy.mockRestore(); }); - it('scrolls to center active tab when showUnreadOnly filter is toggled off', async () => { + it('scrolls active tab into view when showUnreadOnly filter is toggled off', async () => { // Mock requestAnimationFrame const rafSpy = vi.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => { cb(0); return 0; }); - const scrollToSpy = vi.fn(); + const scrollIntoViewSpy = vi.fn(); const tabs = [ createTab({ id: 'tab-1', name: 'Tab 1' }), @@ -1502,12 +1515,14 @@ describe('TabBar', () => { /> ); - // Mock scrollTo on the container - const tabBarContainer = container.firstChild as HTMLElement; - tabBarContainer.scrollTo = scrollToSpy; + // Mock scrollIntoView on the tab elements + const tabElements = container.querySelectorAll('[data-tab-id]'); + tabElements.forEach((el) => { + (el as HTMLElement).scrollIntoView = scrollIntoViewSpy; + }); // Clear initial calls - scrollToSpy.mockClear(); + scrollIntoViewSpy.mockClear(); // Toggle filter off - this should trigger scroll to active tab rerender( @@ -1522,19 +1537,29 @@ describe('TabBar', () => { /> ); - // scrollTo should have been called when filter was toggled - expect(scrollToSpy).toHaveBeenCalled(); + // Re-mock scrollIntoView on tab elements after rerender + const newTabElements = container.querySelectorAll('[data-tab-id]'); + newTabElements.forEach((el) => { + (el as HTMLElement).scrollIntoView = scrollIntoViewSpy; + }); + + // scrollIntoView should have been called when filter was toggled + expect(scrollIntoViewSpy).toHaveBeenCalledWith({ + inline: 'nearest', + behavior: 'smooth', + block: 'nearest', + }); rafSpy.mockRestore(); }); - it('scrolls to center file tab when activeFileTabId changes', async () => { + it('scrolls file tab into view when activeFileTabId changes', async () => { // Mock requestAnimationFrame const rafSpy = vi.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => { cb(0); return 0; }); - const scrollToSpy = vi.fn(); + const scrollIntoViewSpy = vi.fn(); const tabs = [createTab({ id: 'tab-1', name: 'Tab 1' })]; const fileTab: FilePreviewTab = { @@ -1563,12 +1588,14 @@ describe('TabBar', () => { /> ); - // Mock scrollTo on the container - const tabBarContainer = container.firstChild as HTMLElement; - tabBarContainer.scrollTo = scrollToSpy; + // Mock scrollIntoView on the tab elements + const tabElements = container.querySelectorAll('[data-tab-id]'); + tabElements.forEach((el) => { + (el as HTMLElement).scrollIntoView = scrollIntoViewSpy; + }); // Clear initial calls - scrollToSpy.mockClear(); + scrollIntoViewSpy.mockClear(); // Select the file tab - this should trigger scroll to file tab rerender( @@ -1586,8 +1613,18 @@ describe('TabBar', () => { /> ); - // scrollTo should have been called when file tab was selected - expect(scrollToSpy).toHaveBeenCalled(); + // Re-mock scrollIntoView on tab elements after rerender + const newTabElements = container.querySelectorAll('[data-tab-id]'); + newTabElements.forEach((el) => { + (el as HTMLElement).scrollIntoView = scrollIntoViewSpy; + }); + + // scrollIntoView should have been called when file tab was selected + expect(scrollIntoViewSpy).toHaveBeenCalledWith({ + inline: 'nearest', + behavior: 'smooth', + block: 'nearest', + }); rafSpy.mockRestore(); }); @@ -1878,6 +1915,7 @@ describe('TabBar', () => { querySelector: vi.fn().mockReturnValue({ offsetLeft: 100, offsetWidth: 80, + scrollIntoView: vi.fn(), }), scrollTo: vi.fn(), }), diff --git a/src/renderer/components/TabBar.tsx b/src/renderer/components/TabBar.tsx index 010395eb..12b7c72c 100644 --- a/src/renderer/components/TabBar.tsx +++ b/src/renderer/components/TabBar.tsx @@ -1599,7 +1599,7 @@ function TabBarInner({ const tabRefs = useRef>(new Map()); const [isOverflowing, setIsOverflowing] = useState(false); - // Center the active tab in the scrollable area when activeTabId or activeFileTabId changes, or filter is toggled + // Ensure the active tab is fully visible (including close button) when activeTabId or activeFileTabId changes, or filter is toggled useEffect(() => { requestAnimationFrame(() => { const container = tabBarRef.current; @@ -1609,10 +1609,10 @@ function TabBarInner({ `[data-tab-id="${targetTabId}"]` ) as HTMLElement | null; if (container && tabElement) { - // Calculate scroll position to center the tab - const scrollLeft = - tabElement.offsetLeft - container.clientWidth / 2 + tabElement.offsetWidth / 2; - container.scrollTo({ left: scrollLeft, behavior: 'smooth' }); + // Use scrollIntoView with 'nearest' to ensure the full tab is visible + // This scrolls minimally - only if the tab is partially or fully out of view + // The 'end' option ensures the right edge (with close button) is visible + tabElement.scrollIntoView({ inline: 'nearest', behavior: 'smooth', block: 'nearest' }); } }); }, [activeTabId, activeFileTabId, showUnreadOnly]); From 3a8dd62a134d5ebb1184f08788cdb19684abe6b5 Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Tue, 3 Feb 2026 09:21:40 -0600 Subject: [PATCH 3/8] fix(ssh): remove heredoc approach for OpenCode prompts The heredoc syntax (cat << 'EOF' ... EOF) was breaking when passed through buildSshCommand's single-quote escaping. The '\'' escape pattern was being applied to the heredoc delimiters, producing invalid shell syntax like cat << '\''EOF'\''. Solution: Embed OpenCode prompts directly as positional arguments. The prompt will be properly escaped by buildRemoteCommand using shellEscape(), which handles the single-quote escaping correctly for bash -c command execution. This was the root cause of SSH remote execution failures with OpenCode - the OPENCODE_CONFIG_CONTENT env var escaping was correct, but the heredoc escaping was not. --- src/main/ipc/handlers/process.ts | 41 +++++++++++++------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/src/main/ipc/handlers/process.ts b/src/main/ipc/handlers/process.ts index b4c3cbc4..ec0d361d 100644 --- a/src/main/ipc/handlers/process.ts +++ b/src/main/ipc/handlers/process.ts @@ -263,7 +263,11 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void let useShell = false; let sshRemoteUsed: SshRemoteConfig | null = null; let customEnvVarsToPass: Record | undefined = effectiveCustomEnvVars; - let useHereDocForOpenCode = false; + // NOTE: We previously used heredoc for OpenCode prompts over SSH, but this approach + // failed because the heredoc syntax (cat << 'EOF' ... EOF) doesn't survive the + // single-quote escaping in buildSshCommand. Now we embed the prompt directly + // in the args and let buildRemoteCommand handle escaping. + // See: https://github.com/pedramamini/Maestro/issues/XXX if (config.sessionCustomPath) { logger.debug(`Using session-level custom path for ${config.toolType}`, LOG_CONTEXT, { @@ -362,10 +366,17 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void hasStreamJsonInput, }); } else if (config.prompt && !agentSupportsStreamJson) { - // Agent doesn't support stream-json - use alternative methods + // Agent doesn't support stream-json - embed prompt in command line args + // For OpenCode: prompt is a positional argument (no --p flag, no -- separator) + // For other agents: send via stdin as raw text (if they support it) if (config.toolType === 'opencode') { - // OpenCode: mark for here document processing (will be handled after remoteCommand is set) - useHereDocForOpenCode = true; + // OpenCode: add prompt as positional argument + // buildRemoteCommand will properly escape it with shellEscape() + sshArgs = [...sshArgs, config.prompt]; + logger.info(`Embedding prompt in OpenCode command args for SSH`, LOG_CONTEXT, { + sessionId: config.sessionId, + promptLength: config.prompt?.length, + }); } else { // Other agents: send via stdin as raw text shouldSendPromptViaStdinRaw = true; @@ -377,27 +388,7 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void // 2. Otherwise, use the agent's binaryName (e.g., 'codex', 'claude') and let // the remote shell's PATH resolve it. This avoids using local paths like // '/opt/homebrew/bin/codex' which don't exist on the remote host. - let remoteCommand = config.sessionCustomPath || agent?.binaryName || config.command; - - // Handle OpenCode here document for large prompts - if (useHereDocForOpenCode && config.prompt) { - // OpenCode: use here document to avoid command line limits - // Escape single quotes in the prompt for bash here document - const escapedPrompt = config.prompt.replace(/'/g, "'\\''"); - // Construct: cat << 'EOF' | opencode run --format json\nlong prompt here\nEOF - const hereDocCommand = `cat << 'EOF' | ${remoteCommand} ${sshArgs.join(' ')}\n${escapedPrompt}\nEOF`; - sshArgs = []; // Clear args since they're now in the here doc command - remoteCommand = hereDocCommand; // Update to use here document - logger.info( - `Using here document for large OpenCode prompt to avoid command line limits`, - LOG_CONTEXT, - { - sessionId: config.sessionId, - promptLength: config.prompt?.length, - commandLength: hereDocCommand.length, - } - ); - } + const remoteCommand = config.sessionCustomPath || agent?.binaryName || config.command; // Decide whether we'll send input via stdin to the remote command const useStdin = sshArgs.includes('--input-format') && sshArgs.includes('stream-json'); From 07df61fbcf69e563dabea18c5e46ffecbdb18f11 Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Tue, 3 Feb 2026 09:34:16 -0600 Subject: [PATCH 4/8] fix(ssh): use stdin-based execution to bypass shell escaping issues This is a complete rewrite of SSH remote command execution that eliminates all shell escaping issues by sending the entire script via stdin. Previously, the SSH command was built as: ssh host '/bin/bash -c '\''cd /path && VAR='\''value'\'' cmd arg'\''' This required complex nested escaping that broke with: - Heredocs (cat << 'EOF') - Long prompts (command line length limits) - Special characters in prompts Now the SSH command is simply: ssh host /bin/bash And the entire script is piped via stdin: export PATH="$HOME/.local/bin:..." cd '/project/path' export OPENCODE_CONFIG_CONTENT='{"permission":...}' exec opencode run --format json 'prompt here' Benefits: - No shell escaping layers (stdin is binary-safe) - No command line length limits - Works with any remote shell (bash, zsh, fish) - Handles any prompt content (quotes, newlines, $, etc.) - Much simpler to debug and maintain Changes: - Add buildSshCommandWithStdin() in ssh-command-builder.ts - Update process.ts to use stdin-based SSH for all agents - Add sshStdinScript to ProcessConfig type - Update ChildProcessSpawner to send stdin script - Add comprehensive tests for new function --- .../main/utils/ssh-command-builder.test.ts | 157 +++++++++++++++++- src/main/ipc/handlers/process.ts | 131 +++------------ .../spawners/ChildProcessSpawner.ts | 13 +- src/main/process-manager/types.ts | 2 + src/main/utils/ssh-command-builder.ts | 129 ++++++++++++++ 5 files changed, 323 insertions(+), 109 deletions(-) diff --git a/src/__tests__/main/utils/ssh-command-builder.test.ts b/src/__tests__/main/utils/ssh-command-builder.test.ts index 4e9ffddb..3a0f7fa1 100644 --- a/src/__tests__/main/utils/ssh-command-builder.test.ts +++ b/src/__tests__/main/utils/ssh-command-builder.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { buildSshCommand, buildRemoteCommand } from '../../../main/utils/ssh-command-builder'; +import { + buildSshCommand, + buildRemoteCommand, + buildSshCommandWithStdin, +} from '../../../main/utils/ssh-command-builder'; import type { SshRemoteConfig } from '../../../shared/types'; import * as os from 'os'; @@ -660,4 +664,155 @@ describe('ssh-command-builder', () => { expect(remoteCommand).toContain('line3'); }); }); + + describe('buildSshCommandWithStdin', () => { + /** + * Tests for the stdin-based SSH execution approach. + * This method completely bypasses shell escaping issues by: + * 1. SSH connects and runs /bin/bash on the remote + * 2. The entire script (PATH, cd, env, command) is sent via stdin + * 3. No command-line argument parsing/escaping occurs + */ + + it('returns ssh command with /bin/bash as remote command', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run', '--format', 'json'], + }); + + expect(result.command).toBe('ssh'); + // Last arg should be /bin/bash (the remote command) + expect(result.args[result.args.length - 1]).toBe('/bin/bash'); + }); + + it('includes PATH setup in stdin script', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + }); + + expect(result.stdinScript).toBeDefined(); + expect(result.stdinScript).toContain('export PATH='); + expect(result.stdinScript).toContain('.local/bin'); + expect(result.stdinScript).toContain('/opt/homebrew/bin'); + }); + + it('includes cd command in stdin script when cwd provided', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + cwd: '/home/user/project', + }); + + expect(result.stdinScript).toContain("cd '/home/user/project'"); + }); + + it('includes environment variables in stdin script', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + env: { + OPENCODE_CONFIG_CONTENT: '{"permission":{"*":"allow"},"tools":{"question":false}}', + CUSTOM_VAR: 'test-value', + }, + }); + + expect(result.stdinScript).toContain('export OPENCODE_CONFIG_CONTENT='); + expect(result.stdinScript).toContain('export CUSTOM_VAR='); + // The JSON should be in the script (escaped with single quotes) + expect(result.stdinScript).toContain('question'); + }); + + it('includes prompt as final argument in stdin script', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run', '--format', 'json'], + prompt: 'Write hello world to a file', + }); + + expect(result.stdinScript).toContain('opencode'); + expect(result.stdinScript).toContain('Write hello world to a file'); + }); + + it('handles prompts with special characters without escaping issues', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + prompt: "What's the $PATH? Use `echo` and \"quotes\"", + }); + + // The script should contain the prompt (escaped for bash) + expect(result.stdinScript).toBeDefined(); + // Single quotes in the prompt should be escaped + expect(result.stdinScript).toContain("'\\''"); + }); + + it('handles multi-line prompts', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + prompt: 'Line 1\nLine 2\nLine 3', + }); + + expect(result.stdinScript).toContain('Line 1'); + expect(result.stdinScript).toContain('Line 2'); + expect(result.stdinScript).toContain('Line 3'); + }); + + it('uses exec to replace shell with command', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + }); + + // The script should use exec to replace the shell process + expect(result.stdinScript).toContain('exec '); + }); + + it('includes SSH options in args', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + }); + + expect(result.args).toContain('-o'); + expect(result.args).toContain('BatchMode=yes'); + expect(result.args).toContain('StrictHostKeyChecking=accept-new'); + }); + + it('includes private key when provided', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + }); + + expect(result.args).toContain('-i'); + expect(result.args).toContain('/Users/testuser/.ssh/id_ed25519'); + }); + + it('includes username@host destination', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + }); + + expect(result.args).toContain('testuser@dev.example.com'); + }); + + it('merges remote config env with option env', async () => { + const configWithEnv = { + ...baseConfig, + remoteEnv: { REMOTE_VAR: 'from-config' }, + }; + + const result = await buildSshCommandWithStdin(configWithEnv, { + command: 'opencode', + args: ['run'], + env: { OPTION_VAR: 'from-option' }, + }); + + expect(result.stdinScript).toContain('export REMOTE_VAR='); + expect(result.stdinScript).toContain('export OPTION_VAR='); + }); + }); }); diff --git a/src/main/ipc/handlers/process.ts b/src/main/ipc/handlers/process.ts index ec0d361d..02b8cc9b 100644 --- a/src/main/ipc/handlers/process.ts +++ b/src/main/ipc/handlers/process.ts @@ -17,7 +17,7 @@ import { CreateHandlerOptions, } from '../../utils/ipcHandler'; import { getSshRemoteConfig, createSshRemoteStoreAdapter } from '../../utils/ssh-remote-resolver'; -import { buildSshCommand } from '../../utils/ssh-command-builder'; +import { buildSshCommandWithStdin } from '../../utils/ssh-command-builder'; import { buildExpandedEnv } from '../../../shared/pathUtils'; import type { SshRemoteConfig } from '../../../shared/types'; import { powerManager } from '../../power-manager'; @@ -263,11 +263,7 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void let useShell = false; let sshRemoteUsed: SshRemoteConfig | null = null; let customEnvVarsToPass: Record | undefined = effectiveCustomEnvVars; - // NOTE: We previously used heredoc for OpenCode prompts over SSH, but this approach - // failed because the heredoc syntax (cat << 'EOF' ... EOF) doesn't survive the - // single-quote escaping in buildSshCommand. Now we embed the prompt directly - // in the args and let buildRemoteCommand handle escaping. - // See: https://github.com/pedramamini/Maestro/issues/XXX + let sshStdinScript: string | undefined; if (config.sessionCustomPath) { logger.debug(`Using session-level custom path for ${config.toolType}`, LOG_CONTEXT, { @@ -323,8 +319,6 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void willUseSsh: config.toolType !== 'terminal' && config.sessionSshRemoteConfig?.enabled, }); } - let shouldSendPromptViaStdin = false; - let shouldSendPromptViaStdinRaw = false; if (config.toolType !== 'terminal' && config.sessionSshRemoteConfig?.enabled) { // Session-level SSH config provided - resolve and use it logger.info(`Using session-level SSH config`, LOG_CONTEXT, { @@ -340,108 +334,40 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void }); if (sshResult.config) { - // SSH remote is configured - wrap the command for remote execution + // SSH remote is configured - use stdin-based execution + // This completely bypasses shell escaping issues by sending the script via stdin sshRemoteUsed = sshResult.config; - // ALWAYS use stdin for SSH remote execution when there's a prompt. - // Embedding prompts in the command line causes shell escaping nightmares: - // - Multiple layers of quote escaping (local spawn, SSH, remote zsh, bash -c) - // - Embedded newlines in prompts break zsh parsing (e.g., "zsh:35: parse error") - // - Special characters like quotes, $, !, etc. need complex escaping - // Using stdin with --input-format stream-json completely bypasses all these issues. - const hasStreamJsonInput = - finalArgs.includes('--input-format') && finalArgs.includes('stream-json'); - const agentSupportsStreamJson = agent?.capabilities.supportsStreamJsonInput ?? false; - let sshArgs = finalArgs; - if (config.prompt && agentSupportsStreamJson) { - // Agent supports stream-json - always use stdin for prompts - if (!hasStreamJsonInput) { - sshArgs = [...finalArgs, '--input-format', 'stream-json']; - } - shouldSendPromptViaStdin = true; - logger.info(`Using stdin for prompt in SSH remote execution`, LOG_CONTEXT, { - sessionId: config.sessionId, - promptLength: config.prompt?.length, - reason: 'ssh-stdin-for-reliability', - hasStreamJsonInput, - }); - } else if (config.prompt && !agentSupportsStreamJson) { - // Agent doesn't support stream-json - embed prompt in command line args - // For OpenCode: prompt is a positional argument (no --p flag, no -- separator) - // For other agents: send via stdin as raw text (if they support it) - if (config.toolType === 'opencode') { - // OpenCode: add prompt as positional argument - // buildRemoteCommand will properly escape it with shellEscape() - sshArgs = [...sshArgs, config.prompt]; - logger.info(`Embedding prompt in OpenCode command args for SSH`, LOG_CONTEXT, { - sessionId: config.sessionId, - promptLength: config.prompt?.length, - }); - } else { - // Other agents: send via stdin as raw text - shouldSendPromptViaStdinRaw = true; - } - } - // - // Determine the command to run on the remote host: - // 1. If user set a session-specific custom path, use that (they configured it for the remote) - // 2. Otherwise, use the agent's binaryName (e.g., 'codex', 'claude') and let - // the remote shell's PATH resolve it. This avoids using local paths like - // '/opt/homebrew/bin/codex' which don't exist on the remote host. + // Determine the command to run on the remote host const remoteCommand = config.sessionCustomPath || agent?.binaryName || config.command; - // Decide whether we'll send input via stdin to the remote command - const useStdin = sshArgs.includes('--input-format') && sshArgs.includes('stream-json'); - const sshCommand = await buildSshCommand(sshResult.config, { + // Build the SSH command with stdin script + // The script contains PATH setup, cd, env vars, and the actual command + // This eliminates all shell escaping issues + const sshCommand = await buildSshCommandWithStdin(sshResult.config, { command: remoteCommand, - args: sshArgs, - // Use the cwd from config - this is the project directory on the remote + args: finalArgs, cwd: config.cwd, - // Pass custom environment variables to the remote command env: effectiveCustomEnvVars, - // Explicitly indicate whether stdin will be used so ssh-command-builder - // can avoid forcing a TTY for stream-json modes. - useStdin, + prompt: config.prompt, // Prompt is included in the stdin script }); commandToSpawn = sshCommand.command; argsToSpawn = sshCommand.args; + sshStdinScript = sshCommand.stdinScript; - // For SSH, env vars are passed in the remote command string, not locally + // For SSH, env vars are passed in the stdin script, not locally customEnvVarsToPass = undefined; - // On Windows, use PowerShell for SSH commands to avoid cmd.exe's 8191 character limit - // PowerShell supports up to 32,767 characters, which is needed for large prompts - if (isWindows) { - useShell = true; - shellToUse = 'powershell.exe'; - logger.info( - `Using PowerShell for SSH command on Windows to support long command lines`, - LOG_CONTEXT, - { - sessionId: config.sessionId, - commandLength: sshCommand.args.join(' ').length, - } - ); - } - - // Detailed debug logging to diagnose SSH command execution issues - logger.debug(`SSH command details for debugging`, LOG_CONTEXT, { + logger.info(`SSH command built with stdin script`, LOG_CONTEXT, { sessionId: config.sessionId, toolType: config.toolType, sshBinary: sshCommand.command, sshArgsCount: sshCommand.args.length, - sshArgsArray: sshCommand.args, - // Show the last arg which contains the wrapped remote command - remoteCommandString: sshCommand.args[sshCommand.args.length - 1], - // Show the agent command that will execute remotely - agentBinary: remoteCommand, - agentArgs: sshArgs, - agentCwd: config.cwd, - // Full invocation for copy-paste debugging - fullSshInvocation: `${sshCommand.command} ${sshCommand.args - .map((arg) => (arg.includes(' ') ? `'${arg}'` : arg)) - .join(' ')}`, + remoteCommand, + remoteCwd: config.cwd, + promptLength: config.prompt?.length, + scriptLength: sshCommand.stdinScript?.length, }); } } @@ -460,38 +386,31 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void command: commandToSpawn, args: argsToSpawn, // When using SSH, use user's home directory as local cwd - // The remote working directory is embedded in the SSH command itself + // The remote working directory is embedded in the SSH stdin script // This fixes ENOENT errors when session.cwd is a remote-only path cwd: sshRemoteUsed ? os.homedir() : config.cwd, // When using SSH, disable PTY (SSH provides its own terminal handling) - // and env vars are passed via the remote command string requiresPty: sshRemoteUsed ? false : agent?.requiresPty, - // When using SSH with small prompts, the prompt was already added to sshArgs above - // For large prompts or stream-json input, pass it to ProcessManager so it can send via stdin - prompt: - sshRemoteUsed && config.prompt && shouldSendPromptViaStdin - ? config.prompt - : sshRemoteUsed - ? undefined - : config.prompt, + // For SSH, prompt is included in the stdin script, not passed separately + // For local execution, pass prompt as normal + prompt: sshRemoteUsed ? undefined : config.prompt, shell: shellToUse, runInShell: useShell, shellArgs: shellArgsStr, // Shell-specific CLI args (for terminal sessions) shellEnvVars: shellEnvVars, // Shell-specific env vars (for terminal sessions) contextWindow, // Pass configured context window to process manager - // When using SSH, env vars are passed in the remote command string, not locally + // When using SSH, env vars are passed in the stdin script, not locally customEnvVars: customEnvVarsToPass, imageArgs: agent?.imageArgs, // Function to build image CLI args (for Codex, OpenCode) promptArgs: agent?.promptArgs, // Function to build prompt args (e.g., ['-p', prompt] for OpenCode) noPromptSeparator: agent?.noPromptSeparator, // Some agents don't support '--' before prompt - // For SSH with stream-json input, send prompt via stdin instead of command line - sendPromptViaStdin: shouldSendPromptViaStdin ? true : undefined, - sendPromptViaStdinRaw: shouldSendPromptViaStdinRaw ? true : undefined, // Stats tracking: use cwd as projectPath if not explicitly provided projectPath: config.cwd, // SSH remote context (for SSH-specific error messages) sshRemoteId: sshRemoteUsed?.id, sshRemoteHost: sshRemoteUsed?.host, + // SSH stdin script - the entire command is sent via stdin to /bin/bash on remote + sshStdinScript, }); logger.info(`Process spawned successfully`, LOG_CONTEXT, { diff --git a/src/main/process-manager/spawners/ChildProcessSpawner.ts b/src/main/process-manager/spawners/ChildProcessSpawner.ts index 5bfad408..4391366f 100644 --- a/src/main/process-manager/spawners/ChildProcessSpawner.ts +++ b/src/main/process-manager/spawners/ChildProcessSpawner.ts @@ -397,8 +397,17 @@ export class ChildProcessSpawner { this.exitHandler.handleError(sessionId, error); }); - // Handle stdin for batch mode and stream-json - if (isStreamJsonMode && prompt) { + // Handle stdin for SSH script, stream-json, or batch mode + if (config.sshStdinScript) { + // SSH stdin script mode: send the entire script to /bin/bash on remote + // This bypasses all shell escaping issues by piping the script via stdin + logger.debug('[ProcessManager] Sending SSH stdin script', 'ProcessManager', { + sessionId, + scriptLength: config.sshStdinScript.length, + }); + childProcess.stdin?.write(config.sshStdinScript); + childProcess.stdin?.end(); + } else if (isStreamJsonMode && prompt) { if (config.sendPromptViaStdinRaw) { // Send raw prompt via stdin logger.debug('[ProcessManager] Sending raw prompt via stdin', 'ProcessManager', { diff --git a/src/main/process-manager/types.ts b/src/main/process-manager/types.ts index e05e219e..6b2ccb03 100644 --- a/src/main/process-manager/types.ts +++ b/src/main/process-manager/types.ts @@ -34,6 +34,8 @@ export interface ProcessConfig { sendPromptViaStdin?: boolean; /** If true, send the prompt via stdin as raw text instead of command line */ sendPromptViaStdinRaw?: boolean; + /** Script to send via stdin for SSH execution (bypasses shell escaping) */ + sshStdinScript?: string; } /** diff --git a/src/main/utils/ssh-command-builder.ts b/src/main/utils/ssh-command-builder.ts index 4ac58dd8..c129954d 100644 --- a/src/main/utils/ssh-command-builder.ts +++ b/src/main/utils/ssh-command-builder.ts @@ -22,6 +22,8 @@ export interface SshCommandResult { command: string; /** Arguments for the SSH command */ args: string[]; + /** Script to send via stdin (for stdin-based execution) */ + stdinScript?: string; } /** @@ -132,6 +134,133 @@ export function buildRemoteCommand(options: RemoteCommandOptions): string { return parts.join(' && '); } +/** + * Build an SSH command that executes a script via stdin. + * + * This approach completely bypasses shell escaping issues by: + * 1. SSH connects and runs `/bin/bash` on the remote + * 2. The script (with PATH setup, cd, env vars, command) is sent via stdin + * 3. No shell parsing of command-line arguments occurs + * + * This is the preferred method for SSH remote execution as it: + * - Handles any prompt content (special chars, newlines, quotes, etc.) + * - Avoids command-line length limits + * - Works regardless of the remote user's login shell (bash, zsh, fish, etc.) + * - Eliminates the escaping nightmare of nested shell contexts + * + * @param config SSH remote configuration + * @param remoteOptions Options for the remote command + * @returns SSH command/args plus the script to send via stdin + * + * @example + * const result = await buildSshCommandWithStdin(config, { + * command: 'opencode', + * args: ['run', '--format', 'json'], + * cwd: '/home/user/project', + * env: { OPENCODE_CONFIG_CONTENT: '{"permission":{"*":"allow"}}' }, + * prompt: 'Write hello world to a file' + * }); + * // result.command = 'ssh' + * // result.args = ['-o', 'BatchMode=yes', ..., 'user@host', '/bin/bash'] + * // result.stdinScript = '#!/bin/bash\nexport PATH=...\ncd /home/user/project\nOPENCODE_CONFIG_CONTENT=... opencode run --format json "Write hello world to a file"\n' + */ +export async function buildSshCommandWithStdin( + config: SshRemoteConfig, + remoteOptions: RemoteCommandOptions & { prompt?: string } +): Promise { + const args: string[] = []; + + // Resolve the SSH binary path + const sshPath = await resolveSshPath(); + + // For stdin-based execution, we never need TTY (stdin is the script, not user input) + // TTY would interfere with piping the script + + // Private key - only add if explicitly provided + if (config.privateKeyPath && config.privateKeyPath.trim()) { + args.push('-i', expandTilde(config.privateKeyPath)); + } + + // Default SSH options - but RequestTTY is always 'no' for stdin mode + for (const [key, value] of Object.entries(DEFAULT_SSH_OPTIONS)) { + args.push('-o', `${key}=${value}`); + } + + // Port specification + if (!config.useSshConfig || config.port !== 22) { + args.push('-p', config.port.toString()); + } + + // Build destination + if (config.username && config.username.trim()) { + args.push(`${config.username}@${config.host}`); + } else { + args.push(config.host); + } + + // The remote command is just /bin/bash - it will read the script from stdin + args.push('/bin/bash'); + + // Build the script to send via stdin + const scriptLines: string[] = []; + + // PATH setup - same directories as before + scriptLines.push( + 'export PATH="$HOME/.local/bin:$HOME/bin:/usr/local/bin:/opt/homebrew/bin:$HOME/.cargo/bin:$PATH"' + ); + + // Change directory if specified + if (remoteOptions.cwd) { + // In the script context, we can use simple quoting + scriptLines.push(`cd ${shellEscape(remoteOptions.cwd)} || exit 1`); + } + + // Merge environment variables + const mergedEnv: Record = { + ...(config.remoteEnv || {}), + ...(remoteOptions.env || {}), + }; + + // Export environment variables + for (const [key, value] of Object.entries(mergedEnv)) { + if (/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(key)) { + scriptLines.push(`export ${key}=${shellEscape(value)}`); + } + } + + // Build the command line + // For the script, we use simple quoting since we're not going through shell parsing layers + const cmdParts = [remoteOptions.command, ...remoteOptions.args.map((arg) => shellEscape(arg))]; + + // Add prompt as final argument if provided + if (remoteOptions.prompt) { + cmdParts.push(shellEscape(remoteOptions.prompt)); + } + + // Use exec to replace the shell with the command (cleaner process tree) + scriptLines.push(`exec ${cmdParts.join(' ')}`); + + const stdinScript = scriptLines.join('\n') + '\n'; + + logger.info('SSH command built with stdin script', '[ssh-command-builder]', { + host: config.host, + username: config.username || '(using SSH config/system default)', + port: config.port, + sshPath, + sshArgsCount: args.length, + scriptLineCount: scriptLines.length, + scriptLength: stdinScript.length, + // Show first part of script for debugging (truncate if long) + scriptPreview: stdinScript.length > 500 ? stdinScript.substring(0, 500) + '...' : stdinScript, + }); + + return { + command: sshPath, + args, + stdinScript, + }; +} + /** * Build SSH command and arguments for remote execution. * From fc7880cc32a56820602cacdfe59851bf46e5e8b2 Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Tue, 3 Feb 2026 09:57:05 -0600 Subject: [PATCH 5/8] feat(ssh): use heredoc for stdin prompts to avoid CLI length limits IMPORTANT: Prompts must be passed via stdin to avoid CLI argument length limits. Prompts can be huge and contain arbitrary characters that would break if passed as command-line arguments. Changes: - Add stdinInput parameter to buildSshCommandWithStdin for heredoc-based prompt delivery - Use MAESTRO_PROMPT_EOF delimiter with collision detection (appends _N suffix if prompt contains the delimiter) - OpenCode prompts now always sent via stdin heredoc, not CLI args - Add comprehensive tests for heredoc behavior and delimiter collision - Add comment in process.ts documenting this requirement to prevent regressions The heredoc approach: exec opencode 'run' <<'MAESTRO_PROMPT_EOF' ensures prompts of any size with any characters work correctly. --- .../main/utils/ssh-command-builder.test.ts | 51 +++++++++++++++---- src/main/ipc/handlers/process.ts | 9 +++- src/main/utils/ssh-command-builder.ts | 27 +++++++--- 3 files changed, 70 insertions(+), 17 deletions(-) diff --git a/src/__tests__/main/utils/ssh-command-builder.test.ts b/src/__tests__/main/utils/ssh-command-builder.test.ts index 3a0f7fa1..7e13f4a1 100644 --- a/src/__tests__/main/utils/ssh-command-builder.test.ts +++ b/src/__tests__/main/utils/ssh-command-builder.test.ts @@ -723,35 +723,38 @@ describe('ssh-command-builder', () => { expect(result.stdinScript).toContain('question'); }); - it('includes prompt as final argument in stdin script', async () => { + it('includes prompt via stdin heredoc when stdinInput provided', async () => { const result = await buildSshCommandWithStdin(baseConfig, { command: 'opencode', args: ['run', '--format', 'json'], - prompt: 'Write hello world to a file', + stdinInput: 'Write hello world to a file', }); - expect(result.stdinScript).toContain('opencode'); + const execLine = result.stdinScript + ?.split('\n') + .find((line) => line.startsWith('exec ')); + expect(execLine).toBe("exec opencode 'run' '--format' 'json' <<'MAESTRO_PROMPT_EOF'"); expect(result.stdinScript).toContain('Write hello world to a file'); + expect(result.stdinScript).toContain('MAESTRO_PROMPT_EOF'); }); - it('handles prompts with special characters without escaping issues', async () => { + it('handles stdin prompts with special characters without escaping issues', async () => { const result = await buildSshCommandWithStdin(baseConfig, { command: 'opencode', args: ['run'], - prompt: "What's the $PATH? Use `echo` and \"quotes\"", + stdinInput: "What's the $PATH? Use `echo` and \"quotes\"", }); - // The script should contain the prompt (escaped for bash) + // The script should contain the prompt verbatim (no shell interpolation in heredoc) expect(result.stdinScript).toBeDefined(); - // Single quotes in the prompt should be escaped - expect(result.stdinScript).toContain("'\\''"); + expect(result.stdinScript).toContain("What's the $PATH? Use `echo` and \"quotes\""); }); - it('handles multi-line prompts', async () => { + it('handles multi-line stdin prompts', async () => { const result = await buildSshCommandWithStdin(baseConfig, { command: 'opencode', args: ['run'], - prompt: 'Line 1\nLine 2\nLine 3', + stdinInput: 'Line 1\nLine 2\nLine 3', }); expect(result.stdinScript).toContain('Line 1'); @@ -759,6 +762,34 @@ describe('ssh-command-builder', () => { expect(result.stdinScript).toContain('Line 3'); }); + it('uses a unique heredoc delimiter when prompt contains the default token', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + stdinInput: 'Line with MAESTRO_PROMPT_EOF inside', + }); + + const execLine = result.stdinScript + ?.split('\n') + .find((line) => line.startsWith('exec ')); + expect(execLine).toContain("<<'MAESTRO_PROMPT_EOF_0'"); + expect(result.stdinScript).toContain('Line with MAESTRO_PROMPT_EOF inside'); + }); + + it('includes prompt as final argument when stdinInput is not provided', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + prompt: "Say 'hello'", + }); + + const execLine = result.stdinScript + ?.split('\n') + .find((line) => line.startsWith('exec ')); + // The prompt is escaped with single quotes - "Say 'hello'" becomes "'Say '\\''hello'\\''" + expect(execLine).toContain("opencode 'run' 'Say '\\''hello'\\'''"); + }); + it('uses exec to replace shell with command', async () => { const result = await buildSshCommandWithStdin(baseConfig, { command: 'opencode', diff --git a/src/main/ipc/handlers/process.ts b/src/main/ipc/handlers/process.ts index 02b8cc9b..1eb88b09 100644 --- a/src/main/ipc/handlers/process.ts +++ b/src/main/ipc/handlers/process.ts @@ -344,12 +344,19 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void // Build the SSH command with stdin script // The script contains PATH setup, cd, env vars, and the actual command // This eliminates all shell escaping issues + // + // IMPORTANT: OpenCode prompts must be passed via stdin to avoid CLI length limits. + // Prompts can be huge and contain arbitrary characters; do NOT pass them as argv. + const shouldSendPromptViaStdin = config.toolType === 'opencode' && !!config.prompt; + const promptForArgs = shouldSendPromptViaStdin ? undefined : config.prompt; + const stdinInput = shouldSendPromptViaStdin ? config.prompt : undefined; const sshCommand = await buildSshCommandWithStdin(sshResult.config, { command: remoteCommand, args: finalArgs, cwd: config.cwd, env: effectiveCustomEnvVars, - prompt: config.prompt, // Prompt is included in the stdin script + prompt: promptForArgs, + stdinInput, }); commandToSpawn = sshCommand.command; diff --git a/src/main/utils/ssh-command-builder.ts b/src/main/utils/ssh-command-builder.ts index c129954d..632b2116 100644 --- a/src/main/utils/ssh-command-builder.ts +++ b/src/main/utils/ssh-command-builder.ts @@ -158,15 +158,15 @@ export function buildRemoteCommand(options: RemoteCommandOptions): string { * args: ['run', '--format', 'json'], * cwd: '/home/user/project', * env: { OPENCODE_CONFIG_CONTENT: '{"permission":{"*":"allow"}}' }, - * prompt: 'Write hello world to a file' + * stdinInput: 'Write hello world to a file' * }); * // result.command = 'ssh' * // result.args = ['-o', 'BatchMode=yes', ..., 'user@host', '/bin/bash'] - * // result.stdinScript = '#!/bin/bash\nexport PATH=...\ncd /home/user/project\nOPENCODE_CONFIG_CONTENT=... opencode run --format json "Write hello world to a file"\n' + * // result.stdinScript = '#!/bin/bash\nexport PATH=...\ncd /home/user/project\nOPENCODE_CONFIG_CONTENT=...\nexec opencode run --format json <<'MAESTRO_PROMPT_EOF'\nWrite hello world to a file\nMAESTRO_PROMPT_EOF\n' */ export async function buildSshCommandWithStdin( config: SshRemoteConfig, - remoteOptions: RemoteCommandOptions & { prompt?: string } + remoteOptions: RemoteCommandOptions & { prompt?: string; stdinInput?: string } ): Promise { const args: string[] = []; @@ -232,13 +232,28 @@ export async function buildSshCommandWithStdin( // For the script, we use simple quoting since we're not going through shell parsing layers const cmdParts = [remoteOptions.command, ...remoteOptions.args.map((arg) => shellEscape(arg))]; - // Add prompt as final argument if provided - if (remoteOptions.prompt) { + // Add prompt as final argument if provided and not sending via stdin + const hasStdinInput = remoteOptions.stdinInput !== undefined; + if (remoteOptions.prompt && !hasStdinInput) { cmdParts.push(shellEscape(remoteOptions.prompt)); } // Use exec to replace the shell with the command (cleaner process tree) - scriptLines.push(`exec ${cmdParts.join(' ')}`); + if (hasStdinInput) { + // IMPORTANT: Prompts must be passed via stdin to avoid CLI length limits. + // Build a safe heredoc delimiter that won't collide with the prompt content. + const delimiterBase = 'MAESTRO_PROMPT_EOF'; + let delimiter = delimiterBase; + let counter = 0; + while (remoteOptions.stdinInput?.includes(delimiter)) { + delimiter = `${delimiterBase}_${counter++}`; + } + scriptLines.push(`exec ${cmdParts.join(' ')} <<'${delimiter}'`); + scriptLines.push(remoteOptions.stdinInput ?? ''); + scriptLines.push(delimiter); + } else { + scriptLines.push(`exec ${cmdParts.join(' ')}`); + } const stdinScript = scriptLines.join('\n') + '\n'; From ccabe7524899afc96dc94f3c2ebdc21f4ebd3f84 Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Tue, 3 Feb 2026 15:13:55 -0600 Subject: [PATCH 6/8] refactor(ssh): simplify stdin prompt delivery with passthrough approach Replace heredoc-based prompt delivery with simpler stdin passthrough. How it works: 1. Bash script is sent via stdin to /bin/bash on the remote 2. Script sets up PATH, cd, env vars, then calls `exec ` 3. The `exec` replaces bash with the agent process 4. The agent inherits stdin and reads the remaining content (the prompt) Benefits: - No heredoc syntax needed - No delimiter collision detection - No prompt escaping required - prompt is never parsed by any shell - Works with any prompt content (quotes, newlines, $, backticks, etc.) - Simpler, more maintainable code Changes: - Remove heredoc logic from buildSshCommandWithStdin() - Update process.ts to use stdin passthrough for ALL agents over SSH (not just OpenCode - all agents benefit from this approach) - Update tests to verify stdin passthrough behavior Verified locally that both OpenCode and Claude Code read prompts from stdin. --- .../main/utils/ssh-command-builder.test.ts | 71 +++++++++++++++---- src/main/ipc/handlers/process.ts | 21 +++--- src/main/utils/ssh-command-builder.ts | 48 +++++++------ 3 files changed, 97 insertions(+), 43 deletions(-) diff --git a/src/__tests__/main/utils/ssh-command-builder.test.ts b/src/__tests__/main/utils/ssh-command-builder.test.ts index 7e13f4a1..f900a446 100644 --- a/src/__tests__/main/utils/ssh-command-builder.test.ts +++ b/src/__tests__/main/utils/ssh-command-builder.test.ts @@ -668,10 +668,18 @@ describe('ssh-command-builder', () => { describe('buildSshCommandWithStdin', () => { /** * Tests for the stdin-based SSH execution approach. + * * This method completely bypasses shell escaping issues by: * 1. SSH connects and runs /bin/bash on the remote - * 2. The entire script (PATH, cd, env, command) is sent via stdin - * 3. No command-line argument parsing/escaping occurs + * 2. The script (PATH, cd, env, exec command) is sent via stdin + * 3. The prompt is appended after the script and passed through to the exec'd command + * 4. No heredoc, no delimiter collision detection, no prompt escaping needed + * + * How it works: + * - Bash reads the script lines from stdin + * - The `exec` command replaces bash with the target process + * - The target process inherits stdin and reads the remaining content (the prompt) + * - The prompt is NEVER parsed by any shell - it flows through as raw bytes */ it('returns ssh command with /bin/bash as remote command', async () => { @@ -723,31 +731,44 @@ describe('ssh-command-builder', () => { expect(result.stdinScript).toContain('question'); }); - it('includes prompt via stdin heredoc when stdinInput provided', async () => { + it('appends prompt after exec command via stdin passthrough', async () => { const result = await buildSshCommandWithStdin(baseConfig, { command: 'opencode', args: ['run', '--format', 'json'], stdinInput: 'Write hello world to a file', }); + // The exec line should NOT have heredoc - just the command const execLine = result.stdinScript ?.split('\n') .find((line) => line.startsWith('exec ')); - expect(execLine).toBe("exec opencode 'run' '--format' 'json' <<'MAESTRO_PROMPT_EOF'"); + expect(execLine).toBe("exec opencode 'run' '--format' 'json'"); + + // The prompt should appear after the exec line (stdin passthrough) expect(result.stdinScript).toContain('Write hello world to a file'); - expect(result.stdinScript).toContain('MAESTRO_PROMPT_EOF'); + + // Verify the structure: script ends with exec, then prompt follows + const parts = result.stdinScript?.split("exec opencode 'run' '--format' 'json'\n"); + expect(parts?.length).toBe(2); + expect(parts?.[1]).toBe('Write hello world to a file'); }); - it('handles stdin prompts with special characters without escaping issues', async () => { + it('handles stdin prompts with special characters without escaping', async () => { const result = await buildSshCommandWithStdin(baseConfig, { command: 'opencode', args: ['run'], stdinInput: "What's the $PATH? Use `echo` and \"quotes\"", }); - // The script should contain the prompt verbatim (no shell interpolation in heredoc) + // The prompt should be verbatim - no escaping needed since it's stdin passthrough expect(result.stdinScript).toBeDefined(); expect(result.stdinScript).toContain("What's the $PATH? Use `echo` and \"quotes\""); + + // Verify the prompt is AFTER the exec line (not in heredoc) + const execLine = result.stdinScript + ?.split('\n') + .find((line) => line.startsWith('exec ')); + expect(execLine).toBe("exec opencode 'run'"); }); it('handles multi-line stdin prompts', async () => { @@ -762,18 +783,19 @@ describe('ssh-command-builder', () => { expect(result.stdinScript).toContain('Line 3'); }); - it('uses a unique heredoc delimiter when prompt contains the default token', async () => { + it('handles prompts containing heredoc-like tokens without special treatment', async () => { + // With stdin passthrough, we don't need delimiter collision detection const result = await buildSshCommandWithStdin(baseConfig, { command: 'opencode', args: ['run'], - stdinInput: 'Line with MAESTRO_PROMPT_EOF inside', + stdinInput: 'Line with MAESTRO_PROMPT_EOF inside and < line.startsWith('exec ')); - expect(execLine).toContain("<<'MAESTRO_PROMPT_EOF_0'"); - expect(result.stdinScript).toContain('Line with MAESTRO_PROMPT_EOF inside'); + // The prompt should be verbatim - no special handling needed + expect(result.stdinScript).toContain('Line with MAESTRO_PROMPT_EOF inside and < { @@ -845,5 +867,26 @@ describe('ssh-command-builder', () => { expect(result.stdinScript).toContain('export REMOTE_VAR='); expect(result.stdinScript).toContain('export OPTION_VAR='); }); + + it('works with Claude Code stream-json format', async () => { + // Claude Code uses --input-format stream-json and expects JSON on stdin + const streamJsonPrompt = + '{"type":"user","message":{"role":"user","content":[{"type":"text","text":"Hello"}]}}'; + + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'claude', + args: ['--print', '--verbose', '--output-format', 'stream-json', '--input-format', 'stream-json'], + stdinInput: streamJsonPrompt, + }); + + // The JSON should be passed through verbatim + expect(result.stdinScript).toContain(streamJsonPrompt); + + // Verify exec line doesn't have the prompt + const execLine = result.stdinScript + ?.split('\n') + .find((line) => line.startsWith('exec ')); + expect(execLine).not.toContain('{"type"'); + }); }); }); diff --git a/src/main/ipc/handlers/process.ts b/src/main/ipc/handlers/process.ts index 1eb88b09..900e5658 100644 --- a/src/main/ipc/handlers/process.ts +++ b/src/main/ipc/handlers/process.ts @@ -345,17 +345,22 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void // The script contains PATH setup, cd, env vars, and the actual command // This eliminates all shell escaping issues // - // IMPORTANT: OpenCode prompts must be passed via stdin to avoid CLI length limits. - // Prompts can be huge and contain arbitrary characters; do NOT pass them as argv. - const shouldSendPromptViaStdin = config.toolType === 'opencode' && !!config.prompt; - const promptForArgs = shouldSendPromptViaStdin ? undefined : config.prompt; - const stdinInput = shouldSendPromptViaStdin ? config.prompt : undefined; + // IMPORTANT: ALL agent prompts are passed via stdin passthrough for SSH. + // Benefits: + // - Avoids CLI argument length limits (128KB-2MB depending on OS) + // - No shell escaping needed - prompt is never parsed by any shell + // - Works with any prompt content (quotes, newlines, special chars) + // - Simpler code - no heredoc or delimiter collision detection + // + // How it works: bash reads the script, `exec` replaces bash with the agent, + // and the agent reads the remaining stdin (the prompt) directly. + const stdinInput = config.prompt; const sshCommand = await buildSshCommandWithStdin(sshResult.config, { command: remoteCommand, args: finalArgs, cwd: config.cwd, env: effectiveCustomEnvVars, - prompt: promptForArgs, + // prompt is not passed as CLI arg - it goes via stdinInput stdinInput, }); @@ -366,7 +371,7 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void // For SSH, env vars are passed in the stdin script, not locally customEnvVarsToPass = undefined; - logger.info(`SSH command built with stdin script`, LOG_CONTEXT, { + logger.info(`SSH command built with stdin passthrough`, LOG_CONTEXT, { sessionId: config.sessionId, toolType: config.toolType, sshBinary: sshCommand.command, @@ -374,7 +379,7 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void remoteCommand, remoteCwd: config.cwd, promptLength: config.prompt?.length, - scriptLength: sshCommand.stdinScript?.length, + stdinScriptLength: sshCommand.stdinScript?.length, }); } } diff --git a/src/main/utils/ssh-command-builder.ts b/src/main/utils/ssh-command-builder.ts index 632b2116..32c747fd 100644 --- a/src/main/utils/ssh-command-builder.ts +++ b/src/main/utils/ssh-command-builder.ts @@ -140,17 +140,24 @@ export function buildRemoteCommand(options: RemoteCommandOptions): string { * This approach completely bypasses shell escaping issues by: * 1. SSH connects and runs `/bin/bash` on the remote * 2. The script (with PATH setup, cd, env vars, command) is sent via stdin - * 3. No shell parsing of command-line arguments occurs + * 3. The prompt (if any) is appended after the script, passed through to the exec'd command * * This is the preferred method for SSH remote execution as it: * - Handles any prompt content (special chars, newlines, quotes, etc.) * - Avoids command-line length limits * - Works regardless of the remote user's login shell (bash, zsh, fish, etc.) * - Eliminates the escaping nightmare of nested shell contexts + * - No heredoc or delimiter collision detection needed + * + * How stdin passthrough works: + * - Bash reads and executes the script lines + * - The `exec` command replaces bash with the target process + * - Any remaining stdin (the prompt) is inherited by the exec'd command + * - The prompt is NEVER parsed by any shell - it flows through as raw bytes * * @param config SSH remote configuration * @param remoteOptions Options for the remote command - * @returns SSH command/args plus the script to send via stdin + * @returns SSH command/args plus the script+prompt to send via stdin * * @example * const result = await buildSshCommandWithStdin(config, { @@ -162,7 +169,7 @@ export function buildRemoteCommand(options: RemoteCommandOptions): string { * }); * // result.command = 'ssh' * // result.args = ['-o', 'BatchMode=yes', ..., 'user@host', '/bin/bash'] - * // result.stdinScript = '#!/bin/bash\nexport PATH=...\ncd /home/user/project\nOPENCODE_CONFIG_CONTENT=...\nexec opencode run --format json <<'MAESTRO_PROMPT_EOF'\nWrite hello world to a file\nMAESTRO_PROMPT_EOF\n' + * // result.stdinScript = 'export PATH=...\ncd /home/user/project\nexport OPENCODE_CONFIG_CONTENT=...\nexec opencode run --format json\nWrite hello world to a file' */ export async function buildSshCommandWithStdin( config: SshRemoteConfig, @@ -232,30 +239,27 @@ export async function buildSshCommandWithStdin( // For the script, we use simple quoting since we're not going through shell parsing layers const cmdParts = [remoteOptions.command, ...remoteOptions.args.map((arg) => shellEscape(arg))]; - // Add prompt as final argument if provided and not sending via stdin + // Add prompt as final argument if provided and not sending via stdin passthrough const hasStdinInput = remoteOptions.stdinInput !== undefined; if (remoteOptions.prompt && !hasStdinInput) { cmdParts.push(shellEscape(remoteOptions.prompt)); } // Use exec to replace the shell with the command (cleaner process tree) - if (hasStdinInput) { - // IMPORTANT: Prompts must be passed via stdin to avoid CLI length limits. - // Build a safe heredoc delimiter that won't collide with the prompt content. - const delimiterBase = 'MAESTRO_PROMPT_EOF'; - let delimiter = delimiterBase; - let counter = 0; - while (remoteOptions.stdinInput?.includes(delimiter)) { - delimiter = `${delimiterBase}_${counter++}`; - } - scriptLines.push(`exec ${cmdParts.join(' ')} <<'${delimiter}'`); - scriptLines.push(remoteOptions.stdinInput ?? ''); - scriptLines.push(delimiter); - } else { - scriptLines.push(`exec ${cmdParts.join(' ')}`); - } + // When stdinInput is provided, the prompt will be appended after the script + // and passed through to the exec'd command via stdin inheritance + scriptLines.push(`exec ${cmdParts.join(' ')}`); - const stdinScript = scriptLines.join('\n') + '\n'; + // Build the final stdin content: script + optional prompt passthrough + // The script ends with exec, which replaces bash with the target command + // Any content after the script (the prompt) is read by the exec'd command from stdin + let stdinScript = scriptLines.join('\n') + '\n'; + + if (hasStdinInput && remoteOptions.stdinInput) { + // Append the prompt after the script - it will be passed through to the exec'd command + // No escaping needed - the prompt is never parsed by any shell + stdinScript += remoteOptions.stdinInput; + } logger.info('SSH command built with stdin script', '[ssh-command-builder]', { host: config.host, @@ -264,7 +268,9 @@ export async function buildSshCommandWithStdin( sshPath, sshArgsCount: args.length, scriptLineCount: scriptLines.length, - scriptLength: stdinScript.length, + stdinLength: stdinScript.length, + hasStdinInput, + stdinInputLength: remoteOptions.stdinInput?.length, // Show first part of script for debugging (truncate if long) scriptPreview: stdinScript.length > 500 ? stdinScript.substring(0, 500) + '...' : stdinScript, }); From 63d15d1a164000b54fd581762429a0a2046aeb11 Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Tue, 3 Feb 2026 15:17:01 -0600 Subject: [PATCH 7/8] fix(opencode): add question:deny to permission block for robust tool disabling Per OpenCode GitHub issue workaround, add "question": "deny" to the permission block in addition to the existing "tools":{"question":false}. This ensures the question tool is disabled via both configuration methods, preventing stdin hangs in batch mode. Config now: {"permission":{"*":"allow","external_directory":"allow","question":"deny"},"tools":{"question":false}} --- src/main/agents/definitions.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/agents/definitions.ts b/src/main/agents/definitions.ts index 6b893ab5..eb935768 100644 --- a/src/main/agents/definitions.ts +++ b/src/main/agents/definitions.ts @@ -200,11 +200,14 @@ export const AGENT_DEFINITIONS: AgentDefinition[] = [ imageArgs: (imagePath: string) => ['-f', imagePath], // Image/file attachment: opencode run -f /path/to/image.png -- "prompt" noPromptSeparator: true, // OpenCode doesn't need '--' before prompt - yargs handles positional args // Default env vars: enable YOLO mode (allow all permissions including external_directory) - // Also disable the question tool - it waits for stdin input which hangs batch mode + // Disable the question tool via both methods: + // - "question": "deny" in permission block (per OpenCode GitHub issue workaround) + // - "question": false in tools block (original approach) + // The question tool waits for stdin input which hangs batch mode // Users can override by setting customEnvVars in agent config defaultEnvVars: { OPENCODE_CONFIG_CONTENT: - '{"permission":{"*":"allow","external_directory":"allow"},"tools":{"question":false}}', + '{"permission":{"*":"allow","external_directory":"allow","question":"deny"},"tools":{"question":false}}', }, // Agent-specific configuration options shown in UI configOptions: [ From 963531e4ff9b3b0378a06f2b1a41d0de8e32e945 Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Tue, 3 Feb 2026 15:17:25 -0600 Subject: [PATCH 8/8] ## CHANGES MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Swapped moderator selection tiles for a clean agent dropdown picker 🔽 - Added expandable “Customize” panel directly inside both group chat modals ⚙️ - Lazily loads agent config/models only when customization panel expands 🚀 - Resets custom path/args/env overrides automatically when changing agents 🧹 - Displays clear “Detecting agents…” inline spinner during agent discovery ⏳ - Labels Codex, OpenCode, and Factory Droid options as “(Beta)” 🧪 - Expands empty-state messaging to recommend installing Factory Droid too 🧩 - Simplified modal flow by removing separate config-view navigation entirely 🧭 - Improved accessibility with proper combobox/button roles in modal controls ♿ - Strengthened test coverage for dropdown options and moderator-change warning 🧯 --- .../components/GroupChatModals.test.tsx | 98 +++- .../components/EditGroupChatModal.tsx | 463 ++++++++---------- src/renderer/components/NewGroupChatModal.tsx | 451 ++++++++--------- 3 files changed, 502 insertions(+), 510 deletions(-) diff --git a/src/__tests__/renderer/components/GroupChatModals.test.tsx b/src/__tests__/renderer/components/GroupChatModals.test.tsx index acf9f446..1fd05c3b 100644 --- a/src/__tests__/renderer/components/GroupChatModals.test.tsx +++ b/src/__tests__/renderer/components/GroupChatModals.test.tsx @@ -7,7 +7,7 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { render, screen, fireEvent, waitFor, act } from '@testing-library/react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import { NewGroupChatModal } from '../../../renderer/components/NewGroupChatModal'; import { EditGroupChatModal } from '../../../renderer/components/EditGroupChatModal'; import type { Theme, GroupChat, AgentConfig } from '../../../renderer/types'; @@ -167,18 +167,17 @@ describe('Group Chat Modals', () => { /> ); - // Wait for agent detection + // Wait for agent detection and verify dropdown is rendered await waitFor(() => { - expect(screen.getByText('Claude Code')).toBeInTheDocument(); + expect(screen.getByRole('combobox', { name: /select moderator/i })).toBeInTheDocument(); }); - // Select the agent first (the tile is now a div with role="button") - const agentTile = screen.getByText('Claude Code').closest('[role="button"]'); - expect(agentTile).not.toBeNull(); - fireEvent.click(agentTile!); + // Verify Claude Code is selected in dropdown + const dropdown = screen.getByRole('combobox', { name: /select moderator/i }); + expect(dropdown).toHaveValue('claude-code'); - // Click the Customize button to open config panel - const customizeButton = screen.getByText('Customize'); + // Click the Customize button to expand config panel + const customizeButton = screen.getByRole('button', { name: /customize/i }); fireEvent.click(customizeButton); // Wait for config panel to appear and verify MAESTRO_SESSION_RESUMED is displayed @@ -189,6 +188,39 @@ describe('Group Chat Modals', () => { // Also verify the value hint is shown expect(screen.getByText('1 (when resuming)')).toBeInTheDocument(); }); + + it('should show all available agents in dropdown', async () => { + // Setup multiple agents + vi.mocked(window.maestro.agents.detect).mockResolvedValue([ + createMockAgent({ id: 'claude-code', name: 'Claude Code' }), + createMockAgent({ id: 'codex', name: 'Codex' }), + createMockAgent({ id: 'opencode', name: 'OpenCode' }), + createMockAgent({ id: 'factory-droid', name: 'Factory Droid' }), + ]); + + const onCreate = vi.fn(); + const onClose = vi.fn(); + + render( + + ); + + // Wait for dropdown to be rendered + await waitFor(() => { + expect(screen.getByRole('combobox', { name: /select moderator/i })).toBeInTheDocument(); + }); + + // Verify all agents appear as options + expect(screen.getByRole('option', { name: /Claude Code/i })).toBeInTheDocument(); + expect(screen.getByRole('option', { name: /Codex.*Beta/i })).toBeInTheDocument(); + expect(screen.getByRole('option', { name: /OpenCode.*Beta/i })).toBeInTheDocument(); + expect(screen.getByRole('option', { name: /Factory Droid.*Beta/i })).toBeInTheDocument(); + }); }); describe('EditGroupChatModal', () => { @@ -207,13 +239,17 @@ describe('Group Chat Modals', () => { /> ); - // Wait for agent detection + // Wait for dropdown to be rendered await waitFor(() => { - expect(screen.getByText('Claude Code')).toBeInTheDocument(); + expect(screen.getByRole('combobox', { name: /select moderator/i })).toBeInTheDocument(); }); - // Click the Customize button to open config panel - const customizeButton = screen.getByText('Customize'); + // Verify Claude Code is pre-selected + const dropdown = screen.getByRole('combobox', { name: /select moderator/i }); + expect(dropdown).toHaveValue('claude-code'); + + // Click the Customize button to expand config panel + const customizeButton = screen.getByRole('button', { name: /customize/i }); fireEvent.click(customizeButton); // Wait for config panel to appear and verify MAESTRO_SESSION_RESUMED is displayed @@ -224,5 +260,41 @@ describe('Group Chat Modals', () => { // Also verify the value hint is shown expect(screen.getByText('1 (when resuming)')).toBeInTheDocument(); }); + + it('should show warning when changing moderator agent', async () => { + // Setup multiple agents + vi.mocked(window.maestro.agents.detect).mockResolvedValue([ + createMockAgent({ id: 'claude-code', name: 'Claude Code' }), + createMockAgent({ id: 'codex', name: 'Codex' }), + ]); + + const onSave = vi.fn(); + const onClose = vi.fn(); + const groupChat = createMockGroupChat({ moderatorAgentId: 'claude-code' }); + + render( + + ); + + // Wait for dropdown + await waitFor(() => { + expect(screen.getByRole('combobox', { name: /select moderator/i })).toBeInTheDocument(); + }); + + // Change to different agent + const dropdown = screen.getByRole('combobox', { name: /select moderator/i }); + fireEvent.change(dropdown, { target: { value: 'codex' } }); + + // Verify warning message appears + await waitFor(() => { + expect(screen.getByText(/changing the moderator agent/i)).toBeInTheDocument(); + }); + }); }); }); diff --git a/src/renderer/components/EditGroupChatModal.tsx b/src/renderer/components/EditGroupChatModal.tsx index 5685c6ca..f6069a3d 100644 --- a/src/renderer/components/EditGroupChatModal.tsx +++ b/src/renderer/components/EditGroupChatModal.tsx @@ -3,19 +3,19 @@ * * Modal for editing an existing Group Chat. Allows user to: * - Change the name of the group chat - * - Change the moderator agent - * - Customize moderator settings (CLI args, path, ENV vars) + * - Change the moderator agent via dropdown + * - Customize moderator settings (CLI args, path, ENV vars) via expandable panel * * Similar to NewGroupChatModal but pre-populated with existing values. */ import { useState, useEffect, useRef, useCallback } from 'react'; -import { Check, X, Settings, ArrowLeft } from 'lucide-react'; +import { X, Settings, ChevronDown, Check } from 'lucide-react'; import type { Theme, AgentConfig, ModeratorConfig, GroupChat } from '../types'; import type { SshRemoteConfig, AgentSshRemoteConfig } from '../../shared/types'; import { MODAL_PRIORITIES } from '../constants/modalPriorities'; import { Modal, ModalFooter, FormInput } from './ui'; -import { AgentLogo, AGENT_TILES } from './Wizard/screens/AgentSelectionScreen'; +import { AGENT_TILES } from './Wizard/screens/AgentSelectionScreen'; import { AgentConfigPanel } from './shared/AgentConfigPanel'; import { SshRemoteSelector } from './shared/SshRemoteSelector'; @@ -44,9 +44,8 @@ export function EditGroupChatModal({ const [detectedAgents, setDetectedAgents] = useState([]); const [isDetecting, setIsDetecting] = useState(true); - // View mode for switching between grid and config - const [viewMode, setViewMode] = useState<'grid' | 'config'>('grid'); - const [isTransitioning, setIsTransitioning] = useState(false); + // Configuration panel state - expandable below dropdown + const [isConfigExpanded, setIsConfigExpanded] = useState(false); // Custom moderator configuration state const [customPath, setCustomPath] = useState(''); @@ -81,8 +80,7 @@ export function EditGroupChatModal({ setCustomPath(groupChat.moderatorConfig?.customPath || ''); setCustomArgs(groupChat.moderatorConfig?.customArgs || ''); setCustomEnvVars(groupChat.moderatorConfig?.customEnvVars || {}); - setViewMode('grid'); - setIsTransitioning(false); + setIsConfigExpanded(false); setAgentConfig({}); setAvailableModels([]); setLoadingModels(false); @@ -94,8 +92,7 @@ export function EditGroupChatModal({ setName(''); setSelectedAgent(null); setIsDetecting(true); - setViewMode('grid'); - setIsTransitioning(false); + setIsConfigExpanded(false); setCustomPath(''); setCustomArgs(''); setCustomEnvVars({}); @@ -143,10 +140,41 @@ export function EditGroupChatModal({ // Focus name input when agents detected useEffect(() => { - if (!isDetecting && isOpen && viewMode === 'grid') { + if (!isDetecting && isOpen) { nameInputRef.current?.focus(); } - }, [isDetecting, isOpen, viewMode]); + }, [isDetecting, isOpen]); + + // Load agent config when expanding configuration panel + useEffect(() => { + if (isConfigExpanded && selectedAgent) { + loadAgentConfig(selectedAgent); + } + }, [isConfigExpanded, selectedAgent]); + + // Load agent configuration + const loadAgentConfig = useCallback( + async (agentId: string) => { + const config = await window.maestro.agents.getConfig(agentId); + setAgentConfig(config || {}); + agentConfigRef.current = config || {}; + + // Load models if agent supports it + const agent = detectedAgents.find((a) => a.id === agentId); + if (agent?.capabilities?.supportsModelSelection) { + setLoadingModels(true); + try { + const models = await window.maestro.agents.getModels(agentId); + setAvailableModels(models); + } catch (err) { + console.error('Failed to load models:', err); + } finally { + setLoadingModels(false); + } + } + }, + [detectedAgents] + ); // Build moderator config from state const buildModeratorConfig = useCallback((): ModeratorConfig | undefined => { @@ -194,45 +222,9 @@ export function EditGroupChatModal({ const canSave = name.trim().length > 0 && selectedAgent !== null && hasChanges(); - // Open configuration panel for the selected agent - const handleOpenConfig = useCallback(async () => { - if (!selectedAgent) return; - - // Load agent config - const config = await window.maestro.agents.getConfig(selectedAgent); - setAgentConfig(config || {}); - agentConfigRef.current = config || {}; - - // Load models if agent supports it - const agent = detectedAgents.find((a) => a.id === selectedAgent); - // Note: capabilities is added by agent-detector but not in the TypeScript type - if ((agent as any)?.capabilities?.supportsModelSelection) { - setLoadingModels(true); - try { - const models = await window.maestro.agents.getModels(selectedAgent); - setAvailableModels(models); - } catch (err) { - console.error('Failed to load models:', err); - } finally { - setLoadingModels(false); - } - } - - // Transition to config view - setIsTransitioning(true); - setTimeout(() => { - setViewMode('config'); - setIsTransitioning(false); - }, 150); - }, [selectedAgent, detectedAgents]); - - // Close configuration panel - const handleCloseConfig = useCallback(() => { - setIsTransitioning(true); - setTimeout(() => { - setViewMode('grid'); - setIsTransitioning(false); - }, 150); + // Toggle configuration panel + const handleToggleConfig = useCallback(() => { + setIsConfigExpanded((prev) => !prev); }, []); // Refresh agent detection after config changes @@ -266,6 +258,22 @@ export function EditGroupChatModal({ } }, [selectedAgent]); + // Handle agent selection change + const handleAgentChange = useCallback( + (agentId: string) => { + setSelectedAgent(agentId); + // Reset customizations when changing agent + setCustomPath(''); + setCustomArgs(''); + setCustomEnvVars({}); + // If config is expanded, reload config for new agent + if (isConfigExpanded) { + loadAgentConfig(agentId); + } + }, + [isConfigExpanded, loadAgentConfig] + ); + if (!isOpen || !groupChat) return null; // Filter AGENT_TILES to only show supported + detected agents @@ -281,127 +289,6 @@ export function EditGroupChatModal({ // Check if there's any customization set const hasCustomization = customPath || customArgs || Object.keys(customEnvVars).length > 0; - // Render configuration view - if (viewMode === 'config' && selectedAgentConfig && selectedTile) { - return ( - -
- -

- Configure {selectedTile.name} -

-
- - - } - footer={ - - } - > -
- { - /* Local state only */ - }} - onCustomPathClear={() => setCustomPath('')} - customArgs={customArgs} - onCustomArgsChange={setCustomArgs} - onCustomArgsBlur={() => { - /* Local state only */ - }} - onCustomArgsClear={() => setCustomArgs('')} - customEnvVars={customEnvVars} - onEnvVarKeyChange={(oldKey, newKey, value) => { - const newVars = { ...customEnvVars }; - delete newVars[oldKey]; - newVars[newKey] = value; - setCustomEnvVars(newVars); - }} - onEnvVarValueChange={(key, value) => { - setCustomEnvVars({ ...customEnvVars, [key]: value }); - }} - onEnvVarRemove={(key) => { - const newVars = { ...customEnvVars }; - delete newVars[key]; - setCustomEnvVars(newVars); - }} - onEnvVarAdd={() => { - let newKey = 'NEW_VAR'; - let counter = 1; - while (customEnvVars[newKey]) { - newKey = `NEW_VAR_${counter}`; - counter++; - } - setCustomEnvVars({ ...customEnvVars, [newKey]: '' }); - }} - onEnvVarsBlur={() => { - /* Local state only */ - }} - agentConfig={agentConfig} - onConfigChange={(key, value) => { - const newConfig = { ...agentConfig, [key]: value }; - setAgentConfig(newConfig); - agentConfigRef.current = newConfig; - setConfigWasModified(true); - }} - onConfigBlur={async () => { - if (selectedAgent) { - // Use ref to get latest config (state may be stale in async callback) - await window.maestro.agents.setConfig(selectedAgent, agentConfigRef.current); - setConfigWasModified(true); - } - }} - availableModels={availableModels} - loadingModels={loadingModels} - onRefreshModels={handleRefreshModels} - onRefreshAgent={handleRefreshAgent} - refreshingAgent={refreshingAgent} - compact - showBuiltInEnvVars - /> -
- - ); - } - - // Render grid view return ( } > -
+
{/* Name Input */}
- {/* Agent Selection */} -
+ {/* Moderator Selection - Dropdown with Customize button */} +
{isDetecting ? ( -
+
+ + Detecting agents... +
) : availableTiles.length === 0 ? ( -
- No agents available. Please install Claude Code, OpenCode, or Codex. +
+ No agents available. Please install Claude Code, OpenCode, Codex, or Factory Droid.
) : ( -
- {availableTiles.map((tile) => { - const isSelected = selectedAgent === tile.id; +
+ {/* Dropdown */} +
+ + +
- return ( -
setSelectedAgent(tile.id)} - onKeyDown={(e) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault(); - setSelectedAgent(tile.id); - } - }} - className="relative flex flex-col items-center p-4 pb-10 rounded-lg border-2 transition-all outline-none cursor-pointer" - style={{ - backgroundColor: isSelected ? `${tile.brandColor}15` : theme.colors.bgMain, - borderColor: isSelected ? tile.brandColor : theme.colors.border, - }} - > - {isSelected && ( -
- -
- )} - - - {tile.name} + {/* Customize button */} + +
+ )} + + {/* Expandable Configuration Panel */} + {isConfigExpanded && selectedAgentConfig && selectedTile && ( +
+
+ + {selectedTile.name} Configuration + + {hasCustomization && ( +
+ + + Customized - - {/* Customize button */} -
- ); - })} + )} +
+ { + /* Local state only */ + }} + onCustomPathClear={() => setCustomPath('')} + customArgs={customArgs} + onCustomArgsChange={setCustomArgs} + onCustomArgsBlur={() => { + /* Local state only */ + }} + onCustomArgsClear={() => setCustomArgs('')} + customEnvVars={customEnvVars} + onEnvVarKeyChange={(oldKey, newKey, value) => { + const newVars = { ...customEnvVars }; + delete newVars[oldKey]; + newVars[newKey] = value; + setCustomEnvVars(newVars); + }} + onEnvVarValueChange={(key, value) => { + setCustomEnvVars({ ...customEnvVars, [key]: value }); + }} + onEnvVarRemove={(key) => { + const newVars = { ...customEnvVars }; + delete newVars[key]; + setCustomEnvVars(newVars); + }} + onEnvVarAdd={() => { + let newKey = 'NEW_VAR'; + let counter = 1; + while (customEnvVars[newKey]) { + newKey = `NEW_VAR_${counter}`; + counter++; + } + setCustomEnvVars({ ...customEnvVars, [newKey]: '' }); + }} + onEnvVarsBlur={() => { + /* Local state only */ + }} + agentConfig={agentConfig} + onConfigChange={(key, value) => { + const newConfig = { ...agentConfig, [key]: value }; + setAgentConfig(newConfig); + agentConfigRef.current = newConfig; + setConfigWasModified(true); + }} + onConfigBlur={async () => { + if (selectedAgent) { + // Use ref to get latest config (state may be stale in async callback) + await window.maestro.agents.setConfig(selectedAgent, agentConfigRef.current); + setConfigWasModified(true); + } + }} + availableModels={availableModels} + loadingModels={loadingModels} + onRefreshModels={handleRefreshModels} + onRefreshAgent={handleRefreshAgent} + refreshingAgent={refreshingAgent} + compact + showBuiltInEnvVars + />
)}
{/* SSH Remote Execution - Top Level */} {sshRemotes.length > 0 && ( -
+
([]); const [isDetecting, setIsDetecting] = useState(true); - // View mode for switching between grid and config - const [viewMode, setViewMode] = useState<'grid' | 'config'>('grid'); - const [isTransitioning, setIsTransitioning] = useState(false); + // Configuration panel state - expandable below dropdown + const [isConfigExpanded, setIsConfigExpanded] = useState(false); // Custom moderator configuration state const [customPath, setCustomPath] = useState(''); @@ -65,8 +64,7 @@ export function NewGroupChatModal({ setName(''); setSelectedAgent(null); setIsDetecting(true); - setViewMode('grid'); - setIsTransitioning(false); + setIsConfigExpanded(false); setCustomPath(''); setCustomArgs(''); setCustomEnvVars({}); @@ -127,10 +125,41 @@ export function NewGroupChatModal({ // Focus name input when agents detected useEffect(() => { - if (!isDetecting && isOpen && viewMode === 'grid') { + if (!isDetecting && isOpen) { nameInputRef.current?.focus(); } - }, [isDetecting, isOpen, viewMode]); + }, [isDetecting, isOpen]); + + // Load agent config when expanding configuration panel + useEffect(() => { + if (isConfigExpanded && selectedAgent) { + loadAgentConfig(selectedAgent); + } + }, [isConfigExpanded, selectedAgent]); + + // Load agent configuration + const loadAgentConfig = useCallback( + async (agentId: string) => { + const config = await window.maestro.agents.getConfig(agentId); + setAgentConfig(config || {}); + agentConfigRef.current = config || {}; + + // Load models if agent supports it + const agent = detectedAgents.find((a) => a.id === agentId); + if (agent?.capabilities?.supportsModelSelection) { + setLoadingModels(true); + try { + const models = await window.maestro.agents.getModels(agentId); + setAvailableModels(models); + } catch (err) { + console.error('Failed to load models:', err); + } finally { + setLoadingModels(false); + } + } + }, + [detectedAgents] + ); // Build moderator config from state const buildModeratorConfig = useCallback((): ModeratorConfig | undefined => { @@ -155,44 +184,9 @@ export function NewGroupChatModal({ const canCreate = name.trim().length > 0 && selectedAgent !== null; - // Open configuration panel for the selected agent - const handleOpenConfig = useCallback(async () => { - if (!selectedAgent) return; - - // Load agent config - const config = await window.maestro.agents.getConfig(selectedAgent); - setAgentConfig(config || {}); - agentConfigRef.current = config || {}; - - // Load models if agent supports it - const agent = detectedAgents.find((a) => a.id === selectedAgent); - if (agent?.capabilities?.supportsModelSelection) { - setLoadingModels(true); - try { - const models = await window.maestro.agents.getModels(selectedAgent); - setAvailableModels(models); - } catch (err) { - console.error('Failed to load models:', err); - } finally { - setLoadingModels(false); - } - } - - // Transition to config view - setIsTransitioning(true); - setTimeout(() => { - setViewMode('config'); - setIsTransitioning(false); - }, 150); - }, [selectedAgent, detectedAgents]); - - // Close configuration panel - const handleCloseConfig = useCallback(() => { - setIsTransitioning(true); - setTimeout(() => { - setViewMode('grid'); - setIsTransitioning(false); - }, 150); + // Toggle configuration panel + const handleToggleConfig = useCallback(() => { + setIsConfigExpanded((prev) => !prev); }, []); // Refresh agent detection after config changes @@ -226,6 +220,22 @@ export function NewGroupChatModal({ } }, [selectedAgent]); + // Handle agent selection change + const handleAgentChange = useCallback( + (agentId: string) => { + setSelectedAgent(agentId); + // Reset customizations when changing agent + setCustomPath(''); + setCustomArgs(''); + setCustomEnvVars({}); + // If config is expanded, reload config for new agent + if (isConfigExpanded) { + loadAgentConfig(agentId); + } + }, + [isConfigExpanded, loadAgentConfig] + ); + if (!isOpen) return null; // Filter AGENT_TILES to only show supported + detected agents @@ -241,125 +251,6 @@ export function NewGroupChatModal({ // Check if there's any customization set const hasCustomization = customPath || customArgs || Object.keys(customEnvVars).length > 0; - // Render configuration view - if (viewMode === 'config' && selectedAgentConfig && selectedTile) { - return ( - -
- -

- Configure {selectedTile.name} -

-
- -
- } - footer={ - - } - > -
- { - /* Local state only */ - }} - onCustomPathClear={() => setCustomPath('')} - customArgs={customArgs} - onCustomArgsChange={setCustomArgs} - onCustomArgsBlur={() => { - /* Local state only */ - }} - onCustomArgsClear={() => setCustomArgs('')} - customEnvVars={customEnvVars} - onEnvVarKeyChange={(oldKey, newKey, value) => { - const newVars = { ...customEnvVars }; - delete newVars[oldKey]; - newVars[newKey] = value; - setCustomEnvVars(newVars); - }} - onEnvVarValueChange={(key, value) => { - setCustomEnvVars({ ...customEnvVars, [key]: value }); - }} - onEnvVarRemove={(key) => { - const newVars = { ...customEnvVars }; - delete newVars[key]; - setCustomEnvVars(newVars); - }} - onEnvVarAdd={() => { - let newKey = 'NEW_VAR'; - let counter = 1; - while (customEnvVars[newKey]) { - newKey = `NEW_VAR_${counter}`; - counter++; - } - setCustomEnvVars({ ...customEnvVars, [newKey]: '' }); - }} - onEnvVarsBlur={() => { - /* Local state only */ - }} - agentConfig={agentConfig} - onConfigChange={(key, value) => { - const newConfig = { ...agentConfig, [key]: value }; - setAgentConfig(newConfig); - agentConfigRef.current = newConfig; - }} - onConfigBlur={async () => { - if (selectedAgent) { - // Use ref to get latest config (state may be stale in async callback) - await window.maestro.agents.setConfig(selectedAgent, agentConfigRef.current); - } - }} - availableModels={availableModels} - loadingModels={loadingModels} - onRefreshModels={handleRefreshModels} - onRefreshAgent={handleRefreshAgent} - refreshingAgent={refreshingAgent} - compact - showBuiltInEnvVars - /> -
- - ); - } - - // Render grid view return ( } > -
+
{/* Description */}
A Group Chat lets you collaborate with multiple AI agents in a single conversation. The{' '} @@ -422,98 +311,170 @@ export function NewGroupChatModal({ Claude appears to be the best performing moderator.
- {/* Agent Selection */} + {/* Moderator Selection - Dropdown with Customize button */}
{isDetecting ? ( -
+
+ + Detecting agents... +
) : availableTiles.length === 0 ? ( -
- No agents available. Please install Claude Code, OpenCode, or Codex. +
+ No agents available. Please install Claude Code, OpenCode, Codex, or Factory Droid.
) : ( -
- {availableTiles.map((tile) => { - const isSelected = selectedAgent === tile.id; +
+ {/* Dropdown */} +
+ + +
- return ( -
setSelectedAgent(tile.id)} - onKeyDown={(e) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault(); - setSelectedAgent(tile.id); - } - }} - className="relative flex flex-col items-center p-4 pb-10 rounded-lg border-2 transition-all outline-none cursor-pointer" - style={{ - backgroundColor: isSelected ? `${tile.brandColor}15` : theme.colors.bgMain, - borderColor: isSelected ? tile.brandColor : theme.colors.border, - }} - > - {isSelected && ( -
- -
- )} - - - {tile.name} + {/* Customize button */} + +
+ )} + + {/* Expandable Configuration Panel */} + {isConfigExpanded && selectedAgentConfig && selectedTile && ( +
+
+ + {selectedTile.name} Configuration + + {hasCustomization && ( +
+ + + Customized - - {/* Customize button */} -
- ); - })} + )} +
+ { + /* Local state only */ + }} + onCustomPathClear={() => setCustomPath('')} + customArgs={customArgs} + onCustomArgsChange={setCustomArgs} + onCustomArgsBlur={() => { + /* Local state only */ + }} + onCustomArgsClear={() => setCustomArgs('')} + customEnvVars={customEnvVars} + onEnvVarKeyChange={(oldKey, newKey, value) => { + const newVars = { ...customEnvVars }; + delete newVars[oldKey]; + newVars[newKey] = value; + setCustomEnvVars(newVars); + }} + onEnvVarValueChange={(key, value) => { + setCustomEnvVars({ ...customEnvVars, [key]: value }); + }} + onEnvVarRemove={(key) => { + const newVars = { ...customEnvVars }; + delete newVars[key]; + setCustomEnvVars(newVars); + }} + onEnvVarAdd={() => { + let newKey = 'NEW_VAR'; + let counter = 1; + while (customEnvVars[newKey]) { + newKey = `NEW_VAR_${counter}`; + counter++; + } + setCustomEnvVars({ ...customEnvVars, [newKey]: '' }); + }} + onEnvVarsBlur={() => { + /* Local state only */ + }} + agentConfig={agentConfig} + onConfigChange={(key, value) => { + const newConfig = { ...agentConfig, [key]: value }; + setAgentConfig(newConfig); + agentConfigRef.current = newConfig; + }} + onConfigBlur={async () => { + if (selectedAgent) { + // Use ref to get latest config (state may be stale in async callback) + await window.maestro.agents.setConfig(selectedAgent, agentConfigRef.current); + } + }} + availableModels={availableModels} + loadingModels={loadingModels} + onRefreshModels={handleRefreshModels} + onRefreshAgent={handleRefreshAgent} + refreshingAgent={refreshingAgent} + compact + showBuiltInEnvVars + />
)}