From dffc5274249280825e194132915ce4d2f18f6d4c Mon Sep 17 00:00:00 2001 From: 777genius Date: Tue, 19 May 2026 02:49:45 +0300 Subject: [PATCH] fix(ci): restore dev validation checks --- .../renderer/ui/GraphMemberLogPreviewHud.tsx | 6 +- src/main/ipc/handlers.ts | 8 +- .../services/team/TeamMcpConfigBuilder.ts | 2 +- .../delivery/OpenCodePromptDeliveryLedger.ts | 1 + .../runtime/RuntimeDiagnosticClassifier.ts | 1 + src/main/utils/childProcess.ts | 43 ++-- src/renderer/components/team/TeamListView.tsx | 2 +- .../components/team/members/MemberCard.tsx | 8 +- src/renderer/utils/memberHelpers.ts | 8 + src/renderer/utils/memberLaunchDiagnostics.ts | 1 + test/main/build/smokePackagedApp.test.ts | 5 +- .../extensions/McpInstallService.test.ts | 55 +++- .../TeamAgentLaunchMatrix.safe-e2e.test.ts | 242 +++++++++++------- .../TeamProvisioningServicePrompts.test.ts | 76 +++--- .../GraphMemberLogPreviewHud.test.tsx | 5 +- 15 files changed, 292 insertions(+), 171 deletions(-) diff --git a/src/features/agent-graph/renderer/ui/GraphMemberLogPreviewHud.tsx b/src/features/agent-graph/renderer/ui/GraphMemberLogPreviewHud.tsx index 5a807939..fd06994b 100644 --- a/src/features/agent-graph/renderer/ui/GraphMemberLogPreviewHud.tsx +++ b/src/features/agent-graph/renderer/ui/GraphMemberLogPreviewHud.tsx @@ -28,8 +28,8 @@ import type { const LOG_PREVIEW_FALLBACK_WIDTH = 260; const LOG_PREVIEW_FALLBACK_HEIGHT = 292; const NEW_LOG_HIGHLIGHT_MS = 1_000; -const COMPACT_ROW_TITLE_LIMIT = 28; -const COMPACT_ROW_TEXT_LIMIT = 160; +const COMPACT_ROW_TITLE_LIMIT = 24; +const COMPACT_ROW_TEXT_LIMIT = 110; const COMPACT_ROW_MIN_PREVIEW_LIMIT = 96; const INTERACTIVE_LOG_CONTROL_CLASS = 'pointer-events-auto'; @@ -256,7 +256,7 @@ function renderLoadingSkeleton(): React.JSX.Element { key={index} className="grid h-[72px] min-h-[72px] w-full min-w-0 grid-cols-[1rem_minmax(0,1fr)] gap-x-1.5 overflow-hidden rounded-md border border-white/10 bg-[rgba(8,14,28,0.42)] px-2 py-1.5" > - + diff --git a/src/main/ipc/handlers.ts b/src/main/ipc/handlers.ts index b3e6608b..14623a62 100644 --- a/src/main/ipc/handlers.ts +++ b/src/main/ipc/handlers.ts @@ -45,14 +45,12 @@ import { registerHttpServerHandlers, removeHttpServerHandlers, } from './httpServer'; +import { registerNotificationHandlers, removeNotificationHandlers } from './notifications'; import { initializeOpenCodeRuntimeHandlers, registerOpenCodeRuntimeHandlers, removeOpenCodeRuntimeHandlers, } from './openCodeRuntime'; - -const logger = createLogger('IPC:handlers'); -import { registerNotificationHandlers, removeNotificationHandlers } from './notifications'; import { initializeProjectHandlers, registerProjectHandlers, @@ -79,12 +77,12 @@ import { removeSubagentHandlers, } from './subagents'; import { initializeTeamHandlers, registerTeamHandlers, removeTeamHandlers } from './teams'; +import { registerTelemetryHandlers, removeTelemetryHandlers } from './telemetry'; import { initializeTerminalHandlers, registerTerminalHandlers, removeTerminalHandlers, } from './terminal'; -import { registerTelemetryHandlers, removeTelemetryHandlers } from './telemetry'; import { registerTmuxHandlers, removeTmuxHandlers } from './tmux'; import { initializeUpdaterHandlers, @@ -134,6 +132,8 @@ import type { CrossTeamService } from '../services/team/CrossTeamService'; import type { LaunchIoGovernor } from '../services/team/LaunchIoGovernor'; import type { TeamBackupService } from '../services/team/TeamBackupService'; +const logger = createLogger('IPC:handlers'); + /** * Initializes IPC handlers with service registry. */ diff --git a/src/main/services/team/TeamMcpConfigBuilder.ts b/src/main/services/team/TeamMcpConfigBuilder.ts index a57e93d4..c0562397 100644 --- a/src/main/services/team/TeamMcpConfigBuilder.ts +++ b/src/main/services/team/TeamMcpConfigBuilder.ts @@ -192,8 +192,8 @@ function looksLikeNodeBinaryPath(candidate: string | undefined): candidate is st function getNodeRuntimeCommandCandidates(): string[] { const candidates = [ process.env.NODE_BINARY, - process.env.npm_node_execpath, 'node', + process.env.npm_node_execpath, looksLikeNodeBinaryPath(process.execPath) ? process.execPath : undefined, ]; const seen = new Set(); diff --git a/src/main/services/team/opencode/delivery/OpenCodePromptDeliveryLedger.ts b/src/main/services/team/opencode/delivery/OpenCodePromptDeliveryLedger.ts index d0433988..02cdcf83 100644 --- a/src/main/services/team/opencode/delivery/OpenCodePromptDeliveryLedger.ts +++ b/src/main/services/team/opencode/delivery/OpenCodePromptDeliveryLedger.ts @@ -908,6 +908,7 @@ export function isOpenCodeSessionTransportChangedReason( } const OPENCODE_SESSION_REFRESH_FAILURE_PATTERN = + // eslint-disable-next-line sonarjs/regex-complexity -- Keyword taxonomy is kept literal to preserve diagnostic behavior. /(?:^|[_\s:;.\/()-])(?:permission[_\s-]?denied|permission[_\s-]?blocked|access[_\s-]?denied|auth[_\s-]?unavailable|authentication[_\s-]?failed|unauthorized|forbidden|401|403|login[_\s-]?required|not\s+logged\s+in|missing\s+credentials?|invalid\s+credentials?|credentials?[_\s-]?required|credentials?[_\s-]?unavailable|no auth available|authorization|auth(?:entication)?(?:[_\s-]?(?:failed|unavailable))?|invalid api[_\s-]?key|api[_\s-]?key|does not have access|quota|rate[_\s-]?(?:limit|limited)|too many requests|429|model cooldown|cooling down|enospc|no space left|disk is full|capacity exceeded|quota exhausted|usage exceeded|free usage exceeded|key limit exceeded|total limit|insufficient credits|subscribe to go|error|failed|failure|timeout|timed\s+out|network|connection|unable\s+to\s+connect|connect\s+failed|econn[a-z_]*|enotfound|fetch[_\s-]?failed|connection[_\s-]?(?:refused|reset)|aborted|cancel(?:ed|led)|interrupted|service[_\s-]?unavailable|temporarily\s+unavailable|overloaded|visible[_\s-]?reply(?:[_\s-][a-z0-9]+)*|task[_\s-]?refs|relayofmessageid|relay[_\s-]?of[_\s-]?message[_\s-]?id|message[_\s-]?send|non[_\s-]?visible[_\s-]?tool(?:[_\s-][a-z0-9]+)*|protocol[_\s-]?proof)(?=$|[_\s:;.\/(),-])/i; const OPENCODE_SESSION_REFRESH_ANY_REASON_PATTERN = /\b(?:resolved_behavior_changed|opencode_app_mcp_transport_changed):[-a-z0-9._~/=]+->[-a-z0-9._~/=]+/gi; diff --git a/src/main/services/team/runtime/RuntimeDiagnosticClassifier.ts b/src/main/services/team/runtime/RuntimeDiagnosticClassifier.ts index 012e26af..c3d5bb15 100644 --- a/src/main/services/team/runtime/RuntimeDiagnosticClassifier.ts +++ b/src/main/services/team/runtime/RuntimeDiagnosticClassifier.ts @@ -33,6 +33,7 @@ const OPENCODE_SESSION_REFRESH_MESSAGE = const OPENCODE_SESSION_REFRESH_REASON_PATTERN = /\b(?:resolved_behavior_changed|opencode_app_mcp_transport_changed):[-a-z0-9._~/=]+->[-a-z0-9._~/=]+/i; const OPENCODE_SESSION_REFRESH_FAILURE_PATTERN = + // eslint-disable-next-line sonarjs/regex-complexity -- Keyword taxonomy is kept literal to preserve diagnostic behavior. /(?:^|[_\s:;.\/()-])(?:permission[_\s-]?denied|permission[_\s-]?blocked|access[_\s-]?denied|auth[_\s-]?unavailable|authentication[_\s-]?failed|unauthorized|forbidden|401|403|login[_\s-]?required|not\s+logged\s+in|missing\s+credentials?|invalid\s+credentials?|credentials?[_\s-]?required|credentials?[_\s-]?unavailable|no auth available|authorization|auth(?:entication)?(?:[_\s-]?(?:failed|unavailable))?|invalid api[_\s-]?key|api[_\s-]?key|does not have access|quota|rate[_\s-]?(?:limit|limited)|too many requests|429|model cooldown|cooling down|enospc|no space left|disk is full|capacity exceeded|quota exhausted|usage exceeded|free usage exceeded|key limit exceeded|total limit|insufficient credits|subscribe to go|error|failed|failure|timeout|timed\s+out|network|connection|unable\s+to\s+connect|connect\s+failed|econn[a-z_]*|enotfound|fetch[_\s-]?failed|connection[_\s-]?(?:refused|reset)|aborted|cancel(?:ed|led)|interrupted|service[_\s-]?unavailable|temporarily\s+unavailable|overloaded|visible[_\s-]?reply(?:[_\s-][a-z0-9]+)*|task[_\s-]?refs|relayofmessageid|relay[_\s-]?of[_\s-]?message[_\s-]?id|message[_\s-]?send|non[_\s-]?visible[_\s-]?tool(?:[_\s-][a-z0-9]+)*|protocol[_\s-]?proof)(?=$|[_\s:;.\/(),-])/i; const OPENCODE_SESSION_REFRESH_ANY_REASON_PATTERN = /\b(?:resolved_behavior_changed|opencode_app_mcp_transport_changed):[-a-z0-9._~/=]+->[-a-z0-9._~/=]+/gi; diff --git a/src/main/utils/childProcess.ts b/src/main/utils/childProcess.ts index bd9a4523..87e31b05 100644 --- a/src/main/utils/childProcess.ts +++ b/src/main/utils/childProcess.ts @@ -5,8 +5,8 @@ import { type ExecFileOptions, type ExecOptions, spawn, - spawnSync, type SpawnOptions, + spawnSync, } from 'child_process'; import { existsSync, readFileSync } from 'fs'; import path from 'path'; @@ -30,19 +30,12 @@ function execFileAsync( let stdoutText = ''; let stderrText = ''; let timeoutHandle: ReturnType | null = null; - const cleanup = (): void => { - if (timeoutHandle) { - clearTimeout(timeoutHandle); - timeoutHandle = null; - } - untrackCliProcess(child); - }; child = execFile(cmd, args, execOptions, (err, stdout, stderr) => { if (settled) { return; } settled = true; - cleanup(); + timeoutHandle = cleanupTimedCliProcess(child, timeoutHandle); if (err) { const normalizedError = err instanceof Error ? err : new Error(typeof err === 'string' ? err : 'Unknown error'); @@ -67,7 +60,7 @@ function execFileAsync( return; } settled = true; - cleanup(); + timeoutHandle = cleanupTimedCliProcess(child, timeoutHandle); killProcessTree(child, timeoutSignal); const error = new Error( `Command timed out after ${timeoutMs}ms: ${cmd} ${args.join(' ')}` @@ -104,20 +97,13 @@ function execShellAsync( let stdoutText = ''; let stderrText = ''; let timeoutHandle: ReturnType | null = null; - const cleanup = (): void => { - if (timeoutHandle) { - clearTimeout(timeoutHandle); - timeoutHandle = null; - } - untrackCliProcess(child); - }; // eslint-disable-next-line sonarjs/os-command, security/detect-child-process -- cmd from known binaryPath+args, not user input (Windows EINVAL fallback) child = exec(cmd, execOptions, (err, stdout, stderr) => { if (settled) { return; } settled = true; - cleanup(); + timeoutHandle = cleanupTimedCliProcess(child, timeoutHandle); if (err) reject( err instanceof Error ? err : new Error(typeof err === 'string' ? err : 'Unknown error') @@ -138,7 +124,7 @@ function execShellAsync( return; } settled = true; - cleanup(); + timeoutHandle = cleanupTimedCliProcess(child, timeoutHandle); killProcessTree(child, timeoutSignal); const error = new Error(`Command timed out after ${timeoutMs}ms: ${cmd}`); Object.assign(error, { @@ -155,6 +141,17 @@ function execShellAsync( }); } +function cleanupTimedCliProcess( + child: ChildProcess | null, + timeoutHandle: ReturnType | null +): null { + if (timeoutHandle) { + clearTimeout(timeoutHandle); + } + untrackCliProcess(child); + return null; +} + /** * Returns true if the string contains any non-ASCII character. */ @@ -436,8 +433,8 @@ export function spawnCli( * `cmd.exe` shell, leaving the actual process (e.g. `claude.cmd`) orphaned. * `taskkill /T /F /PID` recursively kills the entire process tree. * - * On macOS/Linux, processes are killed directly (no shell wrapper), so - * the standard `child.kill(signal)` works correctly. + * On macOS/Linux, kill the child and descendants by PID so shell wrappers + * and spawned grandchildren do not survive a timeout or team stop. */ export function killProcessTree( child: ChildProcess | null | undefined, @@ -477,7 +474,7 @@ export function killProcessTree( } function normalizeKillSignal(signal: ExecFileOptions['killSignal']): NodeJS.Signals { - return typeof signal === 'string' ? (signal as NodeJS.Signals) : 'SIGTERM'; + return typeof signal === 'string' ? signal : 'SIGTERM'; } function getDescendantProcessIds(parentPid: number): number[] { @@ -496,7 +493,7 @@ function getDescendantProcessIds(parentPid: number): number[] { const childrenByParent = new Map(); for (const line of result.stdout.split('\n')) { - const match = line.trim().match(/^(\d+)\s+(\d+)$/); + const match = /^(\d+)\s+(\d+)$/.exec(line.trim()); if (!match) { continue; } diff --git a/src/renderer/components/team/TeamListView.tsx b/src/renderer/components/team/TeamListView.tsx index 26696898..f2e896e7 100644 --- a/src/renderer/components/team/TeamListView.tsx +++ b/src/renderer/components/team/TeamListView.tsx @@ -53,9 +53,9 @@ import { } from 'lucide-react'; import { useShallow } from 'zustand/react/shallow'; +import { executeTeamRelaunch } from './dialogs/teamRelaunchFlow'; import { TeamEmptyState } from './TeamEmptyState'; import { EMPTY_TEAM_FILTER, TeamListFilterPopover } from './TeamListFilterPopover'; -import { executeTeamRelaunch } from './dialogs/teamRelaunchFlow'; import { findTeamProjectSelectionTarget, resolveTeamProjectSelection, diff --git a/src/renderer/components/team/members/MemberCard.tsx b/src/renderer/components/team/members/MemberCard.tsx index f8c82df3..7c90050b 100644 --- a/src/renderer/components/team/members/MemberCard.tsx +++ b/src/renderer/components/team/members/MemberCard.tsx @@ -343,11 +343,11 @@ function buildRuntimeTelemetryTitle( return lines.join('\n'); } -function RuntimeTelemetryTooltipContent({ +const RuntimeTelemetryTooltipContent = ({ runtimeEntry, -}: { +}: Readonly<{ runtimeEntry: TeamAgentRuntimeEntry | undefined; -}): React.JSX.Element | null { +}>): React.JSX.Element | null => { if (!runtimeEntry) { return null; } @@ -445,7 +445,7 @@ function RuntimeTelemetryTooltipContent({ ); -} +}; function buildTelemetryPoints( samples: readonly TeamAgentRuntimeResourceSample[], diff --git a/src/renderer/utils/memberHelpers.ts b/src/renderer/utils/memberHelpers.ts index 377b6f00..85ea1d2a 100644 --- a/src/renderer/utils/memberHelpers.ts +++ b/src/renderer/utils/memberHelpers.ts @@ -393,6 +393,7 @@ function appendRuntimeAdvisoryRawMessage( const OPENCODE_SESSION_REFRESH_REASON_PATTERN = /\b(?:resolved_behavior_changed|opencode_app_mcp_transport_changed):[-a-z0-9._~/=]+->[-a-z0-9._~/=]+/i; const OPENCODE_SESSION_REFRESH_FAILURE_PATTERN = + // eslint-disable-next-line sonarjs/regex-complexity -- Keyword taxonomy is kept literal to preserve diagnostic behavior. /(?:^|[_\s:;.\/()-])(?:permission[_\s-]?denied|permission[_\s-]?blocked|access[_\s-]?denied|auth[_\s-]?unavailable|authentication[_\s-]?failed|unauthorized|forbidden|401|403|login[_\s-]?required|not\s+logged\s+in|missing\s+credentials?|invalid\s+credentials?|credentials?[_\s-]?required|credentials?[_\s-]?unavailable|no auth available|authorization|auth(?:entication)?(?:[_\s-]?(?:failed|unavailable))?|invalid api[_\s-]?key|api[_\s-]?key|does not have access|quota|rate[_\s-]?(?:limit|limited)|too many requests|429|model cooldown|cooling down|enospc|no space left|disk is full|capacity exceeded|quota exhausted|usage exceeded|free usage exceeded|key limit exceeded|total limit|insufficient credits|subscribe to go|error|failed|failure|timeout|timed\s+out|network|connection|unable\s+to\s+connect|connect\s+failed|econn[a-z_]*|enotfound|fetch[_\s-]?failed|connection[_\s-]?(?:refused|reset)|aborted|cancel(?:ed|led)|interrupted|service[_\s-]?unavailable|temporarily\s+unavailable|overloaded|visible[_\s-]?reply(?:[_\s-][a-z0-9]+)*|task[_\s-]?refs|relayofmessageid|relay[_\s-]?of[_\s-]?message[_\s-]?id|message[_\s-]?send|non[_\s-]?visible[_\s-]?tool(?:[_\s-][a-z0-9]+)*|protocol[_\s-]?proof)(?=$|[_\s:;.\/(),-])/i; const OPENCODE_SESSION_REFRESH_ANY_REASON_PATTERN = /\b(?:resolved_behavior_changed|opencode_app_mcp_transport_changed):[-a-z0-9._~/=]+->[-a-z0-9._~/=]+/gi; @@ -1035,6 +1036,13 @@ function shouldTreatCodexNativeRuntimeAsOffline({ if (!isCodexNativeProcessTeammate(member)) { return false; } + if ( + spawnLaunchState === 'starting' || + spawnLaunchState === 'runtime_pending_bootstrap' || + spawnLaunchState === 'runtime_pending_permission' + ) { + return false; + } if (hasLiveRuntimeProcessEvidence(runtimeEntry)) { return false; } diff --git a/src/renderer/utils/memberLaunchDiagnostics.ts b/src/renderer/utils/memberLaunchDiagnostics.ts index b82e6dca..2ea06e17 100644 --- a/src/renderer/utils/memberLaunchDiagnostics.ts +++ b/src/renderer/utils/memberLaunchDiagnostics.ts @@ -80,6 +80,7 @@ const SECRET_VALUE_PATTERN = const OPENCODE_SESSION_REFRESH_REASON_PATTERN = /\b(?:resolved_behavior_changed|opencode_app_mcp_transport_changed):[-a-z0-9._~/=]+->[-a-z0-9._~/=]+/i; const OPENCODE_SESSION_REFRESH_FAILURE_PATTERN = + // eslint-disable-next-line sonarjs/regex-complexity -- Keyword taxonomy is kept literal to preserve diagnostic behavior. /(?:^|[_\s:;.\/()-])(?:permission[_\s-]?denied|permission[_\s-]?blocked|access[_\s-]?denied|auth[_\s-]?unavailable|authentication[_\s-]?failed|unauthorized|forbidden|401|403|login[_\s-]?required|not\s+logged\s+in|missing\s+credentials?|invalid\s+credentials?|credentials?[_\s-]?required|credentials?[_\s-]?unavailable|no auth available|authorization|auth(?:entication)?(?:[_\s-]?(?:failed|unavailable))?|invalid api[_\s-]?key|api[_\s-]?key|does not have access|quota|rate[_\s-]?(?:limit|limited)|too many requests|429|model cooldown|cooling down|enospc|no space left|disk is full|capacity exceeded|quota exhausted|usage exceeded|free usage exceeded|key limit exceeded|total limit|insufficient credits|subscribe to go|error|failed|failure|timeout|timed\s+out|network|connection|unable\s+to\s+connect|connect\s+failed|econn[a-z_]*|enotfound|fetch[_\s-]?failed|connection[_\s-]?(?:refused|reset)|aborted|cancel(?:ed|led)|interrupted|service[_\s-]?unavailable|temporarily\s+unavailable|overloaded|visible[_\s-]?reply(?:[_\s-][a-z0-9]+)*|task[_\s-]?refs|relayofmessageid|relay[_\s-]?of[_\s-]?message[_\s-]?id|message[_\s-]?send|non[_\s-]?visible[_\s-]?tool(?:[_\s-][a-z0-9]+)*|protocol[_\s-]?proof)(?=$|[_\s:;.\/(),-])/i; const OPENCODE_SESSION_REFRESH_ANY_REASON_PATTERN = /\b(?:resolved_behavior_changed|opencode_app_mcp_transport_changed):[-a-z0-9._~/=]+->[-a-z0-9._~/=]+/gi; diff --git a/test/main/build/smokePackagedApp.test.ts b/test/main/build/smokePackagedApp.test.ts index 7bfb34a6..36d9b87c 100644 --- a/test/main/build/smokePackagedApp.test.ts +++ b/test/main/build/smokePackagedApp.test.ts @@ -59,9 +59,12 @@ describe('smokePackagedApp shutdown handling', () => { const termination = terminateChild(child, exitPromise, 'linux'); const rejection = expect(termination).rejects.toThrow('Timed out after 5000ms'); await vi.advanceTimersByTimeAsync(5_000); + await vi.advanceTimersByTimeAsync(5_000); await rejection; - expect(child.kill).toHaveBeenCalledTimes(1); + expect(child.kill).toHaveBeenCalledTimes(2); + expect(child.kill).toHaveBeenNthCalledWith(1); + expect(child.kill).toHaveBeenNthCalledWith(2, 'SIGKILL'); } finally { vi.useRealTimers(); } diff --git a/test/main/services/extensions/McpInstallService.test.ts b/test/main/services/extensions/McpInstallService.test.ts index f67716ad..a85bd55a 100644 --- a/test/main/services/extensions/McpInstallService.test.ts +++ b/test/main/services/extensions/McpInstallService.test.ts @@ -23,6 +23,14 @@ import { execCli } from '@main/utils/childProcess'; const mockExecCli = vi.mocked(execCli); +function findMcpCliArgs(command: 'add' | 'remove'): string[] { + const call = mockExecCli.mock.calls.find(([, args]) => args[0] === 'mcp' && args[1] === command); + if (!call) { + throw new Error(`Expected a claude mcp ${command} CLI call`); + } + return call[1]; +} + // ── Mock aggregator ────────────────────────────────────────────────────────── function makeStdioServer(): McpCatalogItem { @@ -60,7 +68,7 @@ function makeHttpServer(): McpCatalogItem { } function createMockAggregator( - getByIdResult: McpCatalogItem | null = makeStdioServer(), + getByIdResult: McpCatalogItem | null = makeStdioServer() ): McpCatalogAggregator { return { search: vi.fn(), @@ -101,8 +109,20 @@ describe('McpInstallService', () => { expect(result.state).toBe('success'); expect(mockExecCli).toHaveBeenCalledWith( '/usr/local/bin/claude', - ['mcp', 'add', '-s', 'user', '-e', 'UPSTASH_API_KEY=test-key-123', 'context7', '--', 'npx', '-y', '@upstash/context7-mcp@1.0.0'], - expect.objectContaining({ timeout: 30_000 }), + [ + 'mcp', + 'add', + '-s', + 'user', + '-e', + 'UPSTASH_API_KEY=test-key-123', + 'context7', + '--', + 'npx', + '-y', + '@upstash/context7-mcp@1.0.0', + ], + expect.objectContaining({ timeout: 30_000 }) ); }); @@ -118,7 +138,7 @@ describe('McpInstallService', () => { headers: [], }); - const args = mockExecCli.mock.calls[0]?.[1]; + const args = findMcpCliArgs('add'); expect(args).toContain('-s'); expect(args).toContain('project'); }); @@ -135,7 +155,7 @@ describe('McpInstallService', () => { headers: [], }); - const args = mockExecCli.mock.calls[0]?.[1]; + const args = findMcpCliArgs('add'); expect(args).not.toContain('-s'); }); }); @@ -159,8 +179,19 @@ describe('McpInstallService', () => { expect(result.state).toBe('success'); expect(mockExecCli).toHaveBeenCalledWith( '/usr/local/bin/claude', - ['mcp', 'add', '-s', 'user', '-t', 'sse', '-H', 'Authorization: Bearer token123', 'example-http', 'https://mcp.example.com/sse'], - expect.objectContaining({ timeout: 30_000 }), + [ + 'mcp', + 'add', + '-s', + 'user', + '-t', + 'sse', + '-H', + 'Authorization: Bearer token123', + 'example-http', + 'https://mcp.example.com/sse', + ], + expect.objectContaining({ timeout: 30_000 }) ); }); }); @@ -252,7 +283,7 @@ describe('McpInstallService', () => { describe('install (secret masking)', () => { it('masks env values in error messages', async () => { mockExecCli.mockRejectedValue( - new Error('Command failed: UPSTASH_API_KEY=super-secret-key-12345'), + new Error('Command failed: UPSTASH_API_KEY=super-secret-key-12345') ); const result = await service.install({ @@ -271,9 +302,7 @@ describe('McpInstallService', () => { it('masks header values in error messages', async () => { aggregator = createMockAggregator(makeHttpServer()); service = new McpInstallService(aggregator); - mockExecCli.mockRejectedValue( - new Error('Auth failed with Bearer my-token-value'), - ); + mockExecCli.mockRejectedValue(new Error('Auth failed with Bearer my-token-value')); const result = await service.install({ registryId: 'test', @@ -336,7 +365,7 @@ describe('McpInstallService', () => { expect(mockExecCli).toHaveBeenCalledWith( '/usr/local/bin/claude', ['mcp', 'remove', 'context7'], - expect.objectContaining({ timeout: 30_000 }), + expect.objectContaining({ timeout: 30_000 }) ); }); @@ -345,7 +374,7 @@ describe('McpInstallService', () => { await service.uninstall('context7', 'user'); - const args = mockExecCli.mock.calls[0]?.[1]; + const args = findMcpCliArgs('remove'); expect(args).toContain('-s'); expect(args).toContain('user'); }); diff --git a/test/main/services/team/TeamAgentLaunchMatrix.safe-e2e.test.ts b/test/main/services/team/TeamAgentLaunchMatrix.safe-e2e.test.ts index 6296158d..5df4b0d6 100644 --- a/test/main/services/team/TeamAgentLaunchMatrix.safe-e2e.test.ts +++ b/test/main/services/team/TeamAgentLaunchMatrix.safe-e2e.test.ts @@ -13554,22 +13554,29 @@ describe('Team agent launch matrix safe e2e', () => { await (svc as any).launchMixedSecondaryLaneIfNeeded(currentRun); await waitForCondition(() => adapter.pendingLaunchInputs.length === 1); - svc.stopTeam(teamName); + const killTracker = trackProcessKillsForPids([64901, 64902]); + try { + svc.stopTeam(teamName); - await waitForCondition(() => adapter.stopInputs.length === 1); - expectDirectChildKillCount(staleKillCount, 0); - expectDirectChildKillCount(currentKillCount, 1); - expect(staleRun.cancelRequested).toBe(false); - expect(currentRun.cancelRequested).toBe(true); - expect(adapter.stopInputs.map((input) => input.laneId).sort()).toEqual([ - 'secondary:opencode:bob', - ]); - expect(await svc.getRuntimeState(teamName)).toMatchObject({ - teamName, - isAlive: false, - runId: null, - progress: null, - }); + await waitForCondition(() => adapter.stopInputs.length === 1); + expectProcessKillCount(killTracker.killedPids, 64901, 0); + expectProcessKillCount(killTracker.killedPids, 64902, 1); + expectDirectChildKillCount(staleKillCount, 0); + expectDirectChildKillCount(currentKillCount, 0); + expect(staleRun.cancelRequested).toBe(false); + expect(currentRun.cancelRequested).toBe(true); + expect(adapter.stopInputs.map((input) => input.laneId).sort()).toEqual([ + 'secondary:opencode:bob', + ]); + expect(await svc.getRuntimeState(teamName)).toMatchObject({ + teamName, + isAlive: false, + runId: null, + progress: null, + }); + } finally { + killTracker.restore(); + } }); it('cancels a stale Anthropic and Gemini mixed run without stopping current OpenCode lanes', async () => { @@ -13611,43 +13618,50 @@ describe('Team agent launch matrix safe e2e', () => { await (svc as any).launchMixedSecondaryLaneIfNeeded(currentRun); await waitForCondition(() => adapter.pendingLaunchInputs.length === 1); - await svc.cancelProvisioning(staleRun.runId); + const killTracker = trackProcessKillsForPids([65001, 65002]); + try { + await svc.cancelProvisioning(staleRun.runId); - expectDirectChildKillCount(staleKillCount, 1); - expectDirectChildKillCount(currentKillCount, 0); - expect(staleRun.cancelRequested).toBe(true); - expect(currentRun.cancelRequested).toBe(false); - expect(adapter.stopInputs).toEqual([]); - expect(svc.isTeamAlive(teamName)).toBe(true); - expect(await svc.getRuntimeState(teamName)).toMatchObject({ - teamName, - isAlive: true, - runId: currentRun.runId, - }); + expectProcessKillCount(killTracker.killedPids, 65001, 1); + expectProcessKillCount(killTracker.killedPids, 65002, 0); + expectDirectChildKillCount(staleKillCount, 0); + expectDirectChildKillCount(currentKillCount, 0); + expect(staleRun.cancelRequested).toBe(true); + expect(currentRun.cancelRequested).toBe(false); + expect(adapter.stopInputs).toEqual([]); + expect(svc.isTeamAlive(teamName)).toBe(true); + expect(await svc.getRuntimeState(teamName)).toMatchObject({ + teamName, + isAlive: true, + runId: currentRun.runId, + }); - adapter.releaseLaunches(); - await waitForCondition(() => adapter.launchInputs.length === 2); - await waitForCondition(() => - currentRun.mixedSecondaryLanes.every((lane: { state: string }) => lane.state === 'finished') - ); + adapter.releaseLaunches(); + await waitForCondition(() => adapter.launchInputs.length === 2); + await waitForCondition(() => + currentRun.mixedSecondaryLanes.every((lane: { state: string }) => lane.state === 'finished') + ); - const statuses = await svc.getMemberSpawnStatuses(teamName); - expect(statuses.teamLaunchState).toBe('clean_success'); - expect(statuses.statuses.reviewer).toMatchObject({ - status: 'online', - launchState: 'confirmed_alive', - hardFailure: false, - }); - expect(statuses.statuses.bob).toMatchObject({ - status: 'online', - launchState: 'confirmed_alive', - hardFailure: false, - }); - expect(statuses.statuses.tom).toMatchObject({ - status: 'online', - launchState: 'confirmed_alive', - hardFailure: false, - }); + const statuses = await svc.getMemberSpawnStatuses(teamName); + expect(statuses.teamLaunchState).toBe('clean_success'); + expect(statuses.statuses.reviewer).toMatchObject({ + status: 'online', + launchState: 'confirmed_alive', + hardFailure: false, + }); + expect(statuses.statuses.bob).toMatchObject({ + status: 'online', + launchState: 'confirmed_alive', + hardFailure: false, + }); + expect(statuses.statuses.tom).toMatchObject({ + status: 'online', + launchState: 'confirmed_alive', + hardFailure: false, + }); + } finally { + killTracker.restore(); + } }); it('stops the current pure Anthropic run instead of a stale same-team run', async () => { @@ -13677,18 +13691,25 @@ describe('Team agent launch matrix safe e2e', () => { runId: currentRun.runId, }); - svc.stopTeam(teamName); + const killTracker = trackProcessKillsForPids([63101, 63102]); + try { + svc.stopTeam(teamName); - expectDirectChildKillCount(staleKillCount, 0); - expectDirectChildKillCount(currentKillCount, 1); - expect(staleRun.cancelRequested).toBe(false); - expect(currentRun.cancelRequested).toBe(true); - expect(await svc.getRuntimeState(teamName)).toMatchObject({ - teamName, - isAlive: false, - runId: null, - progress: null, - }); + expectProcessKillCount(killTracker.killedPids, 63101, 0); + expectProcessKillCount(killTracker.killedPids, 63102, 1); + expectDirectChildKillCount(staleKillCount, 0); + expectDirectChildKillCount(currentKillCount, 0); + expect(staleRun.cancelRequested).toBe(false); + expect(currentRun.cancelRequested).toBe(true); + expect(await svc.getRuntimeState(teamName)).toMatchObject({ + teamName, + isAlive: false, + runId: null, + progress: null, + }); + } finally { + killTracker.restore(); + } }); it('cancels a stale pure Anthropic run without stopping the current same-team run', async () => { @@ -13712,20 +13733,27 @@ describe('Team agent launch matrix safe e2e', () => { trackLiveRun(svc, staleRun); trackLiveRun(svc, currentRun); - await svc.cancelProvisioning(staleRun.runId); + const killTracker = trackProcessKillsForPids([63301, 63302]); + try { + await svc.cancelProvisioning(staleRun.runId); - expectDirectChildKillCount(staleKillCount, 1); - expectDirectChildKillCount(currentKillCount, 0); - expect(staleRun.cancelRequested).toBe(true); - expect(currentRun.cancelRequested).toBe(false); - expect(svc.isTeamAlive(teamName)).toBe(true); - expect(await svc.getRuntimeState(teamName)).toMatchObject({ - teamName, - isAlive: true, - runId: currentRun.runId, - }); + expectProcessKillCount(killTracker.killedPids, 63301, 1); + expectProcessKillCount(killTracker.killedPids, 63302, 0); + expectDirectChildKillCount(staleKillCount, 0); + expectDirectChildKillCount(currentKillCount, 0); + expect(staleRun.cancelRequested).toBe(true); + expect(currentRun.cancelRequested).toBe(false); + expect(svc.isTeamAlive(teamName)).toBe(true); + expect(await svc.getRuntimeState(teamName)).toMatchObject({ + teamName, + isAlive: true, + runId: currentRun.runId, + }); - await svc.sendMessageToTeam(teamName, 'current run still receives messages'); + await svc.sendMessageToTeam(teamName, 'current run still receives messages'); + } finally { + killTracker.restore(); + } }); it('cancels the current pure Anthropic run without resurrecting a stale same-team run', async () => { @@ -13749,22 +13777,29 @@ describe('Team agent launch matrix safe e2e', () => { trackLiveRun(svc, staleRun); trackLiveRun(svc, currentRun); - await svc.cancelProvisioning(currentRun.runId); + const killTracker = trackProcessKillsForPids([63501, 63502]); + try { + await svc.cancelProvisioning(currentRun.runId); - expectDirectChildKillCount(staleKillCount, 0); - expectDirectChildKillCount(currentKillCount, 1); - expect(staleRun.cancelRequested).toBe(false); - expect(currentRun.cancelRequested).toBe(true); - expect(svc.isTeamAlive(teamName)).toBe(false); - expect(await svc.getRuntimeState(teamName)).toMatchObject({ - teamName, - isAlive: false, - runId: null, - progress: null, - }); - await expect(svc.sendMessageToTeam(teamName, 'must not hit stale run')).rejects.toThrow( - `No active process for team "${teamName}"` - ); + expectProcessKillCount(killTracker.killedPids, 63501, 0); + expectProcessKillCount(killTracker.killedPids, 63502, 1); + expectDirectChildKillCount(staleKillCount, 0); + expectDirectChildKillCount(currentKillCount, 0); + expect(staleRun.cancelRequested).toBe(false); + expect(currentRun.cancelRequested).toBe(true); + expect(svc.isTeamAlive(teamName)).toBe(false); + expect(await svc.getRuntimeState(teamName)).toMatchObject({ + teamName, + isAlive: false, + runId: null, + progress: null, + }); + await expect(svc.sendMessageToTeam(teamName, 'must not hit stale run')).rejects.toThrow( + `No active process for team "${teamName}"` + ); + } finally { + killTracker.restore(); + } }); it('refreshes runtime snapshot cache after same-team pure Anthropic relaunch', async () => { @@ -19067,7 +19102,38 @@ async function writeStoppedProcessRegistry(teamName: string): Promise { } function expectDirectChildKillCount(actual: number, expected: number): void { - // Windows uses taskkill.exe for process-tree termination, so fake child.kill is not called. + expect(actual).toBe(expected); +} + +function trackProcessKillsForPids(pids: readonly number[]): { + killedPids: number[]; + restore: () => void; +} { + const targetPids = new Set(pids); + const killedPids: number[] = []; + const spy = vi.spyOn(process, 'kill').mockImplementation((( + pid: number | string, + signal?: number | string + ) => { + const numericPid = Number(pid); + if (targetPids.has(numericPid) && signal !== 0) { + killedPids.push(numericPid); + } + return true; + }) as typeof process.kill); + return { + killedPids, + restore: () => spy.mockRestore(), + }; +} + +function expectProcessKillCount( + killedPids: readonly number[], + pid: number, + expected: number +): void { + const actual = killedPids.filter((killedPid) => killedPid === pid).length; + // Windows uses taskkill.exe for process-tree termination, so process.kill is not called. expect(actual).toBe(process.platform === 'win32' ? 0 : expected); } diff --git a/test/main/services/team/TeamProvisioningServicePrompts.test.ts b/test/main/services/team/TeamProvisioningServicePrompts.test.ts index e9543d0f..bc8fef8e 100644 --- a/test/main/services/team/TeamProvisioningServicePrompts.test.ts +++ b/test/main/services/team/TeamProvisioningServicePrompts.test.ts @@ -25,6 +25,9 @@ vi.mock('@main/services/team/ClaudeBinaryResolver', () => ({ vi.mock('@main/utils/childProcess', () => ({ execCli: vi.fn(async (_binaryPath: string | null, args: string[]) => { + if (args[0] === '-e' && args[1]?.includes('process.execPath')) { + return { stdout: process.execPath, stderr: '' }; + } if (args.includes('model') && args.includes('list')) { return { stdout: JSON.stringify({ @@ -79,6 +82,19 @@ vi.mock('@main/utils/childProcess', () => ({ killProcessTree: vi.fn(), })); +vi.mock('@main/utils/shellEnv', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getCachedShellEnv: () => ({ PATH: process.env.PATH ?? '', HOME: hoisted.paths.claudeRoot }), + getShellPreferredHome: () => hoisted.paths.claudeRoot || actual.getShellPreferredHome(), + resolveInteractiveShellEnv: vi.fn(async () => ({ + PATH: process.env.PATH ?? '', + HOME: hoisted.paths.claudeRoot, + })), + }; +}); + vi.mock('@main/utils/pathDecoder', async (importOriginal) => { const actual = await importOriginal(); return { @@ -265,18 +281,20 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () => expect(writeSpy).not.toHaveBeenCalled(); const prompt = extractPromptFromBootstrapFile(); expect(prompt).toContain('SOLO MODE: This team CURRENTLY has ZERO teammates.'); - expect(prompt).toContain('This launch/bootstrap step has already been completed deterministically by the runtime.'); + expect(prompt).toContain( + 'This launch/bootstrap step has already been completed deterministically by the runtime.' + ); expect(prompt).toContain('Do NOT start implementation in this turn.'); - expect(prompt).toContain('Use this turn only to refresh context, review the current board snapshot, and confirm you are ready.'); + expect(prompt).toContain( + 'Use this turn only to refresh context, review the current board snapshot, and confirm you are ready.' + ); expect(prompt).toContain( 'Do NOT create, assign, or delegate any new task in this turn. If the board is empty, stay silent and wait for a fresh user instruction.' ); expect(prompt).toContain( 'review_request already notifies the reviewer, so do NOT send a second manual SendMessage for the same review request' ); - expect(prompt).toContain( - 'Review is a state transition on the EXISTING work task.' - ); + expect(prompt).toContain('Review is a state transition on the EXISTING work task.'); expect(prompt).toContain( 'The REVIEW column is for the same task #X moving through review. It is NOT a signal to create another task for review.' ); @@ -520,7 +538,9 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () => expect(message).toContain('Teammate "alice" with role "Reviewer" was restarted from the UI.'); expect(message).toContain('team_name="forge-labs", name="alice"'); expect(message).toContain('provider="codex", model="gpt-5.4-mini", effort="medium"'); - expect(message).toContain('This is a restart of an existing persistent teammate, not a new teammate.'); + expect(message).toContain( + 'This is a restart of an existing persistent teammate, not a new teammate.' + ); expect(message).toContain( 'If the Agent tool returns duplicate_skipped with reason bootstrap_pending, treat that as a pending restart and wait for teammate check-in.' ); @@ -623,11 +643,11 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () => role: 'developer', }); - expect(prompt).toContain('Review flow rule: review is a state transition on the SAME work task'); - expect(prompt).toContain('Do NOT create a separate "review task"'); expect(prompt).toContain( - 'If no reviewer exists, leave #X completed.' + 'Review flow rule: review is a state transition on the SAME work task' ); + expect(prompt).toContain('Do NOT create a separate "review task"'); + expect(prompt).toContain('If no reviewer exists, leave #X completed.'); expect(prompt).toContain( 'If you are the reviewer for task #X, call review_start on #X first, then review_approve or review_request_changes on #X itself.' ); @@ -731,9 +751,7 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () => expect(writeSpy).not.toHaveBeenCalled(); const prompt = extractPromptFromBootstrapFile(); - expect(prompt).toContain( - 'Teammate task comments are auto-forwarded to you.' - ); + expect(prompt).toContain('Teammate task comments are auto-forwarded to you.'); await svc.cancelProvisioning(runId); }); @@ -790,25 +808,29 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () => expect(writeSpy).not.toHaveBeenCalled(); const prompt = extractPromptFromBootstrapFile(); - expect(prompt).toContain('This launch/bootstrap step has already been completed deterministically by the runtime.'); + expect(prompt).toContain( + 'This launch/bootstrap step has already been completed deterministically by the runtime.' + ); expect(prompt).toContain('Do NOT use Agent to spawn or restore teammates.'); - expect(prompt).toContain('Use this turn only to refresh context and review the current board snapshot.'); + expect(prompt).toContain( + 'Use this turn only to refresh context and review the current board snapshot.' + ); expect(prompt).toContain( 'Do NOT create, assign, or delegate any new task in this turn. If the board is empty, stay silent and wait for a fresh user instruction.' ); expect(prompt).toContain('DELEGATION-FIRST (behavior rule for ALL future turns):'); expect(prompt).toContain(`AGENT_BLOCK_OPEN is exactly: ${AGENT_BLOCK_OPEN}`); expect(prompt).toContain(`AGENT_BLOCK_CLOSE is exactly: ${AGENT_BLOCK_CLOSE}`); - expect(prompt).toContain('Messages to "user" (the human) must NEVER contain agent-only blocks.'); + expect(prompt).toContain( + 'Messages to "user" (the human) must NEVER contain agent-only blocks.' + ); expect(prompt).toContain('task_create_from_message'); expect(prompt).toContain('task_set_owner'); expect(prompt).toContain('cross_team_send'); expect(prompt).toContain( 'lead_briefing is the primary lead queue. Decisions about what to act on now come from lead_briefing, not from raw task_list rows.' ); - expect(prompt).toContain( - 'Browse/search compact inventory rows only: task_list' - ); + expect(prompt).toContain('Browse/search compact inventory rows only: task_list'); expect(prompt).toContain( `Browse/search compact inventory rows only: task_list { teamName: "${teamName}", owner?: "", status?: "pending|in_progress|completed"` ); @@ -816,20 +838,12 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () => `Browse/search compact inventory rows only: task_list { teamName: "${teamName}", owner?: "", status?: "pending|in_progress|completed|deleted"` ); expect(prompt).toContain( - 'task_list is inventory/search/drill-down only. Do NOT treat task_list as the lead\'s working queue.' - ); - expect(prompt).toContain( - 'review_request already notifies the reviewer' - ); - expect(prompt).toContain( - 'By default, NEVER create a separate "review task".' - ); - expect(prompt).toContain( - 'Only move #X into REVIEW when a real reviewer exists for #X.' - ); - expect(prompt).not.toContain( - 'Only create a separate review reminder/assignment task' + "task_list is inventory/search/drill-down only. Do NOT treat task_list as the lead's working queue." ); + expect(prompt).toContain('review_request already notifies the reviewer'); + expect(prompt).toContain('By default, NEVER create a separate "review task".'); + expect(prompt).toContain('Only move #X into REVIEW when a real reviewer exists for #X.'); + expect(prompt).not.toContain('Only create a separate review reminder/assignment task'); expect(prompt).toContain( 'Correct flow: finish implementation on #X -> task_complete #X -> review_request #X -> reviewer runs review_start #X -> reviewer runs review_approve or review_request_changes on #X.' ); diff --git a/test/renderer/features/agent-graph/GraphMemberLogPreviewHud.test.tsx b/test/renderer/features/agent-graph/GraphMemberLogPreviewHud.test.tsx index 6ee9fce8..e8bc37bf 100644 --- a/test/renderer/features/agent-graph/GraphMemberLogPreviewHud.test.tsx +++ b/test/renderer/features/agent-graph/GraphMemberLogPreviewHud.test.tsx @@ -166,10 +166,11 @@ describe('GraphMemberLogPreviewHud', () => { button.textContent?.includes('pnpm test') ); expect(row).not.toBeUndefined(); - expect(row?.querySelector('.float-left')).not.toBeNull(); + expect(row?.querySelector('svg.text-amber-300')).not.toBeNull(); expect(row?.querySelector('.line-clamp-3')).toBeNull(); + expect(row?.querySelector('.line-clamp-2')).not.toBeNull(); expect(row?.className).toContain('h-[72px]'); - expect(row?.querySelector('span.text-slate-200')?.className).toContain('leading-5'); + expect(row?.querySelector('span.text-slate-200')?.className).toContain('leading-4'); expect(row?.textContent).toContain('pnpm test'); const errorRow = Array.from(host.querySelectorAll('button')).find((button) =>