From 5da6247fae1f00973ae95d62766bb1ccec0ba783 Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Thu, 8 Jan 2026 08:33:23 -0600 Subject: [PATCH] =?UTF-8?q?-=20Auto=20Run=20docs=20now=20support=20repo=20?= =?UTF-8?q?paths=20*and*=20GitHub=20attachment=20links=20=F0=9F=93=8E=20-?= =?UTF-8?q?=20Introduced=20`DocumentReference`=20objects=20with=20name,=20?= =?UTF-8?q?path,=20and=20external=20flag=20=F0=9F=A7=A9=20-=20Smarter=20is?= =?UTF-8?q?sue=20parsing=20extracts=20markdown=20`.md`=20links=20into=20do?= =?UTF-8?q?wnloadable=20docs=20=F0=9F=94=8D=20-=20Dedupes=20documents=20by?= =?UTF-8?q?=20filename,=20preferring=20external=20attachments=20when=20pre?= =?UTF-8?q?sent=20=F0=9F=A7=A0=20-=20Added=201MB=20issue-body=20parsing=20?= =?UTF-8?q?cap=20to=20prevent=20performance=20blowups=20=F0=9F=9A=A7=20-?= =?UTF-8?q?=20Path=20traversal=20checks=20now=20apply=20only=20to=20repo-r?= =?UTF-8?q?elative=20document=20references=20=F0=9F=9B=A1=EF=B8=8F=20-=20A?= =?UTF-8?q?uto=20Run=20Docs=20setup=20can=20download=20external=20files=20?= =?UTF-8?q?directly=20via=20`fetch`=20=F0=9F=8C=90=20-=20Switched=20runner?= =?UTF-8?q?=20file=20ops=20to=20Node=20`fs`,=20replacing=20shell=20`cp/rm`?= =?UTF-8?q?=20usage=20=F0=9F=A7=B0=20-=20Runner=20now=20configures=20git?= =?UTF-8?q?=20user.name/email=20to=20ensure=20commits=20always=20work=20?= =?UTF-8?q?=F0=9F=AA=AA=20-=20Failure=20paths=20now=20clean=20up=20local?= =?UTF-8?q?=20repos=20automatically=20to=20reduce=20clutter=20=F0=9F=A7=B9?= =?UTF-8?q?=20-=20History=20virtualization=20now=20measures=20elements=20d?= =?UTF-8?q?irectly=20for=20more=20accurate=20sizing=20=F0=9F=93=8F=20-=20M?= =?UTF-8?q?arketplace=20and=20Symphony=20modals=20widened=20for=20a=20room?= =?UTF-8?q?ier=20workflow=20view=20=F0=9F=96=A5=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/ipc/handlers/symphony.ts | 141 ++++++++++++++++------ src/main/services/symphony-runner.ts | 103 ++++++++++++++-- src/renderer/components/SymphonyModal.tsx | 18 +-- src/shared/symphony-types.ts | 20 ++- 4 files changed, 221 insertions(+), 61 deletions(-) diff --git a/src/main/ipc/handlers/symphony.ts b/src/main/ipc/handlers/symphony.ts index e6b4898d..8f4d2e64 100644 --- a/src/main/ipc/handlers/symphony.ts +++ b/src/main/ipc/handlers/symphony.ts @@ -43,6 +43,7 @@ import type { StartContributionResponse, CompleteContributionResponse, IssueStatus, + DocumentReference, } from '../../../shared/symphony-types'; import { SymphonyError } from '../../../shared/symphony-types'; @@ -126,7 +127,7 @@ function validateContributionParams(params: { repoUrl: string; repoName: string; issueNumber: number; - documentPaths: string[]; + documentPaths: DocumentReference[]; }): { valid: boolean; error?: string } { // Validate repo slug const slugValidation = validateRepoSlug(params.repoSlug); @@ -150,10 +151,12 @@ function validateContributionParams(params: { return { valid: false, error: 'Invalid issue number' }; } - // Validate document paths (check for path traversal) - for (const docPath of params.documentPaths) { - if (docPath.includes('..') || docPath.startsWith('/')) { - return { valid: false, error: `Invalid document path: ${docPath}` }; + // Validate document paths (check for path traversal in repo-relative paths) + for (const doc of params.documentPaths) { + // Skip validation for external URLs (they're downloaded, not file paths) + if (doc.isExternal) continue; + if (doc.path.includes('..') || doc.path.startsWith('/')) { + return { valid: false, error: `Invalid document path: ${doc.path}` }; } } @@ -280,25 +283,69 @@ function generateBranchName(issueNumber: number): string { .replace('{timestamp}', timestamp); } -/** - * Parse document paths from issue body. - */ -function parseDocumentPaths(body: string): string[] { - const paths: Set = new Set(); +/** Maximum body size to parse (1MB) to prevent performance issues */ +const MAX_BODY_SIZE = 1024 * 1024; - for (const pattern of DOCUMENT_PATH_PATTERNS) { - // Reset lastIndex for global regex - pattern.lastIndex = 0; - let match; - while ((match = pattern.exec(body)) !== null) { - const docPath = match[1]; - if (docPath && !docPath.startsWith('http')) { - paths.add(docPath); +/** + * Parse document references from issue body. + * Supports both repository-relative paths and GitHub attachment links. + */ +function parseDocumentPaths(body: string): DocumentReference[] { + // Guard against extremely large bodies that could cause performance issues + if (body.length > MAX_BODY_SIZE) { + logger.warn('Issue body too large, truncating for document parsing', LOG_CONTEXT, { + bodyLength: body.length, + maxSize: MAX_BODY_SIZE, + }); + body = body.substring(0, MAX_BODY_SIZE); + } + + const docs: Map = new Map(); + + // Pattern for markdown links: [filename.md](url) + // Captures: [1] = filename (link text), [2] = URL + const markdownLinkPattern = /\[([^\]]+\.md)\]\(([^)]+)\)/gi; + + // First, check for markdown links (GitHub attachments) + let match; + while ((match = markdownLinkPattern.exec(body)) !== null) { + const filename = match[1]; + const url = match[2]; + // Only add if it's a GitHub attachment URL or similar external URL + if (url.startsWith('http')) { + const key = filename.toLowerCase(); // Dedupe by filename + if (!docs.has(key)) { + docs.set(key, { + name: filename, + path: url, + isExternal: true, + }); } } } - return Array.from(paths); + // Then check for repo-relative paths using existing patterns + for (const pattern of DOCUMENT_PATH_PATTERNS) { + // Reset lastIndex for global regex + pattern.lastIndex = 0; + while ((match = pattern.exec(body)) !== null) { + const docPath = match[1]; + if (docPath && !docPath.startsWith('http')) { + const filename = docPath.split('/').pop() || docPath; + const key = filename.toLowerCase(); + // Don't overwrite external links with same filename + if (!docs.has(key)) { + docs.set(key, { + name: filename, + path: docPath, + isExternal: false, + }); + } + } + } + } + + return Array.from(docs.values()); } // ============================================================================ @@ -751,7 +798,7 @@ export function registerSymphonyHandlers({ app, getMainWindow }: SymphonyHandler repoName: string; issueNumber: number; issueTitle: string; - documentPaths: string[]; + documentPaths: DocumentReference[]; agentType: string; sessionId: string; baseBranch?: string; @@ -1151,7 +1198,7 @@ This PR will be updated automatically when the Auto Run completes.`; issueNumber: number; issueTitle: string; localPath: string; - documentPaths: string[]; + documentPaths: DocumentReference[]; }): Promise<{ success: boolean; branchName?: string; @@ -1172,10 +1219,10 @@ This PR will be updated automatically when the Auto Run completes.`; return { success: false, error: 'Invalid issue number' }; } - // Validate document paths for path traversal - for (const docPath of documentPaths) { - if (docPath.includes('..') || docPath.startsWith('/')) { - return { success: false, error: `Invalid document path: ${docPath}` }; + // Validate document paths for path traversal (only repo-relative paths) + for (const doc of documentPaths) { + if (!doc.isExternal && (doc.path.includes('..') || doc.path.startsWith('/'))) { + return { success: false, error: `Invalid document path: ${doc.path}` }; } } @@ -1235,19 +1282,37 @@ This PR will be updated automatically when the Auto Run completes.`; const autoRunDir = path.join(localPath, 'Auto Run Docs'); await fs.mkdir(autoRunDir, { recursive: true }); - for (const docPath of documentPaths) { - // Ensure the path doesn't escape the localPath - const resolvedSource = path.resolve(localPath, docPath); - if (!resolvedSource.startsWith(localPath)) { - logger.error('Attempted path traversal in document copy', LOG_CONTEXT, { docPath }); - continue; - } - const destPath = path.join(autoRunDir, path.basename(docPath)); - try { - await fs.copyFile(resolvedSource, destPath); - logger.info('Copied document', LOG_CONTEXT, { from: resolvedSource, to: destPath }); - } catch (e) { - logger.warn('Failed to copy document', LOG_CONTEXT, { docPath, error: e instanceof Error ? e.message : String(e) }); + for (const doc of documentPaths) { + const destPath = path.join(autoRunDir, doc.name); + + if (doc.isExternal) { + // Download external file (GitHub attachment) + try { + logger.info('Downloading external document', LOG_CONTEXT, { name: doc.name, url: doc.path }); + const response = await fetch(doc.path); + if (!response.ok) { + logger.warn('Failed to download document', LOG_CONTEXT, { name: doc.name, status: response.status }); + continue; + } + const buffer = await response.arrayBuffer(); + await fs.writeFile(destPath, Buffer.from(buffer)); + logger.info('Downloaded document', LOG_CONTEXT, { name: doc.name, to: destPath }); + } catch (e) { + logger.warn('Failed to download document', LOG_CONTEXT, { name: doc.name, error: e instanceof Error ? e.message : String(e) }); + } + } else { + // Copy from repo - ensure the path doesn't escape the localPath + const resolvedSource = path.resolve(localPath, doc.path); + if (!resolvedSource.startsWith(localPath)) { + logger.error('Attempted path traversal in document copy', LOG_CONTEXT, { docPath: doc.path }); + continue; + } + try { + await fs.copyFile(resolvedSource, destPath); + logger.info('Copied document', LOG_CONTEXT, { from: resolvedSource, to: destPath }); + } catch (e) { + logger.warn('Failed to copy document', LOG_CONTEXT, { docPath: doc.path, error: e instanceof Error ? e.message : String(e) }); + } } } diff --git a/src/main/services/symphony-runner.ts b/src/main/services/symphony-runner.ts index 1ebaa39c..6300d618 100644 --- a/src/main/services/symphony-runner.ts +++ b/src/main/services/symphony-runner.ts @@ -5,20 +5,32 @@ */ import path from 'path'; +import fs from 'fs/promises'; import { logger } from '../utils/logger'; import { execFileNoThrow } from '../utils/execFile'; -// Types imported for documentation and future use -// import type { ActiveContribution, SymphonyIssue } from '../../shared/symphony-types'; +import type { DocumentReference } from '../../shared/symphony-types'; const LOG_CONTEXT = '[SymphonyRunner]'; +/** + * Clean up local repository directory on failure. + */ +async function cleanupLocalRepo(localPath: string): Promise { + try { + await fs.rm(localPath, { recursive: true, force: true }); + logger.info('Cleaned up local repository', LOG_CONTEXT, { localPath }); + } catch (error) { + logger.warn('Failed to cleanup local repository', LOG_CONTEXT, { localPath, error }); + } +} + export interface SymphonyRunnerOptions { contributionId: string; repoSlug: string; repoUrl: string; issueNumber: number; issueTitle: string; - documentPaths: string[]; + documentPaths: DocumentReference[]; localPath: string; branchName: string; onProgress?: (progress: { completedDocuments: number; totalDocuments: number }) => void; @@ -42,6 +54,23 @@ async function createBranch(localPath: string, branchName: string): Promise { + const nameResult = await execFileNoThrow('git', ['config', 'user.name', 'Maestro Symphony'], localPath); + if (nameResult.exitCode !== 0) { + logger.warn('Failed to set git user.name', LOG_CONTEXT, { error: nameResult.stderr }); + return false; + } + const emailResult = await execFileNoThrow('git', ['config', 'user.email', 'symphony@runmaestro.ai'], localPath); + if (emailResult.exitCode !== 0) { + logger.warn('Failed to set git user.email', LOG_CONTEXT, { error: emailResult.stderr }); + return false; + } + return true; +} + /** * Create an empty commit to enable pushing without changes. */ @@ -98,19 +127,58 @@ Closes #${issueNumber} } /** - * Copy Auto Run documents from repo to local Auto Run Docs folder. + * Download a file from a URL. + */ +async function downloadFile(url: string, destPath: string): Promise { + try { + const response = await fetch(url); + if (!response.ok) { + logger.error('Failed to download file', LOG_CONTEXT, { url, status: response.status }); + return false; + } + const buffer = await response.arrayBuffer(); + await fs.writeFile(destPath, Buffer.from(buffer)); + return true; + } catch (error) { + logger.error('Error downloading file', LOG_CONTEXT, { url, error }); + return false; + } +} + +/** + * Copy or download Auto Run documents to local Auto Run Docs folder. + * Handles both repo-relative paths and external URLs (GitHub attachments). */ async function setupAutoRunDocs( localPath: string, - documentPaths: string[] + documentPaths: DocumentReference[] ): Promise { const autoRunPath = path.join(localPath, 'Auto Run Docs'); - await execFileNoThrow('mkdir', ['-p', autoRunPath]); + await fs.mkdir(autoRunPath, { recursive: true }); - for (const docPath of documentPaths) { - const sourcePath = path.join(localPath, docPath); - const destPath = path.join(autoRunPath, path.basename(docPath)); - await execFileNoThrow('cp', [sourcePath, destPath]); + for (const doc of documentPaths) { + const destPath = path.join(autoRunPath, doc.name); + + if (doc.isExternal) { + // Download external file (GitHub attachment) + logger.info('Downloading external document', LOG_CONTEXT, { name: doc.name, url: doc.path }); + const success = await downloadFile(doc.path, destPath); + if (!success) { + logger.warn('Failed to download document, skipping', LOG_CONTEXT, { name: doc.name }); + } + } else { + // Copy from repo using Node.js fs API + const sourcePath = path.join(localPath, doc.path); + try { + await fs.copyFile(sourcePath, destPath); + } catch (error) { + logger.warn('Failed to copy document', LOG_CONTEXT, { + name: doc.name, + source: sourcePath, + error: error instanceof Error ? error.message : String(error) + }); + } + } } return autoRunPath; @@ -154,23 +222,30 @@ export async function startContribution(options: SymphonyRunnerOptions): Promise // 2. Create branch onStatusChange?.('setting_up'); if (!await createBranch(localPath, branchName)) { + await cleanupLocalRepo(localPath); return { success: false, error: 'Branch creation failed' }; } + // 2.5. Configure git user for commits + await configureGitUser(localPath); + // 3. Empty commit const commitMessage = `[Symphony] Start contribution for #${issueNumber}`; if (!await createEmptyCommit(localPath, commitMessage)) { + await cleanupLocalRepo(localPath); return { success: false, error: 'Empty commit failed' }; } // 4. Push branch if (!await pushBranch(localPath, branchName)) { + await cleanupLocalRepo(localPath); return { success: false, error: 'Push failed' }; } // 5. Create draft PR const prResult = await createDraftPR(localPath, issueNumber, issueTitle); if (!prResult.success) { + await cleanupLocalRepo(localPath); return { success: false, error: prResult.error }; } @@ -187,6 +262,7 @@ export async function startContribution(options: SymphonyRunnerOptions): Promise autoRunPath, }; } catch (error) { + await cleanupLocalRepo(localPath); return { success: false, error: error instanceof Error ? error.message : 'Unknown error', @@ -203,6 +279,9 @@ export async function finalizeContribution( issueNumber: number, issueTitle: string ): Promise<{ success: boolean; prUrl?: string; error?: string }> { + // Configure git user for commits (in case not already configured) + await configureGitUser(localPath); + // Commit all changes await execFileNoThrow('git', ['add', '-A'], localPath); @@ -281,9 +360,9 @@ export async function cancelContribution( logger.warn('Failed to close PR', LOG_CONTEXT, { prNumber, error: closeResult.stderr }); } - // Clean up local directory + // Clean up local directory using Node.js fs API if (cleanup) { - await execFileNoThrow('rm', ['-rf', localPath]); + await cleanupLocalRepo(localPath); } return { success: true }; diff --git a/src/renderer/components/SymphonyModal.tsx b/src/renderer/components/SymphonyModal.tsx index 6fa35c80..29932899 100644 --- a/src/renderer/components/SymphonyModal.tsx +++ b/src/renderer/components/SymphonyModal.tsx @@ -290,8 +290,8 @@ function IssueCard({ {issue.documentPaths.length > 0 && (
- {issue.documentPaths.slice(0, 2).map((path, i) => ( -
• {path}
+ {issue.documentPaths.slice(0, 2).map((doc, i) => ( +
• {doc.name}
))} {issue.documentPaths.length > 2 && (
...and {issue.documentPaths.length - 2} more
@@ -491,17 +491,17 @@ function RepositoryDetailView({ {/* Document tabs */}
- {selectedIssue.documentPaths.map((path) => ( + {selectedIssue.documentPaths.map((doc) => ( ))}
@@ -1079,7 +1079,7 @@ export function SymphonyModal({ aria-modal="true" aria-labelledby="symphony-modal-title" tabIndex={-1} - className="w-[1000px] max-w-[95vw] rounded-xl shadow-2xl border overflow-hidden flex flex-col max-h-[85vh] outline-none" + className="w-[1200px] max-w-[95vw] rounded-xl shadow-2xl border overflow-hidden flex flex-col max-h-[85vh] outline-none" style={{ backgroundColor: theme.colors.bgActivity, borderColor: theme.colors.border }} > {/* Detail view for projects */} diff --git a/src/shared/symphony-types.ts b/src/shared/symphony-types.ts index 14e763b0..d98752e0 100644 --- a/src/shared/symphony-types.ts +++ b/src/shared/symphony-types.ts @@ -69,6 +69,22 @@ export type SymphonyCategory = // GitHub Issue Types (Fetched via GitHub API) // ============================================================================ +/** + * Reference to an Auto Run document. + * Supports both repository-relative paths and external URLs (e.g., GitHub attachments). + */ +export interface DocumentReference { + /** Display name (filename without path) */ + name: string; + /** + * For repo-relative paths: the path within the repository (e.g., "docs/task.md") + * For external files: the download URL + */ + path: string; + /** Whether this is an external URL that needs to be downloaded */ + isExternal: boolean; +} + /** * A GitHub issue with the `runmaestro.ai` label. * Represents a contribution opportunity. @@ -90,8 +106,8 @@ export interface SymphonyIssue { createdAt: string; /** When issue was last updated */ updatedAt: string; - /** Parsed Auto Run document paths from issue body */ - documentPaths: string[]; + /** Parsed Auto Run document references from issue body */ + documentPaths: DocumentReference[]; /** Availability status */ status: IssueStatus; /** If in progress, the PR working on it */