diff --git a/apps/desktop/package.json b/apps/desktop/package.json index ad9bbf1d..ed7e357a 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,6 +1,6 @@ { "name": "desktop", - "version": "0.8.2", + "version": "0.8.6", "description": "ClosedLoop Desktop", "author": "ClosedLoop AI ", "private": true, diff --git a/apps/desktop/src/server/operations/symphony-loop.ts b/apps/desktop/src/server/operations/symphony-loop.ts index 8f3e845c..81a99941 100644 --- a/apps/desktop/src/server/operations/symphony-loop.ts +++ b/apps/desktop/src/server/operations/symphony-loop.ts @@ -1,10 +1,10 @@ import { execSync, spawn } from "node:child_process"; -import { gatewayLog } from "../../main/gateway-logger.js"; import crypto from "node:crypto"; import { closeSync, existsSync, mkdirSync, openSync, readFileSync, statSync, unlinkSync, writeFileSync } from "node:fs"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { gatewayLog } from "../../main/gateway-logger.js"; import type { JobStore, LocalJobCommand } from "../../main/job-store.js"; import type { OperationDispatcher, @@ -294,7 +294,7 @@ async function postLoopEvent( loopId: string, token: string, eventBody: Record -): Promise { +): Promise<{ success: boolean; error?: string }> { const url = `${apiBaseUrl}/loops/${loopId}/events`; // Auto-inject timestamp on every event (matches ECS harness reportEvent()) const payload: Record = { @@ -316,14 +316,16 @@ async function postLoopEvent( const text = await resp.text().catch(() => ""); loopError(loopId, `Event POST failed: ${resp.status} ${resp.statusText}`, text); gatewayLog.error("loop-event", `POST ${payload.type} to ${url} failed: ${resp.status} ${resp.statusText} ${text}`); - } else { - loopLog(loopId, `Event POST success: ${resp.status}`); - gatewayLog.debug("loop-event", `POST ${payload.type} to ${url}: ${resp.status}`); + return { success: false, error: "HTTP " + resp.status + " " + resp.statusText }; } + loopLog(loopId, `Event POST success: ${resp.status}`); + gatewayLog.debug("loop-event", `POST ${payload.type} to ${url}: ${resp.status}`); + return { success: true }; } catch (err) { const msg = err instanceof Error ? err.message : String(err); loopError(loopId, "Failed to post event:", err); gatewayLog.error("loop-event", `POST ${payload.type} network error: ${msg}`); + return { success: false, error: msg }; } } @@ -332,7 +334,7 @@ async function uploadArtifacts( loopId: string, token: string, body: Record -): Promise { +): Promise<{ success: boolean; error?: string }> { const url = `${apiBaseUrl}/loops/${loopId}/upload-artifacts`; loopLog(loopId, "Uploading artifacts...", url); try { @@ -348,14 +350,16 @@ async function uploadArtifacts( const text = await resp.text().catch(() => ""); loopError(loopId, `Upload failed: ${resp.status} ${resp.statusText}`, text); gatewayLog.error("loop-upload", `Artifact upload to ${url} failed: ${resp.status} ${resp.statusText} ${text}`); - } else { - loopLog(loopId, `Upload success: ${resp.status}`); - gatewayLog.debug("loop-upload", `Artifact upload to ${url}: ${resp.status}`); + return { success: false, error: `HTTP ${resp.status} ${resp.statusText}` }; } + loopLog(loopId, `Upload success: ${resp.status}`); + gatewayLog.debug("loop-upload", `Artifact upload to ${url}: ${resp.status}`); + return { success: true }; } catch (err) { const msg = err instanceof Error ? err.message : String(err); loopError(loopId, "Failed to upload artifacts:", err); gatewayLog.error("loop-upload", `Artifact upload network error: ${msg}`); + return { success: false, error: msg }; } } @@ -681,15 +685,19 @@ async function attemptLlmCommit( command: string, artifactSlug: string | undefined, webAppOrigin: string, - committer: LoopCommitter | undefined + committer: LoopCommitter | undefined, + onTimeout?: () => void ): Promise { // Build metadata footer for PR body // Strip newlines from user-controlled fields to prevent prompt injection const safeBranch = baseBranch.replace(/[\r\n]/g, ''); const safeLoopId = sanitizeCommitMessage(loopId).replace(/[\r\n]/g, ''); + const safeSlug = artifactSlug + ? sanitizeCommitMessage(artifactSlug).replace(/[\r\n]/g, '') + : null; + let footer: string; - if (artifactSlug) { - const safeSlug = sanitizeCommitMessage(artifactSlug).replace(/[\r\n]/g, ''); + if (safeSlug) { const artifactLink = `${webAppOrigin}/artifact/by-slug/${safeSlug}`; footer = `---\nLoop ID: ${safeLoopId}\nArtifact: ${artifactLink}`; } else { @@ -697,10 +705,10 @@ async function attemptLlmCommit( } // Build slug instruction for the prompt - const slugInstruction = artifactSlug - ? `The artifact slug is ${sanitizeCommitMessage(artifactSlug).replace(/[\r\n]/g, '')}. ` + - `You MUST prefix the PR title with "${sanitizeCommitMessage(artifactSlug).replace(/[\r\n]/g, '')}: " ` + - `(e.g., "${sanitizeCommitMessage(artifactSlug).replace(/[\r\n]/g, '')}: Add feature X"). ` + + const slugInstruction = safeSlug + ? `The artifact slug is ${safeSlug}. ` + + `You MUST prefix the PR title with "${safeSlug}: " ` + + `(e.g., "${safeSlug}: Add feature X"). ` + `Also prefix the commit message the same way.` : "No artifact slug is available — use a descriptive title without a prefix."; @@ -787,6 +795,7 @@ async function attemptLlmCommit( if (!killed) { killed = true; loopError(loopId, "LLM commit timed out after 90s — sending SIGTERM"); + onTimeout?.(); try { process.kill(-pid, "SIGTERM"); } catch (killErr) { @@ -873,6 +882,11 @@ async function attemptLlmCommit( // Git operations (EXECUTE only) // --------------------------------------------------------------------------- +type GitOperationResult = + | { status: 'success'; prUrl: string; prNumber: number; branchName: string; commitSha: string } + | { status: 'no-changes' } + | { status: 'error'; reason: string }; + function executeGitOperations( worktreeDir: string, committer: LoopCommitter | undefined, @@ -881,7 +895,7 @@ function executeGitOperations( command: string, artifactSlug?: string, webAppOrigin?: string -): { prUrl: string; prNumber: number; branchName: string; commitSha: string } | null { +): GitOperationResult { const shortId = loopId.slice(0, 8); const env: Record = { ...process.env } as Record; if (committer) { @@ -891,9 +905,10 @@ function executeGitOperations( env.GIT_COMMITTER_EMAIL = committer.email; } - // Check for changes + // Check for changes, excluding .claude/ which is written by the gateway + // itself (work dir, artifacts) and must never be committed. try { - const status = execSync("git status --porcelain", { + const status = execSync("git status --porcelain -- ':!.claude/'", { cwd: worktreeDir, encoding: "utf-8", stdio: "pipe", @@ -901,10 +916,11 @@ function executeGitOperations( }).trim(); if (!status) { - return null; // No changes + return { status: 'no-changes' }; // No changes } - } catch { - return null; + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + return { status: 'error', reason }; } // Stage, commit, push @@ -1029,13 +1045,25 @@ function executeGitOperations( // Non-critical — PR exists, metadata is best-effort } - return { prUrl, prNumber, branchName, commitSha }; + return { status: 'success', prUrl, prNumber, branchName, commitSha }; } catch (err) { - console.error("[symphony-loop] Git operations failed:", err); - return null; + const reason = err instanceof Error ? err.message : String(err); + return { status: 'error', reason }; } } +// --------------------------------------------------------------------------- +// Sanitize error messages before persisting to job store +// --------------------------------------------------------------------------- + +function sanitizeErrorMessage(msg: string): string { + return msg + .replace(/:\/\/[^@]+@/g, '://***@') + .replace(/\b[0-9a-f]{20,}\b/gi, '[REDACTED]') + .replace(/\b[A-Za-z0-9+/]{40,}={0,2}\b/g, '[REDACTED]') + .slice(0, 500); +} + // --------------------------------------------------------------------------- // Process completion handler (async, runs after spawn) // --------------------------------------------------------------------------- @@ -1088,9 +1116,10 @@ async function handleProcessCompletion( } // Read outputs per command - gatewayLog.debug("loop-harness", `${command} succeeded (exit 0), reading artifacts for loopId=${loopId}`); + gatewayLog.info("loop-harness", `${command} succeeded (exit 0), reading artifacts for loopId=${loopId}`); let artifacts: Record = {}; const metadata: Record = {}; + const warnings: string[] = []; if (command === "PLAN" || command === "REQUEST_CHANGES") { artifacts = readPlanOutputs(claudeWorkDir); @@ -1110,7 +1139,8 @@ async function handleProcessCompletion( command, body.artifactSlug, webAppOrigin ?? "", - committer + committer, + () => { warnings.push(sanitizeErrorMessage('LLM commit timed out after 90s')); } ); // Clean up any remaining LLM scratch files before fallback to prevent @@ -1122,10 +1152,11 @@ async function handleProcessCompletion( try { unlinkSync(path.join(worktreeDir, 'pr-body.md')); } catch { /* may not exist */ } } - const gitResult: { prUrl: string; prNumber: number; branchName: string; commitSha: string } | null = - llmResult ?? executeGitOperations(worktreeDir, committer, baseBranch, loopId, command, body.artifactSlug, webAppOrigin ?? ""); + const gitResult: GitOperationResult = llmResult + ? { status: 'success' as const, ...llmResult } + : executeGitOperations(worktreeDir, committer, baseBranch, loopId, command, body.artifactSlug, webAppOrigin ?? ""); - if (gitResult) { + if (gitResult.status === 'success') { // Merge git info into execution result const execResult = (artifacts.executionResult as Record) ?? {}; @@ -1137,6 +1168,11 @@ async function handleProcessCompletion( execResult.base_branch = baseBranch; artifacts.executionResult = execResult; metadata.branchName = gitResult.branchName; + } else if (gitResult.status === 'no-changes') { + gatewayLog.info('loop-harness', 'no local changes detected, skipping PR creation, loopId=' + loopId); + } else if (gitResult.status === 'error') { + gatewayLog.warn('loop-harness', 'git operations failed: ' + sanitizeErrorMessage(gitResult.reason) + ', loopId=' + loopId); + warnings.push('GIT_PUSH_FAILED'); } } } else if (command === "DECOMPOSE") { @@ -1157,11 +1193,15 @@ async function handleProcessCompletion( // Upload artifacts const artifactKeys = Object.keys(artifacts); loopLog(loopId, "Artifact keys:", artifactKeys); - gatewayLog.debug("loop-harness", `Uploading artifacts for ${command} loopId=${loopId}: [${artifactKeys.join(", ")}]`); - await uploadArtifacts(apiBaseUrl, loopId, closedLoopAuthToken, { + gatewayLog.info("loop-harness", `Uploading artifacts for ${command} loopId=${loopId}: [${artifactKeys.join(", ")}]`); + const uploadResult = await uploadArtifacts(apiBaseUrl, loopId, closedLoopAuthToken, { artifacts, metadata, }); + if (!uploadResult.success) { + gatewayLog.warn('loop-harness', 'Artifact upload failed: ' + (uploadResult.error ?? 'unknown error') + ', loopId=' + loopId); + warnings.push('ARTIFACT_UPLOAD_FAILED'); + } // Parse token usage from claude output const tokensUsed = parseTokenUsage(claudeWorkDir); @@ -1209,10 +1249,15 @@ async function handleProcessCompletion( result, tokensUsed, loopId, + ...(warnings.length > 0 ? { warnings } : {}), }; loopLog(loopId, "Posting completed event..."); - await postLoopEvent(apiBaseUrl, loopId, closedLoopAuthToken, completedEvent); + const eventResult = await postLoopEvent(apiBaseUrl, loopId, closedLoopAuthToken, completedEvent); + if (!eventResult.success) { + gatewayLog.warn('loop-harness', 'Completed event POST failed: ' + (eventResult.error ?? 'unknown error') + ', loopId=' + loopId); + warnings.push('EVENT_POST_FAILED'); + } loopLog(loopId, "Loop completed successfully"); if (jobStore) { @@ -1225,6 +1270,7 @@ async function handleProcessCompletion( exitCode: 0, updatedAt: now, completedAt: now, + warning: warnings.length > 0 ? warnings.map(sanitizeErrorMessage).join('; ') : undefined, }); } } diff --git a/apps/desktop/test/symphony-loop-cloud-failures.test.ts b/apps/desktop/test/symphony-loop-cloud-failures.test.ts new file mode 100644 index 00000000..891fbf18 --- /dev/null +++ b/apps/desktop/test/symphony-loop-cloud-failures.test.ts @@ -0,0 +1,281 @@ +/** + * Integration tests for cloud failure scenarios in the symphony loop: + * + * T-4.2: Cloud failure scenarios + * - Artifact upload failure sets ARTIFACT_UPLOAD_FAILED in job store warning + * and in completed event warnings + * - Event post failure is reflected in job store warning (EVENT_POST_FAILED) + * + * Tests go through the HTTP gateway, not direct function calls. + * Fake binaries (run-loop.sh, claude, git, gh) are placed in a temp fake-bin/ dir + * prepended to PATH. CLOSEDLOOP_SYMPHONY_TEST_RAW_CLAUDE_PIPELINE=1 disables the + * stream_formatter pipeline so the fake claude can emit simple output. + */ + +import assert from "node:assert/strict"; +import fs from "node:fs/promises"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, test } from "node:test"; +import { JobStore } from "../src/main/job-store.js"; +import { DesktopGatewayServer } from "../src/server/server.js"; +import { EMPTY_CAPABILITIES, PORT_PROBE_ORDER } from "../src/shared/contracts.js"; +import { + createFakeRunLoopScript, + initGitRepo, + restoreEnv, + saveEnv, + startMockApiServer, + waitForCompletedEvent, +} from "./symphony-test-utils.js"; + +// --------------------------------------------------------------------------- +// Shared state and cleanup +// --------------------------------------------------------------------------- + +const serversToClose: DesktopGatewayServer[] = []; +const mockServersToClose: http.Server[] = []; +const tempPathsToClean: string[] = []; +const savedEnv = saveEnv(); + +afterEach(async () => { + restoreEnv(savedEnv); + + for (const server of serversToClose.splice(0)) { + await server.stop(); + } + + for (const ms of mockServersToClose.splice(0)) { + await new Promise((resolve, reject) => { + ms.close((err) => (err ? reject(err) : resolve())); + }); + } + + for (const tempPath of tempPathsToClean.splice(0)) { + await fs.rm(tempPath, { recursive: true, force: true }); + } +}); + +/** + * Poll a JobStore until the job for the given loopId reaches a terminal status, + * or until the timeout elapses. + */ +async function waitForJobTerminal( + jobStore: JobStore, + loopId: string, + timeoutMs = 20_000 +): Promise { + const deadline = Date.now() + timeoutMs; + const terminalStatuses = new Set(["COMPLETED", "FAILED", "CANCELLED", "STOPPED", "UNKNOWN"]); + while (Date.now() < deadline) { + const job = jobStore.getByLoopId(loopId); + if (job && terminalStatuses.has(job.status)) { + return job; + } + await new Promise((resolve) => setTimeout(resolve, 100)); + } + throw new Error( + `Timed out waiting for terminal job status for loopId=${loopId} after ${timeoutMs}ms` + ); +} + +// --------------------------------------------------------------------------- +// Test 1: Artifact upload failure sets ARTIFACT_UPLOAD_FAILED in completed event +// warnings and in the job store warning field +// --------------------------------------------------------------------------- + +test("EXECUTE: artifact upload failure sets ARTIFACT_UPLOAD_FAILED in completed event warnings", async () => { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "cloud-fail-upload-")); + tempPathsToClean.push(tmpDir); + + const repoPath = path.join(tmpDir, "repo-upload-fail"); + await initGitRepo(repoPath); + + const worktreeParent = path.join(tmpDir, "worktrees"); + await fs.mkdir(worktreeParent, { recursive: true }); + + process.env.HOME = tmpDir; + + // fake run-loop.sh: exits 0 without making any changes + await createFakeRunLoopScript(tmpDir, "#!/bin/sh\nexit 0\n"); + + // fake-bin: claude exits 0 without writing execution-result.json + const fakeBin = path.join(tmpDir, "fake-bin"); + await fs.mkdir(fakeBin, { recursive: true }); + await fs.writeFile( + path.join(fakeBin, "claude"), + "#!/bin/sh\nexit 0\n", + { mode: 0o755 } + ); + + process.env.CLOSEDLOOP_SYMPHONY_TEST_RAW_CLAUDE_PIPELINE = "1"; + process.env.SYMPHONY_WORKTREE_PARENT_DIR = worktreeParent; + process.env.PATH = `${fakeBin}:/usr/bin:/bin`; + + // Configure mock server to return 500 for upload-artifacts requests + const failUrls = new Map([["upload-artifacts", 500]]); + const mock = await startMockApiServer(failUrls); + mockServersToClose.push(mock.server); + + // Provide a real JobStore so we can verify the warning field + const jobStore = new JobStore({ cwd: tmpDir, name: "test-jobs-upload-fail" }); + + const server = new DesktopGatewayServer({ + host: "127.0.0.1", + preferredPort: PORT_PROBE_ORDER[0], + fallbackPorts: PORT_PROBE_ORDER.slice(1), + webAppOrigin: "https://app.symphony.com", + getAllowedDirectories: () => [tmpDir], + machineName: "cloud-fail-upload-machine", + version: "0.1.0-test", + capabilities: EMPTY_CAPABILITIES, + discoveryFilePath: path.join(tmpDir, "electron-port"), + getApiOrigin: () => `http://127.0.0.1:${mock.port}`, + jobStore, + }); + serversToClose.push(server); + await server.start(); + + const loopId = "00000000-0000-0000-0000-000000000500"; + const response = await fetch( + `http://127.0.0.1:${server.getActivePort()}/api/engineer/symphony/loop`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + loopId, + command: "EXECUTE", + closedLoopAuthToken: "tok", + artifacts: [], + repo: { fullName: `upload-fail/${path.basename(repoPath)}`, branch: "main" }, + }), + } + ); + + assert.equal( + response.status, + 200, + `Expected 200 but got ${response.status}: ${await response.text().catch(() => "")}` + ); + + // The completed event is posted after the upload attempt. Wait for it. + const completedEvent = await waitForCompletedEvent(mock.requests, loopId); + + // Assert ARTIFACT_UPLOAD_FAILED is in the completed event warnings + const warnings = completedEvent.warnings as string[] | undefined; + assert.ok( + Array.isArray(warnings) && warnings.includes("ARTIFACT_UPLOAD_FAILED"), + `Expected ARTIFACT_UPLOAD_FAILED in completed event warnings, got: ${JSON.stringify(warnings)}` + ); + + // Also verify the job store warning field contains ARTIFACT_UPLOAD_FAILED + const job = await waitForJobTerminal(jobStore, loopId); + assert.ok( + typeof job.warning === "string" && job.warning.includes("ARTIFACT_UPLOAD_FAILED"), + `Expected job store warning to contain ARTIFACT_UPLOAD_FAILED, got: ${JSON.stringify(job.warning)}` + ); +}); + +// --------------------------------------------------------------------------- +// Test 2: Event post failure is reflected in job store warning (EVENT_POST_FAILED) +// --------------------------------------------------------------------------- + +test("EXECUTE: event post failure logged as warning in job store", async () => { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "cloud-fail-event-")); + tempPathsToClean.push(tmpDir); + + const repoPath = path.join(tmpDir, "repo-event-fail"); + await initGitRepo(repoPath); + + const worktreeParent = path.join(tmpDir, "worktrees"); + await fs.mkdir(worktreeParent, { recursive: true }); + + process.env.HOME = tmpDir; + + // fake run-loop.sh: exits 0 without making any changes + await createFakeRunLoopScript(tmpDir, "#!/bin/sh\nexit 0\n"); + + // fake-bin: claude exits 0 without writing execution-result.json + const fakeBin = path.join(tmpDir, "fake-bin"); + await fs.mkdir(fakeBin, { recursive: true }); + await fs.writeFile( + path.join(fakeBin, "claude"), + "#!/bin/sh\nexit 0\n", + { mode: 0o755 } + ); + + process.env.CLOSEDLOOP_SYMPHONY_TEST_RAW_CLAUDE_PIPELINE = "1"; + process.env.SYMPHONY_WORKTREE_PARENT_DIR = worktreeParent; + process.env.PATH = `${fakeBin}:/usr/bin:/bin`; + + // Configure mock server to return 500 for all /events requests. + // This causes both the "started" event and the "completed" event to fail. + // The loop should still complete (not crash) and set EVENT_POST_FAILED in + // the job store warning field. + const failUrls = new Map([["events", 500]]); + const mock = await startMockApiServer(failUrls); + mockServersToClose.push(mock.server); + + // Provide a real JobStore so we can verify the warning field + const jobStore = new JobStore({ cwd: tmpDir, name: "test-jobs-event-fail" }); + + const server = new DesktopGatewayServer({ + host: "127.0.0.1", + preferredPort: PORT_PROBE_ORDER[0], + fallbackPorts: PORT_PROBE_ORDER.slice(1), + webAppOrigin: "https://app.symphony.com", + getAllowedDirectories: () => [tmpDir], + machineName: "cloud-fail-event-machine", + version: "0.1.0-test", + capabilities: EMPTY_CAPABILITIES, + discoveryFilePath: path.join(tmpDir, "electron-port"), + getApiOrigin: () => `http://127.0.0.1:${mock.port}`, + jobStore, + }); + serversToClose.push(server); + await server.start(); + + const loopId = "00000000-0000-0000-0000-000000000600"; + const response = await fetch( + `http://127.0.0.1:${server.getActivePort()}/api/engineer/symphony/loop`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + loopId, + command: "EXECUTE", + closedLoopAuthToken: "tok", + artifacts: [], + repo: { fullName: `event-fail/${path.basename(repoPath)}`, branch: "main" }, + }), + } + ); + + assert.equal( + response.status, + 200, + `Expected 200 but got ${response.status}: ${await response.text().catch(() => "")}` + ); + + // Wait for upload-artifacts to confirm the loop progressed past the run phase. + // (upload-artifacts is not in failUrls so it succeeds and can be waited on) + await mock.waitForRequest("upload-artifacts"); + + // Wait for the job to reach a terminal state in the job store. + // Even though the completed event POST fails, the loop still finalizes the job. + const job = await waitForJobTerminal(jobStore, loopId); + + // The loop completes without crashing (status is COMPLETED, not FAILED) + assert.equal( + job.status, + "COMPLETED", + `Expected job status COMPLETED after event post failure, got: ${job.status}` + ); + + // EVENT_POST_FAILED should appear in the job store warning field + assert.ok( + typeof job.warning === "string" && job.warning.includes("EVENT_POST_FAILED"), + `Expected job store warning to contain EVENT_POST_FAILED, got: ${JSON.stringify(job.warning)}` + ); +}); diff --git a/apps/desktop/test/symphony-loop-execute.test.ts b/apps/desktop/test/symphony-loop-execute.test.ts index 5133febd..8daaf198 100644 --- a/apps/desktop/test/symphony-loop-execute.test.ts +++ b/apps/desktop/test/symphony-loop-execute.test.ts @@ -17,17 +17,21 @@ */ import assert from "node:assert/strict"; -import { execFile, execSync } from "node:child_process"; import fs from "node:fs/promises"; import http from "node:http"; import os from "node:os"; import path from "node:path"; import { afterEach, test } from "node:test"; -import { promisify } from "node:util"; import { DesktopGatewayServer } from "../src/server/server.js"; import { EMPTY_CAPABILITIES, PORT_PROBE_ORDER } from "../src/shared/contracts.js"; - -const execFileAsync = promisify(execFile); +import { + createFakeRunLoopScript, + initGitRepo, + restoreEnv, + saveEnv, + startMockApiServer, + waitForCompletedEvent, +} from "./symphony-test-utils.js"; // --------------------------------------------------------------------------- // Shared state and cleanup @@ -36,36 +40,10 @@ const execFileAsync = promisify(execFile); const serversToClose: DesktopGatewayServer[] = []; const mockServersToClose: http.Server[] = []; const tempPathsToClean: string[] = []; - -const originalSymphonyWorktreeParentDir = process.env.SYMPHONY_WORKTREE_PARENT_DIR; -const originalPath = process.env.PATH; -const originalHome = process.env.HOME; -const originalRawPipeline = process.env.CLOSEDLOOP_SYMPHONY_TEST_RAW_CLAUDE_PIPELINE; +const savedEnv = saveEnv(); afterEach(async () => { - if (originalSymphonyWorktreeParentDir === undefined) { - delete process.env.SYMPHONY_WORKTREE_PARENT_DIR; - } else { - process.env.SYMPHONY_WORKTREE_PARENT_DIR = originalSymphonyWorktreeParentDir; - } - - if (originalPath === undefined) { - delete process.env.PATH; - } else { - process.env.PATH = originalPath; - } - - if (originalHome === undefined) { - delete process.env.HOME; - } else { - process.env.HOME = originalHome; - } - - if (originalRawPipeline === undefined) { - delete process.env.CLOSEDLOOP_SYMPHONY_TEST_RAW_CLAUDE_PIPELINE; - } else { - process.env.CLOSEDLOOP_SYMPHONY_TEST_RAW_CLAUDE_PIPELINE = originalRawPipeline; - } + restoreEnv(savedEnv); for (const server of serversToClose.splice(0)) { await server.stop(); @@ -82,113 +60,6 @@ afterEach(async () => { } }); -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -async function initGitRepo(repoPath: string): Promise { - await execFileAsync("git", ["init", "-b", "main", repoPath]); - await execFileAsync("git", ["-C", repoPath, "config", "user.email", "test@test.com"]); - await execFileAsync("git", ["-C", repoPath, "config", "user.name", "Test"]); - await fs.writeFile(path.join(repoPath, "README.md"), "# initial\n"); - await execFileAsync("git", ["-C", repoPath, "add", "."]); - await execFileAsync("git", ["-C", repoPath, "commit", "-m", "initial"]); -} - -type RecordedRequest = { method: string; url: string; body: string }; - -async function startMockApiServer(): Promise<{ - server: http.Server; - port: number; - requests: RecordedRequest[]; - waitForRequest: (urlSubstring: string, timeoutMs?: number) => Promise; -}> { - const requests: RecordedRequest[] = []; - const waiters: Array<{ urlSubstring: string; resolve: (r: RecordedRequest) => void }> = []; - - const server = http.createServer((req, res) => { - void (async () => { - const chunks: Buffer[] = []; - for await (const chunk of req) { - chunks.push(typeof chunk === "string" ? Buffer.from(chunk) : chunk); - } - const recorded: RecordedRequest = { - method: req.method ?? "", - url: req.url ?? "", - body: Buffer.concat(chunks).toString("utf-8"), - }; - requests.push(recorded); - - for (let i = waiters.length - 1; i >= 0; i--) { - if (recorded.url.includes(waiters[i].urlSubstring)) { - waiters[i].resolve(recorded); - waiters.splice(i, 1); - } - } - - res.statusCode = 200; - res.setHeader("content-type", "application/json"); - res.end(JSON.stringify({ success: true })); - })(); - }); - - await new Promise((resolve, reject) => { - server.listen(0, "127.0.0.1", () => resolve()); - server.once("error", reject); - }); - - const address = server.address(); - if (!address || typeof address === "string") { - throw new Error("failed to bind mock API server"); - } - - function waitForRequest(urlSubstring: string, timeoutMs = 20_000): Promise { - const existing = requests.find((r) => r.url.includes(urlSubstring)); - if (existing) { - return Promise.resolve(existing); - } - return new Promise((resolve, reject) => { - const timer = setTimeout(() => { - reject( - new Error( - `Timed out waiting for request matching "${urlSubstring}" after ${timeoutMs}ms` - ) - ); - }, timeoutMs); - waiters.push({ - urlSubstring, - resolve: (r) => { - clearTimeout(timer); - resolve(r); - }, - }); - }); - } - - return { server, port: address.port, requests, waitForRequest }; -} - -/** - * Create the fake plugin cache structure so findPluginScript("code", "run-loop.sh") - * finds the provided script content. - */ -async function createFakeRunLoopScript(homeDir: string, scriptContent: string): Promise { - const scriptDir = path.join( - homeDir, - ".claude", - "plugins", - "cache", - "closedloop-ai", - "code", - "1.0.0", - "scripts" - ); - await fs.mkdir(scriptDir, { recursive: true }); - const scriptPath = path.join(scriptDir, "run-loop.sh"); - await fs.writeFile(scriptPath, scriptContent, { mode: 0o755 }); - return scriptPath; -} - // --------------------------------------------------------------------------- // Test 1: No-changes → executeGitOperations returns null (no PR URL in upload) // --------------------------------------------------------------------------- @@ -288,6 +159,14 @@ test("EXECUTE: no PR URL in upload when worktree has no changes (git status empt undefined, "Expected has_changes to be absent when there are no changes" ); + + // Also check the completed event does NOT contain GIT_PUSH_FAILED in warnings. + // The completed event is posted after upload-artifacts, so poll until it appears. + const completedEvent = await waitForCompletedEvent(mock.requests, loopId); + assert.ok( + !(completedEvent.warnings as string[] | undefined)?.includes("GIT_PUSH_FAILED"), + `Expected no GIT_PUSH_FAILED warning in completed event for no-changes path, got warnings: ${JSON.stringify(completedEvent.warnings)}` + ); }); // --------------------------------------------------------------------------- @@ -552,3 +431,101 @@ test("EXECUTE: uses existing PR URL from gh pr view without calling gh pr create `gh pr create should not have been called, but capture file contains: ${ghCalls}` ); }); + +// --------------------------------------------------------------------------- +// Test 4: git status exits 1 → executeGitOperations returns 'error' → +// completed event warnings contains 'GIT_PUSH_FAILED' +// --------------------------------------------------------------------------- + +test("EXECUTE: git status failure sets GIT_PUSH_FAILED in completed event warnings", async () => { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "execute-gitstatus-fail-")); + tempPathsToClean.push(tmpDir); + + const repoPath = path.join(tmpDir, "repo-gitstatus-fail"); + await initGitRepo(repoPath); + + const worktreeParent = path.join(tmpDir, "worktrees"); + await fs.mkdir(worktreeParent, { recursive: true }); + + process.env.HOME = tmpDir; + + // fake run-loop.sh: exits 0 (loop runs successfully, no LLM commits) + await createFakeRunLoopScript(tmpDir, "#!/bin/sh\nexit 0\n"); + + const fakeBin = path.join(tmpDir, "fake-bin"); + await fs.mkdir(fakeBin, { recursive: true }); + + // fake claude: exits 0 without writing execution-result.json + // → attemptLlmCommit returns null → falls through to executeGitOperations + await fs.writeFile( + path.join(fakeBin, "claude"), + "#!/bin/sh\nexit 0\n", + { mode: 0o755 } + ); + + // fake git: delegates most commands to real git, but exits 1 for 'status --porcelain'. + // This causes executeGitOperations to return { status: 'error' }, which adds + // GIT_PUSH_FAILED to the warnings array posted in the completed event. + const fakeGitScript = [ + "#!/bin/sh", + "# Exit 1 for 'git status --porcelain' to simulate a git status failure", + "if [ \"$1\" = status ]; then exit 1; fi", + "# Delegate everything else (worktree, fetch, rev-parse, etc.) to real git", + `exec /usr/bin/git "$@"`, + ].join("\n"); + await fs.writeFile(path.join(fakeBin, "git"), fakeGitScript, { mode: 0o755 }); + + process.env.CLOSEDLOOP_SYMPHONY_TEST_RAW_CLAUDE_PIPELINE = "1"; + process.env.SYMPHONY_WORKTREE_PARENT_DIR = worktreeParent; + process.env.PATH = `${fakeBin}:/usr/bin:/bin`; + + const mock = await startMockApiServer(); + mockServersToClose.push(mock.server); + + const server = new DesktopGatewayServer({ + host: "127.0.0.1", + preferredPort: PORT_PROBE_ORDER[0], + fallbackPorts: PORT_PROBE_ORDER.slice(1), + webAppOrigin: "https://app.symphony.com", + getAllowedDirectories: () => [tmpDir], + machineName: "execute-gitstatus-fail-machine", + version: "0.1.0-test", + capabilities: EMPTY_CAPABILITIES, + discoveryFilePath: path.join(tmpDir, "electron-port"), + getApiOrigin: () => `http://127.0.0.1:${mock.port}`, + }); + serversToClose.push(server); + await server.start(); + + const loopId = "00000000-0000-0000-0000-000000000400"; + const response = await fetch( + `http://127.0.0.1:${server.getActivePort()}/api/engineer/symphony/loop`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + loopId, + command: "EXECUTE", + closedLoopAuthToken: "tok", + artifacts: [], + repo: { fullName: `gitstatus-fail/${path.basename(repoPath)}`, branch: "main" }, + }), + } + ); + + assert.equal( + response.status, + 200, + `Expected 200 but got ${response.status}: ${await response.text().catch(() => "")}` + ); + + // Wait for the completed event and assert GIT_PUSH_FAILED is in warnings. + // The loop posts upload-artifacts first, then the completed event. + await mock.waitForRequest("upload-artifacts"); + const completedEvent = await waitForCompletedEvent(mock.requests, loopId); + const warnings = completedEvent.warnings as string[] | undefined; + assert.ok( + Array.isArray(warnings) && warnings.includes("GIT_PUSH_FAILED"), + `Expected GIT_PUSH_FAILED in completed event warnings when git status exits 1, got warnings: ${JSON.stringify(warnings)}` + ); +}); diff --git a/apps/desktop/test/symphony-test-utils.ts b/apps/desktop/test/symphony-test-utils.ts new file mode 100644 index 00000000..4d97bfbb --- /dev/null +++ b/apps/desktop/test/symphony-test-utils.ts @@ -0,0 +1,231 @@ +/** + * Shared test helpers for symphony loop integration tests. + * + * Extracted from symphony-loop-execute.test.ts and + * symphony-loop-cloud-failures.test.ts to eliminate duplication. + */ + +import { execFile } from "node:child_process"; +import fs from "node:fs/promises"; +import http from "node:http"; +import path from "node:path"; +import { promisify } from "node:util"; + +const execFileAsync = promisify(execFile); + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +export type RecordedRequest = { method: string; url: string; body: string }; + +// --------------------------------------------------------------------------- +// Environment save/restore +// --------------------------------------------------------------------------- + +const ENV_KEYS = [ + "SYMPHONY_WORKTREE_PARENT_DIR", + "PATH", + "HOME", + "CLOSEDLOOP_SYMPHONY_TEST_RAW_CLAUDE_PIPELINE", +] as const; + +export function saveEnv(): Record { + const saved: Record = {}; + for (const key of ENV_KEYS) { + saved[key] = process.env[key]; + } + return saved; +} + +export function restoreEnv(saved: Record): void { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } +} + +// --------------------------------------------------------------------------- +// Git helpers +// --------------------------------------------------------------------------- + +export async function initGitRepo(repoPath: string): Promise { + await execFileAsync("git", ["init", "-b", "main", repoPath]); + await execFileAsync("git", ["-C", repoPath, "config", "user.email", "test@test.com"]); + await execFileAsync("git", ["-C", repoPath, "config", "user.name", "Test"]); + await fs.writeFile(path.join(repoPath, "README.md"), "# initial\n"); + await execFileAsync("git", ["-C", repoPath, "add", "."]); + await execFileAsync("git", ["-C", repoPath, "commit", "-m", "initial"]); +} + +// --------------------------------------------------------------------------- +// Mock API server +// --------------------------------------------------------------------------- + +/** + * Start a mock API server. When failUrls is provided, any request whose URL + * contains a key from the map will receive the mapped status code and an error + * body. All other requests receive HTTP 200. + */ +export async function startMockApiServer(failUrls?: Map): Promise<{ + server: http.Server; + port: number; + requests: RecordedRequest[]; + waitForRequest: (urlSubstring: string, timeoutMs?: number) => Promise; +}> { + const requests: RecordedRequest[] = []; + const waiters: Array<{ urlSubstring: string; resolve: (r: RecordedRequest) => void }> = []; + + const server = http.createServer((req, res) => { + void (async () => { + const chunks: Buffer[] = []; + for await (const chunk of req) { + chunks.push(typeof chunk === "string" ? Buffer.from(chunk) : chunk); + } + const recorded: RecordedRequest = { + method: req.method ?? "", + url: req.url ?? "", + body: Buffer.concat(chunks).toString("utf-8"), + }; + requests.push(recorded); + + for (let i = waiters.length - 1; i >= 0; i--) { + if (recorded.url.includes(waiters[i].urlSubstring)) { + waiters[i].resolve(recorded); + waiters.splice(i, 1); + } + } + + // Check if this request should fail + let failStatus: number | undefined; + if (failUrls) { + for (const [urlSubstring, status] of failUrls) { + if (recorded.url.includes(urlSubstring)) { + failStatus = status; + break; + } + } + } + + if (failStatus !== undefined) { + res.statusCode = failStatus; + res.setHeader("content-type", "application/json"); + res.end(JSON.stringify({ error: "injected failure" })); + } else { + res.statusCode = 200; + res.setHeader("content-type", "application/json"); + res.end(JSON.stringify({ success: true })); + } + })().catch((err) => { + console.error("Mock server handler error:", err); + if (!res.headersSent) { + res.statusCode = 500; + res.end(); + } + }); + }); + + await new Promise((resolve, reject) => { + server.listen(0, "127.0.0.1", () => resolve()); + server.once("error", reject); + }); + + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("failed to bind mock API server"); + } + + function waitForRequest(urlSubstring: string, timeoutMs = 20_000): Promise { + const existing = requests.find((r) => r.url.includes(urlSubstring)); + if (existing) { + return Promise.resolve(existing); + } + return new Promise((resolve, reject) => { + const entry = { + urlSubstring, + resolve: (r: RecordedRequest) => { + clearTimeout(timer); + resolve(r); + }, + }; + const timer = setTimeout(() => { + const idx = waiters.indexOf(entry); + if (idx !== -1) waiters.splice(idx, 1); + reject( + new Error( + `Timed out waiting for request matching "${urlSubstring}" after ${timeoutMs}ms` + ) + ); + }, timeoutMs); + waiters.push(entry); + }); + } + + return { server, port: address.port, requests, waitForRequest }; +} + +// --------------------------------------------------------------------------- +// Fake plugin script +// --------------------------------------------------------------------------- + +/** + * Create the fake plugin cache structure so findPluginScript("code", "run-loop.sh") + * finds the provided script content. + */ +export async function createFakeRunLoopScript( + homeDir: string, + scriptContent: string +): Promise { + const scriptDir = path.join( + homeDir, + ".claude", + "plugins", + "cache", + "closedloop-ai", + "code", + "1.0.0", + "scripts" + ); + await fs.mkdir(scriptDir, { recursive: true }); + const scriptPath = path.join(scriptDir, "run-loop.sh"); + await fs.writeFile(scriptPath, scriptContent, { mode: 0o755 }); + return scriptPath; +} + +// --------------------------------------------------------------------------- +// Polling helpers +// --------------------------------------------------------------------------- + +/** + * Poll mock.requests until a request to /loops/{loopId}/events with + * type === "completed" is found, or until the timeout elapses. + */ +export async function waitForCompletedEvent( + requests: RecordedRequest[], + loopId: string, + timeoutMs = 20_000 +): Promise> { + const eventsUrlSubstring = `/loops/${loopId}/events`; + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + for (const req of requests) { + if (!req.url.includes(eventsUrlSubstring)) continue; + let parsed: Record; + try { + parsed = JSON.parse(req.body) as Record; + } catch { + continue; + } + if (parsed.type === "completed") { + return parsed; + } + } + await new Promise((resolve) => setTimeout(resolve, 100)); + } + throw new Error( + `Timed out waiting for completed event for loopId=${loopId} after ${timeoutMs}ms` + ); +}