diff --git a/apps/desktop/src/main/packagedRuntimeSmoke.ts b/apps/desktop/src/main/packagedRuntimeSmoke.ts index 8f1753ca..e1ee20ec 100644 --- a/apps/desktop/src/main/packagedRuntimeSmoke.ts +++ b/apps/desktop/src/main/packagedRuntimeSmoke.ts @@ -10,31 +10,37 @@ const execFileAsync = promisify(execFile); const PTY_PROBE_TIMEOUT_MS = 4_000; const CLAUDE_PROBE_TIMEOUT_MS = 20_000; +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +const AUTH_FAILURE_PATTERNS = [ + "not authenticated", + "not logged in", + "authentication required", + "authentication error", + "authentication_error", + "login required", + "sign in", + "claude auth login", + "/login", + "authentication_failed", + "invalid authentication credentials", + "invalid api key", + "api error: 401", + "status code: 401", + "status 401", +]; + function isClaudeAuthFailureMessage(input: unknown): boolean { const text = input instanceof Error ? input.message : String(input ?? ""); const lower = text.toLowerCase(); - return ( - lower.includes("not authenticated") - || lower.includes("not logged in") - || lower.includes("authentication required") - || lower.includes("authentication error") - || lower.includes("authentication_error") - || lower.includes("login required") - || lower.includes("sign in") - || lower.includes("claude auth login") - || lower.includes("/login") - || lower.includes("authentication_failed") - || lower.includes("invalid authentication credentials") - || lower.includes("invalid api key") - || lower.includes("api error: 401") - || lower.includes("status code: 401") - || lower.includes("status 401") - ); + return AUTH_FAILURE_PATTERNS.some((pattern) => lower.includes(pattern)); } async function probePty(): Promise<{ ok: true; output: string }> { const pty = await import("node-pty"); - return await new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { let output = ""; const term = pty.spawn("/bin/sh", ["-lc", 'printf "ADE_PTY_OK\\n"'], { name: "xterm-256color", @@ -124,12 +130,12 @@ async function probeClaudeStartup( if (isClaudeAuthFailureMessage(error)) { return { state: "auth-failed", - message: error instanceof Error ? error.message : String(error), + message: errorMessage(error), }; } return { state: "runtime-failed", - message: error instanceof Error ? error.message : String(error), + message: errorMessage(error), }; } finally { clearTimeout(timeout); @@ -193,6 +199,6 @@ async function main(): Promise { } void main().catch((error) => { - process.stderr.write(error instanceof Error ? error.stack ?? error.message : String(error)); + process.stderr.write(error instanceof Error ? (error.stack ?? error.message) : String(error)); process.exit(1); }); diff --git a/apps/desktop/src/main/services/ai/claudeRuntimeProbe.test.ts b/apps/desktop/src/main/services/ai/claudeRuntimeProbe.test.ts index fe110b82..75161117 100644 --- a/apps/desktop/src/main/services/ai/claudeRuntimeProbe.test.ts +++ b/apps/desktop/src/main/services/ai/claudeRuntimeProbe.test.ts @@ -14,11 +14,18 @@ const mockState = vi.hoisted(() => ({ env: { ADE_PROJECT_ROOT: "/tmp/project" }, }, })), - resolveAdeMcpServerLaunch: vi.fn(() => ({ + resolveDesktopAdeMcpLaunch: vi.fn(() => ({ + mode: "headless_source", command: "node", cmdArgs: ["probe.js"], env: { ADE_PROJECT_ROOT: "/tmp/project" }, + entryPath: "probe.js", + runtimeRoot: "/tmp/runtime", + socketPath: "/tmp/project/.ade/mcp.sock", + packaged: false, + resourcesPath: null, })), + resolveRepoRuntimeRoot: vi.fn(() => "/tmp/runtime"), })); vi.mock("@anthropic-ai/claude-agent-sdk", () => ({ @@ -39,8 +46,9 @@ vi.mock("./providerResolver", () => ({ normalizeCliMcpServers: mockState.normalizeCliMcpServers, })); -vi.mock("../orchestrator/unifiedOrchestratorAdapter", () => ({ - resolveAdeMcpServerLaunch: mockState.resolveAdeMcpServerLaunch, +vi.mock("../runtime/adeMcpLaunch", () => ({ + resolveDesktopAdeMcpLaunch: mockState.resolveDesktopAdeMcpLaunch, + resolveRepoRuntimeRoot: mockState.resolveRepoRuntimeRoot, })); let probeClaudeRuntimeHealth: typeof import("./claudeRuntimeProbe").probeClaudeRuntimeHealth; @@ -68,7 +76,8 @@ beforeEach(async () => { mockState.reportProviderRuntimeFailure.mockReset(); mockState.resolveClaudeCodeExecutable.mockClear(); mockState.normalizeCliMcpServers.mockClear(); - mockState.resolveAdeMcpServerLaunch.mockClear(); + mockState.resolveDesktopAdeMcpLaunch.mockClear(); + mockState.resolveRepoRuntimeRoot.mockClear(); const mod = await import("./claudeRuntimeProbe"); probeClaudeRuntimeHealth = mod.probeClaudeRuntimeHealth; resetClaudeRuntimeProbeCache = mod.resetClaudeRuntimeProbeCache; @@ -153,4 +162,52 @@ describe("claudeRuntimeProbe", () => { expect(mockState.reportProviderRuntimeAuthFailure).toHaveBeenCalledTimes(1); expect(mockState.reportProviderRuntimeFailure).not.toHaveBeenCalled(); }); + + it("calls resolveDesktopAdeMcpLaunch with defaultRole external and projectRoot", async () => { + const query = makeStream([ + { + type: "result", + subtype: "success", + duration_ms: 50, + duration_api_ms: 50, + is_error: false, + num_turns: 1, + result: "ok", + session_id: "session-ok", + total_cost_usd: 0.001, + usage: { + input_tokens: 10, + cache_creation_input_tokens: 0, + cache_read_input_tokens: 0, + output_tokens: 5, + server_tool_use: { web_search_requests: 0 }, + service_tier: "standard", + }, + }, + ]); + mockState.query.mockReturnValue(query.stream); + + await probeClaudeRuntimeHealth({ projectRoot: "/my/custom/project", force: true }); + + expect(mockState.resolveDesktopAdeMcpLaunch).toHaveBeenCalledWith( + expect.objectContaining({ + projectRoot: "/my/custom/project", + workspaceRoot: "/my/custom/project", + defaultRole: "external", + }), + ); + expect(mockState.resolveRepoRuntimeRoot).toHaveBeenCalled(); + expect(mockState.reportProviderRuntimeReady).toHaveBeenCalledTimes(1); + }); + + it("reports runtime-failed when the probe stream throws an error", async () => { + mockState.query.mockImplementation(() => { + throw new Error("spawn ENOENT"); + }); + + await probeClaudeRuntimeHealth({ projectRoot: "/tmp/project", force: true }); + + expect(mockState.reportProviderRuntimeFailure).toHaveBeenCalledTimes(1); + expect(mockState.reportProviderRuntimeAuthFailure).not.toHaveBeenCalled(); + }); }); diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 96e50e9c..7d96c8fa 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -1083,8 +1083,7 @@ function isLightweightSession(session: Pick) let _mcpRuntimeRootCache: string | null = null; function resolveMcpRuntimeRoot(): string { - if (_mcpRuntimeRootCache !== null) return _mcpRuntimeRootCache; - _mcpRuntimeRootCache = resolveUnifiedRuntimeRoot(); + _mcpRuntimeRootCache ??= resolveUnifiedRuntimeRoot(); return _mcpRuntimeRootCache; } @@ -1342,22 +1341,25 @@ export function createAgentChatService(args: { ownerId?: string | null; computerUsePolicy?: ComputerUsePolicy | null; }) => { - const launch = resolveAdeMcpServerLaunch({ + const { mode, command, entryPath, runtimeRoot, socketPath, packaged, resourcesPath } = resolveAdeMcpServerLaunch({ workspaceRoot: projectRoot, runtimeRoot: resolveMcpRuntimeRoot(), defaultRole: args.defaultRole, ownerId: args.ownerId ?? undefined, computerUsePolicy: normalizeComputerUsePolicy(args.computerUsePolicy, createDefaultComputerUsePolicy()), }); - return { - mode: launch.mode, - command: launch.command, - entryPath: launch.entryPath, - runtimeRoot: launch.runtimeRoot, - socketPath: launch.socketPath, - packaged: launch.packaged, - resourcesPath: launch.resourcesPath, - }; + return { mode, command, entryPath, runtimeRoot, socketPath, packaged, resourcesPath }; + }; + + /** Best-effort diagnostic: resolve the MCP launch config for a session, returning undefined on failure. */ + const tryDiagnosticMcpLaunch = (managed: ManagedChatSession): ReturnType | undefined => { + try { + return summarizeAdeMcpLaunch({ + defaultRole: managed.session.identityKey === "cto" ? "cto" : "agent", + ownerId: resolveWorkerIdentityAgentId(managed.session.identityKey), + computerUsePolicy: managed.session.computerUse, + }); + } catch { return undefined; } }; const readTranscriptConversationEntries = (managed: ManagedChatSession): string[] => { @@ -4692,14 +4694,7 @@ export function createAgentChatService(args: { }; const startCodexRuntime = async (managed: ManagedChatSession): Promise => { - let adeMcpLaunch: ReturnType | undefined; - try { - adeMcpLaunch = summarizeAdeMcpLaunch({ - defaultRole: managed.session.identityKey === "cto" ? "cto" : "agent", - ownerId: resolveWorkerIdentityAgentId(managed.session.identityKey), - computerUsePolicy: managed.session.computerUse, - }); - } catch { /* best-effort diagnostic — must not block Codex startup */ } + const adeMcpLaunch = tryDiagnosticMcpLaunch(managed); logger.info("agent_chat.codex_runtime_start", { sessionId: managed.session.id, @@ -5191,17 +5186,10 @@ export function createAgentChatService(args: { ); } let diagClaudePath: string | undefined; - let diagMcpLaunch: ReturnType | undefined; try { diagClaudePath = runtime.v2Session ? undefined : buildClaudeV2SessionOpts(managed, runtime).pathToClaudeCodeExecutable; } catch { /* best-effort diagnostic */ } - try { - diagMcpLaunch = summarizeAdeMcpLaunch({ - defaultRole: managed.session.identityKey === "cto" ? "cto" : "agent", - ownerId: resolveWorkerIdentityAgentId(managed.session.identityKey), - computerUsePolicy: managed.session.computerUse, - }); - } catch { /* best-effort diagnostic */ } + const diagMcpLaunch = tryDiagnosticMcpLaunch(managed); logger.warn("agent_chat.claude_v2_prewarm_failed", { sessionId: managed.session.id, error: error instanceof Error ? error.message : String(error), diff --git a/apps/desktop/src/main/services/conflicts/conflictService.test.ts b/apps/desktop/src/main/services/conflicts/conflictService.test.ts index 8a606550..91c36d4a 100644 --- a/apps/desktop/src/main/services/conflicts/conflictService.test.ts +++ b/apps/desktop/src/main/services/conflicts/conflictService.test.ts @@ -952,4 +952,319 @@ describe("conflictService conflict context integrity", () => { expect(rebased.success).toBe(true); expect(git(repoRoot, ["rev-list", "--count", "HEAD..main"])).toBe("0"); }); + + it("prefers the current parent lane branch when baseRef is stale", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-conflicts-parent-branch-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + const projectId = randomUUID(); + const now = "2026-03-24T12:00:00.000Z"; + + try { + db.run( + "insert into projects(id, root_path, display_name, default_base_ref, created_at, last_opened_at) values (?, ?, ?, ?, ?, ?)", + [projectId, repoRoot, "demo", "main", now, now] + ); + + fs.writeFileSync(path.join(repoRoot, "file.txt"), "base\n", "utf8"); + git(repoRoot, ["init", "-b", "main"]); + git(repoRoot, ["config", "user.email", "ade@test.local"]); + git(repoRoot, ["config", "user.name", "ADE Test"]); + git(repoRoot, ["add", "."]); + git(repoRoot, ["commit", "-m", "base"]); + + git(repoRoot, ["checkout", "-b", "feature/parent-current"]); + git(repoRoot, ["checkout", "-b", "feature/child"]); + fs.writeFileSync(path.join(repoRoot, "file.txt"), "child\n", "utf8"); + git(repoRoot, ["add", "file.txt"]); + git(repoRoot, ["commit", "-m", "child work"]); + + git(repoRoot, ["checkout", "main"]); + fs.writeFileSync(path.join(repoRoot, "main.txt"), "main advance\n", "utf8"); + git(repoRoot, ["add", "main.txt"]); + git(repoRoot, ["commit", "-m", "main advance"]); + git(repoRoot, ["checkout", "feature/child"]); + + const parentLane = { + ...createLaneSummary(repoRoot, { + id: "lane-parent", + name: "Primary", + branchRef: "feature/parent-current", + baseRef: "main", + parentLaneId: null + }), + laneType: "primary" as const, + }; + const childLane = createLaneSummary(repoRoot, { + id: "lane-child", + name: "Child", + branchRef: "feature/child", + baseRef: "main", + parentLaneId: "lane-parent" + }); + + const service = createConflictService({ + db, + logger: createLogger(), + projectId, + projectRoot: repoRoot, + laneService: { + list: async () => [parentLane, childLane], + getLaneBaseAndBranch: () => ({ worktreePath: repoRoot, baseRef: "main", branchRef: "feature/child" }) + } as any, + projectConfigService: { + get: () => ({ effective: { providerMode: "guest" }, local: {} }) + } as any, + }); + + expect(await service.scanRebaseNeeds()).toEqual([]); + expect(await service.getRebaseNeed("lane-child")).toBeNull(); + } finally { + db.close(); + fs.rmSync(repoRoot, { recursive: true, force: true }); + } + }); + + it("uses non-primary parent branchRef directly (no origin/ prefix) as rebase target", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-conflicts-worktree-parent-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + const projectId = randomUUID(); + const now = "2026-03-24T12:00:00.000Z"; + + try { + db.run( + "insert into projects(id, root_path, display_name, default_base_ref, created_at, last_opened_at) values (?, ?, ?, ?, ?, ?)", + [projectId, repoRoot, "demo", "main", now, now] + ); + + fs.writeFileSync(path.join(repoRoot, "file.txt"), "base\n", "utf8"); + git(repoRoot, ["init", "-b", "main"]); + git(repoRoot, ["config", "user.email", "ade@test.local"]); + git(repoRoot, ["config", "user.name", "ADE Test"]); + git(repoRoot, ["add", "."]); + git(repoRoot, ["commit", "-m", "base"]); + + // Create a worktree-type parent lane + git(repoRoot, ["checkout", "-b", "feature/worktree-parent"]); + fs.writeFileSync(path.join(repoRoot, "parent.txt"), "parent work\n", "utf8"); + git(repoRoot, ["add", "parent.txt"]); + git(repoRoot, ["commit", "-m", "parent work"]); + + // Create a child branch off the parent + git(repoRoot, ["checkout", "-b", "feature/grandchild"]); + fs.writeFileSync(path.join(repoRoot, "child.txt"), "grandchild\n", "utf8"); + git(repoRoot, ["add", "child.txt"]); + git(repoRoot, ["commit", "-m", "grandchild work"]); + + // Advance the parent so the child is behind + git(repoRoot, ["checkout", "feature/worktree-parent"]); + fs.writeFileSync(path.join(repoRoot, "parent2.txt"), "parent advance\n", "utf8"); + git(repoRoot, ["add", "parent2.txt"]); + git(repoRoot, ["commit", "-m", "parent advance"]); + git(repoRoot, ["checkout", "feature/grandchild"]); + + // Parent is NOT primary — it's a regular worktree lane + const parentLane = createLaneSummary(repoRoot, { + id: "lane-wt-parent", + name: "Worktree Parent", + branchRef: "feature/worktree-parent", + baseRef: "main", + parentLaneId: null + }); + const childLane = createLaneSummary(repoRoot, { + id: "lane-grandchild", + name: "Grandchild", + branchRef: "feature/grandchild", + baseRef: "main", + parentLaneId: "lane-wt-parent" + }); + + const service = createConflictService({ + db, + logger: createLogger(), + projectId, + projectRoot: repoRoot, + laneService: { + list: async () => [parentLane, childLane], + getLaneBaseAndBranch: () => ({ worktreePath: repoRoot, baseRef: "main", branchRef: "feature/grandchild" }) + } as any, + projectConfigService: { + get: () => ({ effective: { providerMode: "guest" }, local: {} }) + } as any, + }); + + // The child should see the parent's new commits via the local branchRef + // (not origin/feature/worktree-parent, since the parent is not primary) + const needs = await service.scanRebaseNeeds(); + expect(needs).toHaveLength(1); + expect(needs[0]).toMatchObject({ + laneId: "lane-grandchild", + baseBranch: "feature/worktree-parent", + }); + expect(needs[0]!.behindBy).toBeGreaterThan(0); + + // Also verify getRebaseNeed returns the same result + const single = await service.getRebaseNeed("lane-grandchild"); + expect(single).toBeTruthy(); + expect(single!.baseBranch).toBe("feature/worktree-parent"); + } finally { + db.close(); + fs.rmSync(repoRoot, { recursive: true, force: true }); + } + }); + + it("falls back to local branchRef when origin/ remote ref is unavailable for primary parent", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-conflicts-primary-fallback-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + const projectId = randomUUID(); + const now = "2026-03-24T12:00:00.000Z"; + + try { + db.run( + "insert into projects(id, root_path, display_name, default_base_ref, created_at, last_opened_at) values (?, ?, ?, ?, ?, ?)", + [projectId, repoRoot, "demo", "main", now, now] + ); + + fs.writeFileSync(path.join(repoRoot, "file.txt"), "base\n", "utf8"); + git(repoRoot, ["init", "-b", "main"]); + git(repoRoot, ["config", "user.email", "ade@test.local"]); + git(repoRoot, ["config", "user.name", "ADE Test"]); + git(repoRoot, ["add", "."]); + git(repoRoot, ["commit", "-m", "base"]); + + // Primary parent with local branch but no origin remote + git(repoRoot, ["checkout", "-b", "feature/primary-local"]); + fs.writeFileSync(path.join(repoRoot, "parent.txt"), "parent\n", "utf8"); + git(repoRoot, ["add", "parent.txt"]); + git(repoRoot, ["commit", "-m", "parent advance"]); + + git(repoRoot, ["checkout", "-b", "feature/child-of-primary"]); + git(repoRoot, ["checkout", "feature/primary-local"]); + fs.writeFileSync(path.join(repoRoot, "parent2.txt"), "more parent\n", "utf8"); + git(repoRoot, ["add", "parent2.txt"]); + git(repoRoot, ["commit", "-m", "parent advance again"]); + git(repoRoot, ["checkout", "feature/child-of-primary"]); + + const parentLane = { + ...createLaneSummary(repoRoot, { + id: "lane-primary", + name: "Primary", + branchRef: "feature/primary-local", + baseRef: "main", + parentLaneId: null, + }), + laneType: "primary" as const, + }; + const childLane = createLaneSummary(repoRoot, { + id: "lane-child-primary", + name: "Child", + branchRef: "feature/child-of-primary", + baseRef: "main", + parentLaneId: "lane-primary", + }); + + const service = createConflictService({ + db, + logger: createLogger(), + projectId, + projectRoot: repoRoot, + laneService: { + list: async () => [parentLane, childLane], + getLaneBaseAndBranch: () => ({ worktreePath: repoRoot, baseRef: "main", branchRef: "feature/child-of-primary" }), + } as any, + projectConfigService: { + get: () => ({ effective: { providerMode: "guest" }, local: {} }), + } as any, + }); + + // origin/feature/primary-local doesn't exist, but the fallback to local + // feature/primary-local should still detect the child is behind. + const needs = await service.scanRebaseNeeds(); + expect(needs).toHaveLength(1); + expect(needs[0]).toMatchObject({ + laneId: "lane-child-primary", + baseBranch: "feature/primary-local", + }); + expect(needs[0]!.behindBy).toBeGreaterThan(0); + } finally { + db.close(); + fs.rmSync(repoRoot, { recursive: true, force: true }); + } + }); + + it("falls back to lane.baseRef when parent lane has no branchRef", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-conflicts-no-parent-branch-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + const projectId = randomUUID(); + const now = "2026-03-24T12:00:00.000Z"; + + try { + db.run( + "insert into projects(id, root_path, display_name, default_base_ref, created_at, last_opened_at) values (?, ?, ?, ?, ?, ?)", + [projectId, repoRoot, "demo", "main", now, now] + ); + + fs.writeFileSync(path.join(repoRoot, "file.txt"), "base\n", "utf8"); + git(repoRoot, ["init", "-b", "main"]); + git(repoRoot, ["config", "user.email", "ade@test.local"]); + git(repoRoot, ["config", "user.name", "ADE Test"]); + git(repoRoot, ["add", "."]); + git(repoRoot, ["commit", "-m", "base"]); + + git(repoRoot, ["checkout", "-b", "feature/orphan-child"]); + fs.writeFileSync(path.join(repoRoot, "orphan.txt"), "orphan\n", "utf8"); + git(repoRoot, ["add", "orphan.txt"]); + git(repoRoot, ["commit", "-m", "orphan work"]); + + git(repoRoot, ["checkout", "main"]); + fs.writeFileSync(path.join(repoRoot, "main2.txt"), "main advance\n", "utf8"); + git(repoRoot, ["add", "main2.txt"]); + git(repoRoot, ["commit", "-m", "main advance"]); + git(repoRoot, ["checkout", "feature/orphan-child"]); + + // Parent lane exists but has an empty branchRef — make it primary so it's + // skipped by scanRebaseNeeds and only the child is evaluated. + const parentLane = { + ...createLaneSummary(repoRoot, { + id: "lane-empty-parent", + name: "Empty Parent", + branchRef: "", + baseRef: "main", + parentLaneId: null + }), + laneType: "primary" as const, + }; + const childLane = createLaneSummary(repoRoot, { + id: "lane-orphan", + name: "Orphan", + branchRef: "feature/orphan-child", + baseRef: "main", + parentLaneId: "lane-empty-parent" + }); + + const service = createConflictService({ + db, + logger: createLogger(), + projectId, + projectRoot: repoRoot, + laneService: { + list: async () => [parentLane, childLane], + getLaneBaseAndBranch: () => ({ worktreePath: repoRoot, baseRef: "main", branchRef: "feature/orphan-child" }) + } as any, + projectConfigService: { + get: () => ({ effective: { providerMode: "guest" }, local: {} }) + } as any, + }); + + // Should fall back to baseRef ("main") since parent branchRef is empty + const needs = await service.scanRebaseNeeds(); + expect(needs).toHaveLength(1); + expect(needs[0]).toMatchObject({ + laneId: "lane-orphan", + baseBranch: "main", + }); + } finally { + db.close(); + fs.rmSync(repoRoot, { recursive: true, force: true }); + } + }); }); diff --git a/apps/desktop/src/main/services/conflicts/conflictService.ts b/apps/desktop/src/main/services/conflicts/conflictService.ts index ed78208b..5996a936 100644 --- a/apps/desktop/src/main/services/conflicts/conflictService.ts +++ b/apps/desktop/src/main/services/conflicts/conflictService.ts @@ -75,6 +75,7 @@ import { redactSecretsDeep } from "../../utils/redaction"; import { extractFirstJsonObject } from "../ai/utils"; import { safeSegment } from "../shared/packLegacyUtils"; import { fetchQueueTargetTrackingBranches, resolveQueueRebaseOverride } from "../shared/queueRebase"; +import type { QueueRebaseOverride } from "../shared/queueRebase"; import { asString, isRecord, parseDiffNameOnly, safeJsonParse, uniqueSorted } from "../shared/utils"; type PredictionStatus = "clean" | "conflict" | "unknown"; @@ -269,6 +270,43 @@ async function readTouchedFiles(cwd: string, mergeBase: string, headSha: string) return new Set(parseDiffNameOnly(res.stdout)); } +function resolveLaneRebaseTarget(args: { + lane: LaneSummary; + lanesById: Map; + queueOverride: QueueRebaseOverride | null; +}): { + comparisonRef: string; + fallbackRef?: string; + displayBaseBranch: string; +} { + if (args.queueOverride) { + return { + comparisonRef: args.queueOverride.comparisonRef, + displayBaseBranch: args.queueOverride.displayBaseBranch, + }; + } + + const parent = args.lane.parentLaneId ? args.lanesById.get(args.lane.parentLaneId) ?? null : null; + const parentBranchRef = parent?.branchRef?.trim() ?? ""; + if (parentBranchRef) { + // For primary lanes, prefer the remote tracking ref (origin/) to stay + // consistent with laneService.resolveParentRebaseTarget which rebases against + // the remote tracking ref rather than the local HEAD. Fall back to the local + // branch ref when the remote ref is unavailable (e.g. before first fetch). + const comparisonRef = parent?.laneType === "primary" ? `origin/${parentBranchRef}` : parentBranchRef; + return { + comparisonRef, + fallbackRef: parentBranchRef, + displayBaseBranch: parentBranchRef, + }; + } + + return { + comparisonRef: args.lane.baseRef, + displayBaseBranch: args.lane.baseRef, + }; +} + async function readDiffNumstat(cwd: string, mergeBase: string, headSha: string): Promise<{ files: Set; insertions: number; @@ -4173,6 +4211,7 @@ export function createConflictService({ } const lanes = await listActiveLanes(); + const lanesById = new Map(lanes.map((lane) => [lane.id, lane] as const)); const needs: RebaseNeed[] = []; // Skip primary lane — it IS the base, rebasing it is nonsensical @@ -4185,9 +4224,15 @@ export function createConflictService({ projectRoot, laneId: lane.id, }); - const comparisonRef = queueOverride?.comparisonRef ?? lane.baseRef; - const displayBaseBranch = queueOverride?.displayBaseBranch ?? lane.baseRef; - const baseHead = await readHeadSha(projectRoot, comparisonRef); + const { comparisonRef, fallbackRef, displayBaseBranch } = resolveLaneRebaseTarget({ + lane, + lanesById, + queueOverride, + }); + const baseHead = await readHeadSha(projectRoot, comparisonRef) + .catch(() => fallbackRef ? readHeadSha(projectRoot, fallbackRef) : Promise.reject()) + .catch(() => ""); + if (!baseHead) continue; const laneHead = await readHeadSha(lane.worktreePath, "HEAD"); // Count how many commits the lane is behind base @@ -4244,6 +4289,7 @@ export function createConflictService({ }); const lanes = await listActiveLanes(); + const lanesById = new Map(lanes.map((entry) => [entry.id, entry] as const)); const lane = lanes.find((l) => l.id === laneId); if (!lane || lane.laneType === "primary") return null; @@ -4254,9 +4300,15 @@ export function createConflictService({ projectRoot, laneId: lane.id, }); - const comparisonRef = queueOverride?.comparisonRef ?? lane.baseRef; - const displayBaseBranch = queueOverride?.displayBaseBranch ?? lane.baseRef; - const baseHead = await readHeadSha(projectRoot, comparisonRef); + const { comparisonRef, fallbackRef, displayBaseBranch } = resolveLaneRebaseTarget({ + lane, + lanesById, + queueOverride, + }); + const baseHead = await readHeadSha(projectRoot, comparisonRef) + .catch(() => fallbackRef ? readHeadSha(projectRoot, fallbackRef) : Promise.reject()) + .catch(() => ""); + if (!baseHead) return null; const laneHead = await readHeadSha(lane.worktreePath, "HEAD"); const behindRes = await runGit( @@ -4340,6 +4392,7 @@ export function createConflictService({ try { const lanes = await listActiveLanes(); + const lanesById = new Map(lanes.map((entry) => [entry.id, entry] as const)); const lane = lanes.find((l) => l.id === args.laneId); if (!lane) { return { @@ -4384,7 +4437,11 @@ export function createConflictService({ projectRoot, laneId: lane.id, }); - const rebaseTarget = queueOverride?.comparisonRef ?? lane.baseRef; + const { comparisonRef: rebaseTarget } = resolveLaneRebaseTarget({ + lane, + lanesById, + queueOverride, + }); const rebaseRes = await runGit( ["rebase", rebaseTarget], { cwd: lane.worktreePath, timeoutMs: 120_000 } diff --git a/apps/desktop/src/main/services/lanes/laneService.test.ts b/apps/desktop/src/main/services/lanes/laneService.test.ts index 860c403e..24ae0854 100644 --- a/apps/desktop/src/main/services/lanes/laneService.test.ts +++ b/apps/desktop/src/main/services/lanes/laneService.test.ts @@ -11,7 +11,7 @@ vi.mock("../git/git", () => ({ runGitOrThrow: vi.fn(), })); -import { getHeadSha, runGit } from "../git/git"; +import { getHeadSha, runGit, runGitOrThrow } from "../git/git"; function createLogger() { return { @@ -61,6 +61,7 @@ describe("laneService rebaseStart", () => { beforeEach(() => { vi.mocked(getHeadSha).mockReset(); vi.mocked(runGit).mockReset(); + vi.mocked(runGitOrThrow).mockReset(); }); it("skips rebasing when the parent head is already an ancestor of the lane head", async () => { @@ -118,14 +119,14 @@ describe("laneService rebaseStart", () => { if (args[0] === "merge-base" && args[1] === "--is-ancestor") { return Promise.resolve({ exitCode: 1, stdout: "", stderr: "" }); } + if (args[0] === "status" && args[1] === "--porcelain=v1") { + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + } if (args[0] === "rebase") { return new Promise((resolve) => { resolveRebase = resolve; }); } - if (args[0] === "status" && args[1] === "--porcelain=v1") { - return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); - } throw new Error(`Unexpected git call: ${args.join(" ")}`); }); @@ -155,4 +156,412 @@ describe("laneService rebaseStart", () => { const completed = await firstRun; expect(completed.run.state).toBe("completed"); }); + + it("rebases against the primary lane remote tracking ref when it is available", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-primary-remote-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + await seedProjectAndStack(db, { projectId: "proj-primary-remote", repoRoot }); + + vi.mocked(runGitOrThrow).mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" } as any); + vi.mocked(getHeadSha).mockImplementation(async (cwd: string) => { + if (cwd.endsWith("/parent")) return "sha-parent"; + return "sha-main"; + }); + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + if ( + args[0] === "rev-parse" + && args[1] === "--abbrev-ref" + && args[2] === "--symbolic-full-name" + && args[3] === "@{upstream}" + ) { + return { exitCode: 0, stdout: "origin/main\n", stderr: "" }; + } + if (args[0] === "rev-parse" && args[1] === "--verify" && args[2] === "origin/main") { + return { exitCode: 0, stdout: "sha-origin-main\n", stderr: "" }; + } + if (args[0] === "merge-base" && args[1] === "--is-ancestor") { + expect(args[2]).toBe("sha-origin-main"); + expect(args[3]).toBe("sha-parent"); + return { exitCode: 1, stdout: "", stderr: "" }; + } + if (args[0] === "status" && args[1] === "--porcelain=v1") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if (args[0] === "rebase") { + expect(args[1]).toBe("sha-origin-main"); + return { exitCode: 0, stdout: "", stderr: "" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-primary-remote", + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + }); + + const result = await service.rebaseStart({ laneId: "lane-parent", scope: "lane_only", actor: "user" }); + + expect(result.run.state).toBe("completed"); + expect(result.run.error).toBeNull(); + expect(result.run.lanes[0]?.status).toBe("succeeded"); + expect(vi.mocked(runGitOrThrow)).toHaveBeenCalled(); + }); + + it("falls back to origin/ when upstream is not configured for primary parent", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-origin-fallback-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + await seedProjectAndStack(db, { projectId: "proj-origin-fallback", repoRoot }); + const logs: string[] = []; + + vi.mocked(runGitOrThrow).mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" } as any); + vi.mocked(getHeadSha).mockImplementation(async (cwd: string) => { + if (cwd.endsWith("/parent")) return "sha-parent"; + return "sha-main"; + }); + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + // upstream detection fails (no upstream configured) + if ( + args[0] === "rev-parse" + && args[1] === "--abbrev-ref" + && args[2] === "--symbolic-full-name" + && args[3] === "@{upstream}" + ) { + return { exitCode: 1, stdout: "", stderr: "fatal: no upstream configured" }; + } + // origin/main exists and resolves + if (args[0] === "rev-parse" && args[1] === "--verify" && args[2] === "origin/main") { + return { exitCode: 0, stdout: "sha-origin-main\n", stderr: "" }; + } + if (args[0] === "merge-base" && args[1] === "--is-ancestor") { + expect(args[2]).toBe("sha-origin-main"); + return { exitCode: 1, stdout: "", stderr: "" }; + } + if (args[0] === "status" && args[1] === "--porcelain=v1") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if (args[0] === "rebase") { + expect(args[1]).toBe("sha-origin-main"); + return { exitCode: 0, stdout: "", stderr: "" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-origin-fallback", + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + onRebaseEvent: (event) => { + if (event.type === "rebase-run-log") logs.push(event.message); + }, + }); + + const result = await service.rebaseStart({ laneId: "lane-parent", scope: "lane_only", actor: "user" }); + + expect(result.run.state).toBe("completed"); + expect(result.run.lanes[0]?.status).toBe("succeeded"); + // The log should show the parentTargetLabel format "Main (origin/main)" + const rebaseLog = logs.find((line) => line.includes("Rebasing")); + expect(rebaseLog, "expected a 'Rebasing' log entry").toBeTruthy(); + expect(rebaseLog).toContain("Main (origin/main)"); + }); + + it("falls back to parent HEAD when both upstream and origin ref are unavailable for primary parent", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-all-remote-fail-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + await seedProjectAndStack(db, { projectId: "proj-all-remote-fail", repoRoot }); + const logs: string[] = []; + + vi.mocked(runGitOrThrow).mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" } as any); + vi.mocked(getHeadSha).mockImplementation(async (cwd: string) => { + if (cwd.endsWith("/parent")) return "sha-parent-local"; + if (cwd.endsWith("/main")) return "sha-main-local"; + return "sha-main"; + }); + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + // upstream detection fails + if ( + args[0] === "rev-parse" + && args[1] === "--abbrev-ref" + && args[2] === "--symbolic-full-name" + && args[3] === "@{upstream}" + ) { + return { exitCode: 1, stdout: "", stderr: "" }; + } + // origin/main also fails to resolve + if (args[0] === "rev-parse" && args[1] === "--verify") { + return { exitCode: 1, stdout: "", stderr: "fatal: not a valid ref" }; + } + if (args[0] === "merge-base" && args[1] === "--is-ancestor") { + // parent local HEAD is used instead + expect(args[2]).toBe("sha-main-local"); + return { exitCode: 1, stdout: "", stderr: "" }; + } + if (args[0] === "status" && args[1] === "--porcelain=v1") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if (args[0] === "rebase") { + expect(args[1]).toBe("sha-main-local"); + return { exitCode: 0, stdout: "", stderr: "" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-all-remote-fail", + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + onRebaseEvent: (event) => { + if (event.type === "rebase-run-log") logs.push(event.message); + }, + }); + + const result = await service.rebaseStart({ laneId: "lane-parent", scope: "lane_only", actor: "user" }); + + expect(result.run.state).toBe("completed"); + expect(result.run.lanes[0]?.status).toBe("succeeded"); + // When label === parent.name, describeParentRebaseTarget returns just the name + const rebaseLog = logs.find((line) => line.includes("Rebasing")); + expect(rebaseLog, "expected a 'Rebasing' log entry").toBeTruthy(); + expect(rebaseLog).toContain("onto Main (sha-main"); + }); + + it("uses parent HEAD directly for non-primary (worktree) parent without remote resolution", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-worktree-parent-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + await seedProjectAndStack(db, { projectId: "proj-worktree-parent", repoRoot }); + const logs: string[] = []; + + // lane-child has parent lane-parent (which is lane_type=worktree, not primary) + vi.mocked(runGitOrThrow).mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" } as any); + vi.mocked(getHeadSha).mockImplementation(async (cwd: string) => { + if (cwd.endsWith("/parent")) return "sha-parent-head"; + if (cwd.endsWith("/child")) return "sha-child-head"; + return "sha-main"; + }); + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + // For a worktree parent, resolveParentRebaseTarget should NOT call + // rev-parse for upstream or origin refs. It goes straight to getHeadSha. + if (args[0] === "rev-parse" && args[1] === "--abbrev-ref") { + throw new Error("Should not resolve upstream for non-primary parent"); + } + if (args[0] === "merge-base" && args[1] === "--is-ancestor") { + expect(args[2]).toBe("sha-parent-head"); + expect(args[3]).toBe("sha-child-head"); + return { exitCode: 1, stdout: "", stderr: "" }; + } + if (args[0] === "status" && args[1] === "--porcelain=v1") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if (args[0] === "rebase") { + expect(args[1]).toBe("sha-parent-head"); + return { exitCode: 0, stdout: "", stderr: "" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-worktree-parent", + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + onRebaseEvent: (event) => { + if (event.type === "rebase-run-log") logs.push(event.message); + }, + }); + + const result = await service.rebaseStart({ laneId: "lane-child", scope: "lane_only", actor: "user" }); + + expect(result.run.state).toBe("completed"); + expect(result.run.lanes[0]?.status).toBe("succeeded"); + // For worktree parent, the label is the parent name itself, so no parenthesized ref + const rebaseLog = logs.find((line) => line.includes("Rebasing")); + expect(rebaseLog, "expected a 'Rebasing' log entry").toBeTruthy(); + // parentHead.slice(0, 8) truncates the sha, so check substring + expect(rebaseLog).toContain("onto Parent (sha-pare"); + expect(rebaseLog).not.toContain("origin/"); + }); + + it("fails the rebase run when resolveParentRebaseTarget throws (parent HEAD unresolvable)", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-unresolvable-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + await seedProjectAndStack(db, { projectId: "proj-unresolvable", repoRoot }); + + vi.mocked(runGitOrThrow).mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" } as any); + // getHeadSha returns null for the primary parent, simulating an unresolvable HEAD + vi.mocked(getHeadSha).mockImplementation(async (cwd: string) => { + if (cwd.endsWith("/parent")) return "sha-parent"; + if (cwd.endsWith("/main")) return null; + return null; + }); + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + // All remote resolution attempts fail + if (args[0] === "rev-parse") { + return { exitCode: 1, stdout: "", stderr: "fatal: not found" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-unresolvable", + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + }); + + const result = await service.rebaseStart({ laneId: "lane-parent", scope: "lane_only", actor: "user" }); + + expect(result.run.state).toBe("failed"); + expect(result.run.error).toContain("Unable to resolve parent HEAD for Main"); + expect(result.run.lanes[0]?.status).toBe("blocked"); + }); + + it("includes parentTargetLabel in skip log when already up to date with a remote ref", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-skip-label-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + await seedProjectAndStack(db, { projectId: "proj-skip-label", repoRoot }); + const logs: string[] = []; + + vi.mocked(runGitOrThrow).mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" } as any); + vi.mocked(getHeadSha).mockImplementation(async (cwd: string) => { + if (cwd.endsWith("/parent")) return "sha-parent"; + return "sha-main"; + }); + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + if ( + args[0] === "rev-parse" + && args[1] === "--abbrev-ref" + && args[2] === "--symbolic-full-name" + && args[3] === "@{upstream}" + ) { + return { exitCode: 0, stdout: "origin/main\n", stderr: "" }; + } + if (args[0] === "rev-parse" && args[1] === "--verify" && args[2] === "origin/main") { + return { exitCode: 0, stdout: "sha-origin-main\n", stderr: "" }; + } + if (args[0] === "merge-base" && args[1] === "--is-ancestor") { + // Already an ancestor => skip + return { exitCode: 0, stdout: "", stderr: "" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-skip-label", + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + onRebaseEvent: (event) => { + if (event.type === "rebase-run-log") logs.push(event.message); + }, + }); + + const result = await service.rebaseStart({ laneId: "lane-parent", scope: "lane_only", actor: "user" }); + + expect(result.run.state).toBe("completed"); + expect(result.run.lanes[0]?.status).toBe("skipped"); + const skipLog = logs.find((line) => line.includes("already up to date")); + expect(skipLog, "expected an 'already up to date' log entry").toBeTruthy(); + expect(skipLog).toContain("Main (origin/main)"); + }); + + it("fails the rebase run when the worktree has uncommitted changes", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-dirty-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + await seedProjectAndStack(db, { projectId: "proj-dirty", repoRoot }); + const logs: string[] = []; + + vi.mocked(getHeadSha).mockImplementation(async (cwd: string) => { + if (cwd.endsWith("/parent")) return "sha-parent"; + if (cwd.endsWith("/child")) return "sha-child"; + return "sha-main"; + }); + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + if (args[0] === "merge-base" && args[1] === "--is-ancestor") { + return { exitCode: 1, stdout: "", stderr: "" }; + } + if (args[0] === "status" && args[1] === "--porcelain=v1") { + // Worktree is dirty + return { exitCode: 0, stdout: " M src/file.ts\n", stderr: "" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-dirty", + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + onRebaseEvent: (event) => { + if (event.type === "rebase-run-log") logs.push(event.message); + }, + }); + + const result = await service.rebaseStart({ laneId: "lane-child", scope: "lane_only", actor: "user" }); + + expect(result.run.state).toBe("failed"); + expect(result.run.error).toContain("uncommitted changes"); + expect(result.run.lanes[0]?.status).toBe("blocked"); + const dirtyLog = logs.find((line) => line.includes("dirty")); + expect(dirtyLog, "expected a dirty worktree log entry").toBeTruthy(); + }); + + it("uses deduplicated candidate refs when upstream equals origin/", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-dedup-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + await seedProjectAndStack(db, { projectId: "proj-dedup", repoRoot }); + + vi.mocked(runGitOrThrow).mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" } as any); + vi.mocked(getHeadSha).mockImplementation(async (cwd: string) => { + if (cwd.endsWith("/parent")) return "sha-parent"; + return "sha-main"; + }); + + const revParseVerifyCalls: string[] = []; + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + if ( + args[0] === "rev-parse" + && args[1] === "--abbrev-ref" + && args[2] === "--symbolic-full-name" + && args[3] === "@{upstream}" + ) { + // upstream IS origin/main, matching the fallback origin/ + return { exitCode: 0, stdout: "origin/main\n", stderr: "" }; + } + if (args[0] === "rev-parse" && args[1] === "--verify") { + revParseVerifyCalls.push(args[2] ?? ""); + return { exitCode: 0, stdout: "sha-origin-main\n", stderr: "" }; + } + if (args[0] === "merge-base" && args[1] === "--is-ancestor") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-dedup", + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + }); + + const result = await service.rebaseStart({ laneId: "lane-parent", scope: "lane_only", actor: "user" }); + + expect(result.run.state).toBe("completed"); + // When upstream is already origin/main, it should NOT add origin/main twice + // to candidateRefs. So only one rev-parse --verify call should happen. + expect(revParseVerifyCalls).toHaveLength(1); + expect(revParseVerifyCalls[0]).toBe("origin/main"); + }); }); diff --git a/apps/desktop/src/main/services/lanes/laneService.ts b/apps/desktop/src/main/services/lanes/laneService.ts index 4c5c7d68..8d16dc29 100644 --- a/apps/desktop/src/main/services/lanes/laneService.ts +++ b/apps/desktop/src/main/services/lanes/laneService.ts @@ -4,7 +4,7 @@ import { randomUUID } from "node:crypto"; import type { AdeDb } from "../state/kvDb"; import { getHeadSha, runGit, runGitOrThrow } from "../git/git"; import { isWithinDir } from "../shared/utils"; -import { resolveQueueRebaseOverride, type QueueRebaseOverride } from "../shared/queueRebase"; +import { fetchRemoteTrackingBranch, resolveQueueRebaseOverride, type QueueRebaseOverride } from "../shared/queueRebase"; import { detectConflictKind } from "../git/gitConflictState"; import type { createOperationService } from "../history/operationService"; import type { @@ -226,6 +226,58 @@ async function computeLaneStatus(worktreePath: string, baseRef: string, branchRe return { dirty, ahead, behind, remoteBehind, rebaseInProgress }; } +async function resolveParentRebaseTarget(args: { + projectRoot: string; + parent: LaneRow; +}): Promise<{ headSha: string; label: string }> { + const { projectRoot, parent } = args; + + if (parent.lane_type === "primary") { + await fetchRemoteTrackingBranch({ + projectRoot, + targetBranch: parent.branch_ref, + }).catch(() => {}); + + const candidateRefs: string[] = []; + const upstreamRes = await runGit( + ["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"], + { cwd: parent.worktree_path, timeoutMs: 5_000 }, + ); + const upstreamRef = upstreamRes.exitCode === 0 ? upstreamRes.stdout.trim() : ""; + if (upstreamRef) { + candidateRefs.push(upstreamRef); + } + const originRef = `origin/${parent.branch_ref}`; + if (!candidateRefs.includes(originRef)) { + candidateRefs.push(originRef); + } + + for (const ref of candidateRefs) { + const res = await runGit( + ["rev-parse", "--verify", ref], + { cwd: parent.worktree_path, timeoutMs: 5_000 }, + ); + const sha = res.exitCode === 0 ? res.stdout.trim() : ""; + if (sha) { + return { headSha: sha, label: ref }; + } + } + } + + const headSha = await getHeadSha(parent.worktree_path); + if (!headSha) { + throw new Error(`Unable to resolve parent HEAD for ${parent.name}`); + } + return { + headSha, + label: parent.name, + }; +} + +function describeParentRebaseTarget(parent: LaneRow, label: string): string { + return label === parent.name ? parent.name : `${parent.name} (${label})`; +} + function computeStackDepth(args: { laneId: string; rowsById: Map; @@ -1274,11 +1326,20 @@ export function createLaneService({ break; } - const parentHead = await getHeadSha(parent.worktree_path); - if (!parentHead) { - failRunAtLane(laneItem, lane.id, index, `Unable to resolve parent HEAD for ${parent.name}`); + let parentTarget: { headSha: string; label: string }; + try { + parentTarget = await resolveParentRebaseTarget({ projectRoot, parent }); + } catch (error) { + failRunAtLane( + laneItem, + lane.id, + index, + error instanceof Error ? error.message : `Unable to resolve parent HEAD for ${parent.name}`, + ); break; } + const parentHead = parentTarget.headSha; + const parentTargetLabel = describeParentRebaseTarget(parent, parentTarget.label); run.currentLaneId = lane.id; laneItem.preHeadSha = await getHeadSha(lane.worktree_path); @@ -1298,7 +1359,7 @@ export function createLaneService({ emitRunLog({ runId, laneId: lane.id, - message: `${lane.name} is already up to date with ${parent.name}; skipping rebase.`, + message: `${lane.name} is already up to date with ${parentTargetLabel}; skipping rebase.`, }); emitRunUpdated(run); continue; @@ -1323,7 +1384,7 @@ export function createLaneService({ emitRunLog({ runId, laneId: lane.id, - message: `Rebasing ${lane.name} onto ${parent.name} (${parentHead.slice(0, 8)})` + message: `Rebasing ${lane.name} onto ${parentTargetLabel} (${parentHead.slice(0, 8)})` }); const operation = operationService?.start({ diff --git a/apps/desktop/src/main/services/orchestrator/unifiedOrchestratorAdapter.test.ts b/apps/desktop/src/main/services/orchestrator/unifiedOrchestratorAdapter.test.ts index a469e6f9..f7d2f4ae 100644 --- a/apps/desktop/src/main/services/orchestrator/unifiedOrchestratorAdapter.test.ts +++ b/apps/desktop/src/main/services/orchestrator/unifiedOrchestratorAdapter.test.ts @@ -7,6 +7,7 @@ import { buildCodexMcpConfigFlags, createUnifiedOrchestratorAdapter, resolveAdeMcpServerLaunch, + resolveUnifiedRuntimeRoot, } from "./unifiedOrchestratorAdapter"; describe("buildCodexMcpConfigFlags", () => { @@ -88,6 +89,72 @@ describe("resolveAdeMcpServerLaunch", () => { ADE_DEFAULT_ROLE: "external", }); }); + + it("returns expanded result fields including mode, entryPath, socketPath, and runtimeRoot", () => { + const runtimeRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-mcp-expanded-")); + const projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-mcp-expanded-proj-")); + const workspaceRoot = path.join(projectRoot, "workspace"); + const builtEntry = path.join(runtimeRoot, "apps", "mcp-server", "dist", "index.cjs"); + + fs.mkdirSync(path.dirname(builtEntry), { recursive: true }); + fs.mkdirSync(workspaceRoot, { recursive: true }); + fs.writeFileSync(builtEntry, "module.exports = {};\n", "utf8"); + + const launch = resolveAdeMcpServerLaunch({ + projectRoot, + workspaceRoot, + runtimeRoot, + preferBundledProxy: false, + }); + + expect(launch.mode).toBe("headless_built"); + expect(launch.entryPath).toBe(builtEntry); + expect(launch.runtimeRoot).toBe(path.resolve(runtimeRoot)); + expect(launch.socketPath).toBe(path.join(path.resolve(projectRoot), ".ade", "mcp.sock")); + expect(typeof launch.packaged).toBe("boolean"); + // resourcesPath is null in test env (no Electron app.asar) + expect(launch.resourcesPath === null || typeof launch.resourcesPath === "string").toBe(true); + }); + + it("falls back to headless source mode when no built entry exists", () => { + const runtimeRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-mcp-source-fallback-")); + const projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-mcp-source-proj-")); + const workspaceRoot = path.join(projectRoot, "workspace"); + + fs.mkdirSync(workspaceRoot, { recursive: true }); + // Only create the mcp-server directory, NOT the dist/index.cjs file + fs.mkdirSync(path.join(runtimeRoot, "apps", "mcp-server", "src"), { recursive: true }); + + const launch = resolveAdeMcpServerLaunch({ + projectRoot, + workspaceRoot, + runtimeRoot, + preferBundledProxy: false, + }); + + expect(launch.mode).toBe("headless_source"); + expect(launch.command).toBe("npx"); + expect(launch.entryPath).toBe( + path.join(runtimeRoot, "apps", "mcp-server", "src", "index.ts"), + ); + expect(launch.cmdArgs[0]).toBe("tsx"); + expect(launch.cmdArgs[1]).toBe(launch.entryPath); + }); +}); + +describe("resolveUnifiedRuntimeRoot", () => { + it("returns an absolute path", () => { + const root = resolveUnifiedRuntimeRoot(); + expect(typeof root).toBe("string"); + expect(path.isAbsolute(root)).toBe(true); + }); + + it("returns the same result as the underlying resolveRepoRuntimeRoot", async () => { + // Since resolveUnifiedRuntimeRoot delegates to resolveRepoRuntimeRoot, + // both should return the same value in the same environment + const { resolveRepoRuntimeRoot: directResolver } = await import("../runtime/adeMcpLaunch"); + expect(resolveUnifiedRuntimeRoot()).toBe(directResolver()); + }); }); describe("buildClaudeReadOnlyWorkerAllowedTools", () => { diff --git a/apps/desktop/src/main/services/orchestrator/unifiedOrchestratorAdapter.ts b/apps/desktop/src/main/services/orchestrator/unifiedOrchestratorAdapter.ts index b77ee7e9..f36d6da0 100644 --- a/apps/desktop/src/main/services/orchestrator/unifiedOrchestratorAdapter.ts +++ b/apps/desktop/src/main/services/orchestrator/unifiedOrchestratorAdapter.ts @@ -26,7 +26,7 @@ import { normalizeMissionPermissions, providerPermissionsToLegacyConfig, } from "./permissionMapping"; -import { resolveDesktopAdeMcpLaunch, resolveRepoRuntimeRoot } from "../runtime/adeMcpLaunch"; +import { type AdeMcpLaunch, resolveDesktopAdeMcpLaunch, resolveRepoRuntimeRoot } from "../runtime/adeMcpLaunch"; /** * Build environment variable assignments for worker identity. @@ -69,31 +69,8 @@ export function resolveAdeMcpServerLaunch(args: { computerUsePolicy?: ComputerUsePolicy | null; bundledProxyPath?: string; preferBundledProxy?: boolean; -}): { - mode: "bundled_proxy" | "headless_built" | "headless_source"; - command: string; - cmdArgs: string[]; - env: Record; - entryPath: string; - runtimeRoot: string | null; - socketPath: string; - packaged: boolean; - resourcesPath: string | null; -} { - return resolveDesktopAdeMcpLaunch({ - projectRoot: args.projectRoot, - workspaceRoot: args.workspaceRoot, - runtimeRoot: args.runtimeRoot, - missionId: args.missionId, - runId: args.runId, - stepId: args.stepId, - attemptId: args.attemptId, - defaultRole: args.defaultRole, - ownerId: args.ownerId, - computerUsePolicy: args.computerUsePolicy, - bundledProxyPath: args.bundledProxyPath, - preferBundledProxy: args.preferBundledProxy, - }); +}): AdeMcpLaunch { + return resolveDesktopAdeMcpLaunch(args); } export function getUnifiedUnsupportedModelReason(modelRef: string): string | null { diff --git a/apps/desktop/src/main/services/processes/processService.test.ts b/apps/desktop/src/main/services/processes/processService.test.ts new file mode 100644 index 00000000..65f555a4 --- /dev/null +++ b/apps/desktop/src/main/services/processes/processService.test.ts @@ -0,0 +1,220 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it, vi } from "vitest"; +import { openKvDb } from "../state/kvDb"; +import { createProcessService } from "./processService"; + +function createLogger() { + return { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + } as any; +} + +function makeMinimalConfig(processes: Array<{ + id: string; + command: string[]; + cwd?: string; +}>) { + const defs = processes.map((p) => ({ + id: p.id, + name: p.id, + command: p.command, + cwd: p.cwd ?? ".", + env: {}, + readiness: { type: "immediate" as const }, + restart: { policy: "never" as const }, + dependsOn: [], + healthCheck: null, + icon: null, + color: null, + description: null, + })); + return { + effective: { + processes: defs, + stackButtons: [], + laneOverlayPolicies: [], + }, + local: {}, + }; +} + +function makeLaneSummary(tmpDir: string, laneId: string) { + return { + id: laneId, + name: laneId, + description: null, + laneType: "worktree", + branchRef: "feature/test", + baseRef: "main", + worktreePath: tmpDir, + attachedRootPath: null, + isEditProtected: false, + parentLaneId: null, + color: null, + icon: null, + tags: [], + status: { dirty: false, ahead: 0, behind: 0, conflict: "unknown", tests: "unknown", pr: "none" }, + stackDepth: 0, + createdAt: "2026-03-24T12:00:00.000Z", + archivedAt: null, + }; +} + +/** Wait for a process to fully exit by polling its runtime status. */ +async function waitForExit( + service: ReturnType, + laneId: string, + processId: string, + timeoutMs = 5000, +): Promise { + const start = Date.now(); + while (Date.now() - start < timeoutMs) { + const runtimes = service.listRuntime(laneId); + const rt = runtimes.find((r) => r.processId === processId); + if (rt && (rt.status === "stopped" || rt.status === "crashed")) return; + await new Promise((r) => setTimeout(r, 50)); + } +} + +describe("processService start logging", () => { + it("includes envPath and envShell in the process.start log entry", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-process-startlog-")); + const dbPath = path.join(tmpDir, "kv.sqlite"); + const logsDir = path.join(tmpDir, "logs"); + const projectId = "proj-startlog"; + const logger = createLogger(); + + const db = await openKvDb(dbPath, createLogger()); + const now = "2026-03-24T12:00:00.000Z"; + db.run( + "insert into projects(id, root_path, display_name, default_base_ref, created_at, last_opened_at) values (?, ?, ?, ?, ?, ?)", + [projectId, tmpDir, "test", "main", now, now], + ); + db.run( + `insert into lanes( + id, project_id, name, description, lane_type, base_ref, branch_ref, worktree_path, + attached_root_path, is_edit_protected, parent_lane_id, color, icon, tags_json, status, created_at, archived_at + ) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ["lane-ok", projectId, "Lane OK", null, "worktree", "main", "feature/ok", tmpDir, null, 0, null, null, null, null, "active", now, null], + ); + + const config = makeMinimalConfig([ + { id: "echo-proc", command: ["echo", "hello"] }, + ]); + + const service = createProcessService({ + db, + projectId, + processLogsDir: logsDir, + logger, + laneService: { + getLaneWorktreePath: () => tmpDir, + list: async () => [makeLaneSummary(tmpDir, "lane-ok")], + } as any, + projectConfigService: { + get: () => config, + getEffective: () => config.effective, + getExecutableConfig: () => config.effective, + } as any, + broadcastEvent: () => {}, + }); + + try { + const runtime = await service.start({ laneId: "lane-ok", processId: "echo-proc" }); + expect(runtime.status).toMatch(/starting|running|stopped/); + + // Wait for echo to complete before asserting / closing db + await waitForExit(service, "lane-ok", "echo-proc"); + + const infoCalls = logger.info.mock.calls.filter( + (call: any[]) => call[0] === "process.start", + ); + expect(infoCalls.length).toBe(1); + const logData = infoCalls[0][1]; + expect(logData).toHaveProperty("envPath"); + expect(logData).toHaveProperty("envShell"); + expect(logData.processId).toBe("echo-proc"); + expect(logData.laneId).toBe("lane-ok"); + expect(logData.runId).toBeTruthy(); + expect(logData.command).toEqual(["echo", "hello"]); + } finally { + service.disposeAll(); + db.close(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("transitions to crashed status when the spawned process exits with non-zero code", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "ade-process-error-")); + const dbPath = path.join(tmpDir, "kv.sqlite"); + const logsDir = path.join(tmpDir, "logs"); + const projectId = "proj-crash"; + const logger = createLogger(); + const events: any[] = []; + + const db = await openKvDb(dbPath, createLogger()); + const now = "2026-03-24T12:00:00.000Z"; + db.run( + "insert into projects(id, root_path, display_name, default_base_ref, created_at, last_opened_at) values (?, ?, ?, ?, ?, ?)", + [projectId, tmpDir, "test", "main", now, now], + ); + db.run( + `insert into lanes( + id, project_id, name, description, lane_type, base_ref, branch_ref, worktree_path, + attached_root_path, is_edit_protected, parent_lane_id, color, icon, tags_json, status, created_at, archived_at + ) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ["lane-err", projectId, "Lane Error", null, "worktree", "main", "feature/err", tmpDir, null, 0, null, null, null, null, "active", now, null], + ); + + const config = makeMinimalConfig([ + { id: "fail-proc", command: ["sh", "-c", "exit 42"] }, + ]); + + const service = createProcessService({ + db, + projectId, + processLogsDir: logsDir, + logger, + laneService: { + getLaneWorktreePath: () => tmpDir, + list: async () => [makeLaneSummary(tmpDir, "lane-err")], + } as any, + projectConfigService: { + get: () => config, + getEffective: () => config.effective, + getExecutableConfig: () => config.effective, + } as any, + broadcastEvent: (ev: any) => events.push(ev), + }); + + try { + await service.start({ laneId: "lane-err", processId: "fail-proc" }); + + // Wait for the process to exit + await waitForExit(service, "lane-err", "fail-proc"); + + const runtimes = service.listRuntime("lane-err"); + const current = runtimes.find((r) => r.processId === "fail-proc"); + expect(current).toBeTruthy(); + expect(current!.status).toBe("crashed"); + expect(current!.lastExitCode).toBe(42); + + const runRow = db.get<{ exit_code: number | null; termination_reason: string }>( + "select exit_code, termination_reason from process_runs where project_id = ? and process_key = ?", + [projectId, "fail-proc"], + ); + expect(runRow).toBeTruthy(); + expect(runRow!.exit_code).toBe(42); + expect(runRow!.termination_reason).toBe("crashed"); + } finally { + service.disposeAll(); + db.close(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); +}); diff --git a/apps/desktop/src/main/services/prs/prPollingService.test.ts b/apps/desktop/src/main/services/prs/prPollingService.test.ts index bffd662b..6334153f 100644 --- a/apps/desktop/src/main/services/prs/prPollingService.test.ts +++ b/apps/desktop/src/main/services/prs/prPollingService.test.ts @@ -140,4 +140,334 @@ describe("prPollingService", () => { expect(refresh).toHaveBeenCalledTimes(2); expect(refresh).toHaveBeenLastCalledWith({ prIds: ["pr-1"] }); }); + + it("emits review_requested notification with generic messaging", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00.000Z")); + vi.spyOn(Math, "random").mockReturnValue(0.5); + + let summary = createSummary({ + title: "Add feature", + headBranch: "feature/add", + checksStatus: "passing", + reviewStatus: "none", + }); + let refreshCount = 0; + const events: any[] = []; + + const prService = { + listAll: () => [summary], + refresh: vi.fn(async () => { + refreshCount += 1; + if (refreshCount >= 2) { + summary = { + ...summary, + reviewStatus: "requested" as const, + updatedAt: new Date(Date.now()).toISOString(), + }; + } + return [summary]; + }), + getHotRefreshDelayMs: () => null, + getHotRefreshPrIds: () => [], + } as any; + + const service = createPrPollingService({ + logger: createLogger() as any, + prService, + projectConfigService: { get: () => ({ effective: {} }) } as any, + onEvent: (event) => events.push(event), + }); + + service.start(); + await vi.advanceTimersByTimeAsync(12_000); + + service.poke(); + await vi.advanceTimersByTimeAsync(0); + + expect(events).toContainEqual(expect.objectContaining({ + type: "pr-notification", + kind: "review_requested", + title: "Review requested", + message: "This pull request is waiting on an approving review.", + prTitle: "Add feature", + repoOwner: "acme", + repoName: "ade", + baseBranch: "main", + headBranch: "feature/add", + })); + }); + + it("emits changes_requested notification with generic messaging", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00.000Z")); + vi.spyOn(Math, "random").mockReturnValue(0.5); + + let summary = createSummary({ + title: "Refactor module", + headBranch: "refactor/module", + checksStatus: "passing", + reviewStatus: "requested", + }); + let refreshCount = 0; + const events: any[] = []; + + const prService = { + listAll: () => [summary], + refresh: vi.fn(async () => { + refreshCount += 1; + if (refreshCount >= 2) { + summary = { + ...summary, + reviewStatus: "changes_requested" as const, + updatedAt: new Date(Date.now()).toISOString(), + }; + } + return [summary]; + }), + getHotRefreshDelayMs: () => null, + getHotRefreshPrIds: () => [], + } as any; + + const service = createPrPollingService({ + logger: createLogger() as any, + prService, + projectConfigService: { get: () => ({ effective: {} }) } as any, + onEvent: (event) => events.push(event), + }); + + service.start(); + await vi.advanceTimersByTimeAsync(12_000); + + service.poke(); + await vi.advanceTimersByTimeAsync(0); + + expect(events).toContainEqual(expect.objectContaining({ + type: "pr-notification", + kind: "changes_requested", + title: "Changes requested", + message: "A reviewer requested changes before this pull request can merge.", + prTitle: "Refactor module", + repoOwner: "acme", + repoName: "ade", + })); + }); + + it("emits merge_ready notification when checks pass and review is approved", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00.000Z")); + vi.spyOn(Math, "random").mockReturnValue(0.5); + + let summary = createSummary({ + title: "Ready PR", + headBranch: "feature/ready", + checksStatus: "pending", + reviewStatus: "approved", + }); + let refreshCount = 0; + const events: any[] = []; + + const prService = { + listAll: () => [summary], + refresh: vi.fn(async () => { + refreshCount += 1; + if (refreshCount >= 2) { + summary = { + ...summary, + checksStatus: "passing" as const, + updatedAt: new Date(Date.now()).toISOString(), + }; + } + return [summary]; + }), + getHotRefreshDelayMs: () => null, + getHotRefreshPrIds: () => [], + } as any; + + const service = createPrPollingService({ + logger: createLogger() as any, + prService, + projectConfigService: { get: () => ({ effective: {} }) } as any, + onEvent: (event) => events.push(event), + }); + + service.start(); + await vi.advanceTimersByTimeAsync(12_000); + + service.poke(); + await vi.advanceTimersByTimeAsync(0); + + expect(events).toContainEqual(expect.objectContaining({ + type: "pr-notification", + kind: "merge_ready", + title: "Checks passing & approved", + message: expect.stringContaining("Required checks are passing"), + prTitle: "Ready PR", + repoOwner: "acme", + repoName: "ade", + baseBranch: "main", + headBranch: "feature/ready", + })); + }); + + it("notification title no longer includes the PR number", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00.000Z")); + vi.spyOn(Math, "random").mockReturnValue(0.5); + + let summary = createSummary({ + githubPrNumber: 999, + checksStatus: "passing", + reviewStatus: "none", + }); + let refreshCount = 0; + const events: any[] = []; + + const prService = { + listAll: () => [summary], + refresh: vi.fn(async () => { + refreshCount += 1; + if (refreshCount >= 2) { + summary = { + ...summary, + checksStatus: "failing" as const, + updatedAt: new Date(Date.now()).toISOString(), + }; + } + return [summary]; + }), + getHotRefreshDelayMs: () => null, + getHotRefreshPrIds: () => [], + } as any; + + const service = createPrPollingService({ + logger: createLogger() as any, + prService, + projectConfigService: { get: () => ({ effective: {} }) } as any, + onEvent: (event) => events.push(event), + }); + + service.start(); + await vi.advanceTimersByTimeAsync(12_000); + + service.poke(); + await vi.advanceTimersByTimeAsync(0); + + const notification = events.find((e) => e.type === "pr-notification" && e.kind === "checks_failing"); + expect(notification, "Expected a checks_failing notification to be emitted").toBeTruthy(); + // Title should NOT contain #999 any more + expect(notification.title).not.toContain("#999"); + expect(notification.title).toBe("Checks failing"); + }); + + it("includes onPullRequestsChanged hook with changed PRs details", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00.000Z")); + vi.spyOn(Math, "random").mockReturnValue(0.5); + + let summary = createSummary({ + checksStatus: "passing", + reviewStatus: "approved", + }); + let refreshCount = 0; + const changedCalls: any[] = []; + + const prService = { + listAll: () => [summary], + refresh: vi.fn(async () => { + refreshCount += 1; + if (refreshCount >= 2) { + summary = { + ...summary, + checksStatus: "failing" as const, + updatedAt: new Date(Date.now()).toISOString(), + }; + } + return [summary]; + }), + getHotRefreshDelayMs: () => null, + getHotRefreshPrIds: () => [], + } as any; + + const service = createPrPollingService({ + logger: createLogger() as any, + prService, + projectConfigService: { get: () => ({ effective: {} }) } as any, + onEvent: () => {}, + onPullRequestsChanged: (args) => { changedCalls.push(args); }, + }); + + service.start(); + // First tick initializes + await vi.advanceTimersByTimeAsync(12_000); + expect(changedCalls).toHaveLength(0); + + // Second tick has changed data + service.poke(); + await vi.advanceTimersByTimeAsync(0); + + expect(changedCalls).toHaveLength(1); + expect(changedCalls[0].changedPrs).toHaveLength(1); + expect(changedCalls[0].changes[0]).toEqual(expect.objectContaining({ + previousChecksStatus: "passing", + })); + expect(changedCalls[0].changes[0].pr.checksStatus).toBe("failing"); + }); + + it("emits informative PR notifications with PR metadata", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00.000Z")); + vi.spyOn(Math, "random").mockReturnValue(0.5); + + let summary = createSummary({ + title: "Fix lanes tab", + headBranch: "fix-lanes-tab", + checksStatus: "passing", + reviewStatus: "approved", + }); + let refreshCount = 0; + const events: any[] = []; + + const prService = { + listAll: () => [summary], + refresh: vi.fn(async () => { + refreshCount += 1; + if (refreshCount >= 2) { + summary = { + ...summary, + checksStatus: "failing", + updatedAt: new Date(Date.now()).toISOString(), + }; + } + return [summary]; + }), + getHotRefreshDelayMs: () => null, + getHotRefreshPrIds: () => [], + } as any; + + const service = createPrPollingService({ + logger: createLogger() as any, + prService, + projectConfigService: { get: () => ({ effective: {} }) } as any, + onEvent: (event) => events.push(event), + }); + + service.start(); + await vi.advanceTimersByTimeAsync(12_000); + + service.poke(); + await vi.advanceTimersByTimeAsync(0); + + expect(events).toContainEqual(expect.objectContaining({ + type: "pr-notification", + kind: "checks_failing", + title: "Checks failing", + prTitle: "Fix lanes tab", + repoOwner: "acme", + repoName: "ade", + baseBranch: "main", + headBranch: "fix-lanes-tab", + message: "One or more required CI checks failed on this pull request.", + })); + }); }); diff --git a/apps/desktop/src/main/services/prs/prPollingService.ts b/apps/desktop/src/main/services/prs/prPollingService.ts index e9be0294..d9f23e40 100644 --- a/apps/desktop/src/main/services/prs/prPollingService.ts +++ b/apps/desktop/src/main/services/prs/prPollingService.ts @@ -17,18 +17,29 @@ function jitterMs(value: number): number { return Math.max(1000, Math.round(value + rand)); } -function summarizeNotification(args: { kind: PrNotificationKind; pr: PrSummary }): { title: string; message: string } { - const prLabel = args.pr.githubPrNumber ? `#${args.pr.githubPrNumber}` : "PR"; - if (args.kind === "checks_failing") { - return { title: `Checks failing ${prLabel}`, message: args.pr.title || "A pull request has failing checks." }; +function summarizeNotification(kind: PrNotificationKind): { title: string; message: string } { + switch (kind) { + case "checks_failing": + return { + title: "Checks failing", + message: "One or more required CI checks failed on this pull request.", + }; + case "review_requested": + return { + title: "Review requested", + message: "This pull request is waiting on an approving review.", + }; + case "changes_requested": + return { + title: "Changes requested", + message: "A reviewer requested changes before this pull request can merge.", + }; + case "merge_ready": + return { + title: "Checks passing & approved", + message: "Required checks are passing and the pull request has approval. Other merge requirements (e.g. base branch currency) may still apply.", + }; } - if (args.kind === "review_requested") { - return { title: `Review requested ${prLabel}`, message: args.pr.title || "A pull request needs review." }; - } - if (args.kind === "changes_requested") { - return { title: `Changes requested ${prLabel}`, message: args.pr.title || "A pull request has requested changes." }; - } - return { title: `Merge ready ${prLabel}`, message: args.pr.title || "A pull request looks merge-ready." }; } /** @@ -217,7 +228,7 @@ export function createPrPollingService({ const kinds: PrNotificationKind[] = ["checks_failing", "review_requested", "changes_requested", "merge_ready"]; for (const kind of kinds) { if (!shouldNotify(kind)) continue; - const summary = summarizeNotification({ kind, pr }); + const summary = summarizeNotification(kind); onEvent({ type: "pr-notification", polledAt, diff --git a/apps/desktop/src/main/services/runtime/adeMcpLaunch.test.ts b/apps/desktop/src/main/services/runtime/adeMcpLaunch.test.ts index 8b900a98..f21259f0 100644 --- a/apps/desktop/src/main/services/runtime/adeMcpLaunch.test.ts +++ b/apps/desktop/src/main/services/runtime/adeMcpLaunch.test.ts @@ -2,7 +2,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { resolveDesktopAdeMcpLaunch } from "./adeMcpLaunch"; +import { resolveDesktopAdeMcpLaunch, resolveRepoRuntimeRoot } from "./adeMcpLaunch"; const originalResourcesPath = process.resourcesPath; @@ -106,4 +106,197 @@ describe("resolveDesktopAdeMcpLaunch", () => { expect(launch.entryPath).toBe(packagedProxy); expect(launch.cmdArgs[0]).toBe(packagedProxy); }); + + it("falls back to headless source mode when no built entry exists", () => { + const runtimeRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-runtime-src-")); + const projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-project-src-")); + const workspaceRoot = path.join(projectRoot, "workspace"); + + fs.mkdirSync(workspaceRoot, { recursive: true }); + // Create the mcp-server src directory but NOT the dist directory + fs.mkdirSync(path.join(runtimeRoot, "apps", "mcp-server", "src"), { recursive: true }); + + const launch = resolveDesktopAdeMcpLaunch({ + projectRoot, + workspaceRoot, + runtimeRoot, + preferBundledProxy: false, + }); + + const expectedSrcEntry = path.join(runtimeRoot, "apps", "mcp-server", "src", "index.ts"); + expect(launch.mode).toBe("headless_source"); + expect(launch.command).toBe("npx"); + expect(launch.cmdArgs).toEqual([ + "tsx", + expectedSrcEntry, + "--project-root", + path.resolve(projectRoot), + "--workspace-root", + path.resolve(workspaceRoot), + ]); + expect(launch.entryPath).toBe(expectedSrcEntry); + expect(launch.runtimeRoot).toBe(path.resolve(runtimeRoot)); + }); + + it("defaults projectRoot to workspaceRoot when projectRoot is empty or missing", () => { + const runtimeRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-runtime-nopr-")); + const workspaceRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-ws-nopr-")); + + const launch = resolveDesktopAdeMcpLaunch({ + workspaceRoot, + runtimeRoot, + preferBundledProxy: false, + }); + + expect(launch.env.ADE_PROJECT_ROOT).toBe(path.resolve(workspaceRoot)); + expect(launch.env.ADE_WORKSPACE_ROOT).toBe(path.resolve(workspaceRoot)); + expect(launch.socketPath).toBe(path.join(path.resolve(workspaceRoot), ".ade", "mcp.sock")); + + // Also test with empty string projectRoot + const launchEmpty = resolveDesktopAdeMcpLaunch({ + projectRoot: " ", + workspaceRoot, + runtimeRoot, + preferBundledProxy: false, + }); + + expect(launchEmpty.env.ADE_PROJECT_ROOT).toBe(path.resolve(workspaceRoot)); + }); + + it("populates computerUsePolicy env vars when policy is provided", () => { + const runtimeRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-runtime-cup-")); + const projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-project-cup-")); + const workspaceRoot = path.join(projectRoot, "workspace"); + fs.mkdirSync(workspaceRoot, { recursive: true }); + + const launch = resolveDesktopAdeMcpLaunch({ + projectRoot, + workspaceRoot, + runtimeRoot, + preferBundledProxy: false, + computerUsePolicy: { + mode: "enabled", + allowLocalFallback: true, + retainArtifacts: false, + preferredBackend: "vnc", + }, + }); + + expect(launch.env.ADE_COMPUTER_USE_MODE).toBe("enabled"); + expect(launch.env.ADE_COMPUTER_USE_ALLOW_LOCAL_FALLBACK).toBe("1"); + expect(launch.env.ADE_COMPUTER_USE_RETAIN_ARTIFACTS).toBe("0"); + expect(launch.env.ADE_COMPUTER_USE_PREFERRED_BACKEND).toBe("vnc"); + }); + + it("leaves computerUsePolicy env vars empty when policy is null", () => { + const runtimeRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-runtime-nocup-")); + const projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-project-nocup-")); + const workspaceRoot = path.join(projectRoot, "workspace"); + fs.mkdirSync(workspaceRoot, { recursive: true }); + + const launch = resolveDesktopAdeMcpLaunch({ + projectRoot, + workspaceRoot, + runtimeRoot, + preferBundledProxy: false, + computerUsePolicy: null, + }); + + expect(launch.env.ADE_COMPUTER_USE_MODE).toBe(""); + expect(launch.env.ADE_COMPUTER_USE_ALLOW_LOCAL_FALLBACK).toBe(""); + expect(launch.env.ADE_COMPUTER_USE_RETAIN_ARTIFACTS).toBe(""); + expect(launch.env.ADE_COMPUTER_USE_PREFERRED_BACKEND).toBe(""); + }); + + it("sets ownerId and defaultRole in env when provided", () => { + const runtimeRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-runtime-owner-")); + const projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-project-owner-")); + const workspaceRoot = path.join(projectRoot, "workspace"); + fs.mkdirSync(workspaceRoot, { recursive: true }); + + const launch = resolveDesktopAdeMcpLaunch({ + projectRoot, + workspaceRoot, + runtimeRoot, + preferBundledProxy: false, + defaultRole: "cto", + ownerId: "agent-42", + }); + + expect(launch.env.ADE_DEFAULT_ROLE).toBe("cto"); + expect(launch.env.ADE_OWNER_ID).toBe("agent-42"); + }); + + it("defaults defaultRole to 'agent' and ownerId to empty when not provided", () => { + const runtimeRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-runtime-noowner-")); + const projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-project-noowner-")); + const workspaceRoot = path.join(projectRoot, "workspace"); + fs.mkdirSync(workspaceRoot, { recursive: true }); + + const launch = resolveDesktopAdeMcpLaunch({ + projectRoot, + workspaceRoot, + runtimeRoot, + preferBundledProxy: false, + }); + + expect(launch.env.ADE_DEFAULT_ROLE).toBe("agent"); + expect(launch.env.ADE_OWNER_ID).toBe(""); + }); + + it("bundled proxy mode preserves runtimeRoot when provided", () => { + const runtimeRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-runtime-proxy-rt-")); + const projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-project-proxy-rt-")); + const workspaceRoot = path.join(projectRoot, "workspace"); + const proxyEntry = path.join(projectRoot, "dist", "main", "adeMcpProxy.cjs"); + + fs.mkdirSync(workspaceRoot, { recursive: true }); + fs.mkdirSync(path.dirname(proxyEntry), { recursive: true }); + fs.writeFileSync(proxyEntry, "module.exports = {};\n", "utf8"); + + const launch = resolveDesktopAdeMcpLaunch({ + projectRoot, + workspaceRoot, + runtimeRoot, + bundledProxyPath: proxyEntry, + }); + + expect(launch.mode).toBe("bundled_proxy"); + expect(launch.runtimeRoot).toBe(path.resolve(runtimeRoot)); + }); + + it("bundled proxy mode sets runtimeRoot to null when not provided", () => { + const projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-launch-project-proxy-nort-")); + const workspaceRoot = path.join(projectRoot, "workspace"); + const proxyEntry = path.join(projectRoot, "dist", "main", "adeMcpProxy.cjs"); + + fs.mkdirSync(workspaceRoot, { recursive: true }); + fs.mkdirSync(path.dirname(proxyEntry), { recursive: true }); + fs.writeFileSync(proxyEntry, "module.exports = {};\n", "utf8"); + + const launch = resolveDesktopAdeMcpLaunch({ + projectRoot, + workspaceRoot, + bundledProxyPath: proxyEntry, + }); + + expect(launch.mode).toBe("bundled_proxy"); + expect(launch.runtimeRoot).toBeNull(); + }); +}); + +describe("resolveRepoRuntimeRoot", () => { + it("returns a string path that is a resolved absolute path", () => { + const root = resolveRepoRuntimeRoot(); + expect(typeof root).toBe("string"); + expect(path.isAbsolute(root)).toBe(true); + }); + + it("finds the monorepo root when apps/mcp-server/package.json exists above cwd", () => { + // The ADE project itself has this structure, so running in the repo should find it + const root = resolveRepoRuntimeRoot(); + // The function should find a directory containing apps/mcp-server/package.json + // or fall back to cwd. Either way, it returns a valid path. + expect(fs.existsSync(root)).toBe(true); + }); }); diff --git a/apps/desktop/src/main/services/runtime/adeMcpLaunch.ts b/apps/desktop/src/main/services/runtime/adeMcpLaunch.ts index 6ae8291d..ad5fa080 100644 --- a/apps/desktop/src/main/services/runtime/adeMcpLaunch.ts +++ b/apps/desktop/src/main/services/runtime/adeMcpLaunch.ts @@ -91,15 +91,8 @@ export function resolveRepoRuntimeRoot(): string { function buildLaunchEnv(args: { projectRoot: string; workspaceRoot: string; - missionId?: string; - runId?: string; - stepId?: string; - attemptId?: string; - defaultRole?: string; - ownerId?: string; socketPath: string; - computerUsePolicy?: ComputerUsePolicy | null; -}): Record { +} & Pick): Record { return { ADE_PROJECT_ROOT: args.projectRoot, ADE_WORKSPACE_ROOT: args.workspaceRoot, diff --git a/apps/desktop/src/renderer/components/app/AppShell.tsx b/apps/desktop/src/renderer/components/app/AppShell.tsx index 0ddf2216..805c263d 100644 --- a/apps/desktop/src/renderer/components/app/AppShell.tsx +++ b/apps/desktop/src/renderer/components/app/AppShell.tsx @@ -1,9 +1,19 @@ import React, { useEffect, useMemo, useRef, useState } from "react"; +import { + ArrowSquareOut, + CheckCircle, + GitBranch, + GithubLogo, + GitPullRequest, + WarningCircle, + XCircle, +} from "@phosphor-icons/react"; import { Link, useLocation, useNavigate } from "react-router-dom"; import { CommandPalette } from "./CommandPalette"; import { TabNav } from "./TabNav"; import { TopBar } from "./TopBar"; import { RightEdgeFloatingPane } from "./RightEdgeFloatingPane"; +import { getPrToastHeadline, getPrToastMeta, getPrToastSummary, getPrToastTone, type PrToastTone } from "./prToastPresentation"; import { TabBackground } from "../ui/TabBackground"; import { useAppStore } from "../../state/appStore"; import { Button } from "../ui/Button"; @@ -56,6 +66,51 @@ function shortId(id: string): string { return trimmed.length <= 8 ? trimmed : trimmed.slice(0, 8); } +function getPrToastToneClasses(tone: PrToastTone): { + panel: string; + badge: string; + iconWrap: string; + iconClass: string; +} { + if (tone === "danger") { + return { + panel: "border-red-500/25 bg-card/95", + badge: "border border-red-500/30 bg-red-500/10 text-red-300", + iconWrap: "border border-red-500/30 bg-red-500/12", + iconClass: "text-red-300", + }; + } + if (tone === "warning") { + return { + panel: "border-amber-500/25 bg-card/95", + badge: "border border-amber-500/30 bg-amber-500/10 text-amber-300", + iconWrap: "border border-amber-500/30 bg-amber-500/12", + iconClass: "text-amber-300", + }; + } + if (tone === "success") { + return { + panel: "border-emerald-500/25 bg-card/95", + badge: "border border-emerald-500/30 bg-emerald-500/10 text-emerald-300", + iconWrap: "border border-emerald-500/30 bg-emerald-500/12", + iconClass: "text-emerald-300", + }; + } + return { + panel: "border-sky-500/25 bg-card/95", + badge: "border border-sky-500/30 bg-sky-500/10 text-sky-300", + iconWrap: "border border-sky-500/30 bg-sky-500/12", + iconClass: "text-sky-300", + }; +} + +function getPrToastIcon(kind: PrToast["event"]["kind"]) { + if (kind === "checks_failing") return XCircle; + if (kind === "changes_requested") return WarningCircle; + if (kind === "merge_ready") return CheckCircle; + return GitPullRequest; +} + export function AppShell({ children }: { children: React.ReactNode }) { const location = useLocation(); const navigate = useNavigate(); @@ -80,6 +135,12 @@ export function AppShell({ children }: { children: React.ReactNode }) { const isFirstVisit = !visitedTabsRef.current.has(location.pathname); const [prToasts, setPrToasts] = useState([]); const toastTimersRef = useRef>(new Map()); + const dismissPrToast = (id: string) => { + setPrToasts((prev) => prev.filter((t) => t.id !== id)); + const timer = toastTimersRef.current.get(id); + if (timer != null) window.clearTimeout(timer); + toastTimersRef.current.delete(id); + }; const [linearWorkflowToasts, setLinearWorkflowToasts] = useState([]); const linearToastTimersRef = useRef>(new Map()); const [aiFailure, setAiFailure] = useState(null); @@ -614,53 +675,93 @@ export function AppShell({ children }: { children: React.ReactNode }) {
{prToasts.map((toast) => { const laneName = lanes.find((lane) => lane.id === toast.event.laneId)?.name ?? toast.event.laneId; + const tone = getPrToastTone(toast.event.kind); + const toneClasses = getPrToastToneClasses(tone); + const Icon = getPrToastIcon(toast.event.kind); + const headline = getPrToastHeadline(toast.event); + const summary = getPrToastSummary(toast.event); + const meta = getPrToastMeta(toast.event, laneName); return ( -
-
-
-
{toast.event.title}
-
{laneName}
+
+
+
+ +
+
+
+
+
+ + {toast.event.title} + + #{toast.event.prNumber} +
+
+ {headline} +
+
+ +
+ {meta.length > 0 ? ( +
+ {meta.map((item, index) => ( + + {item.includes("/") ? : item.includes("#") || (toast.event.repoOwner && item.includes(toast.event.repoOwner)) ? : } + {item} + + ))} +
+ ) : null} +
{summary}
+
+ + +
- -
-
{toast.event.message}
-
- -
); diff --git a/apps/desktop/src/renderer/components/app/prToastPresentation.test.ts b/apps/desktop/src/renderer/components/app/prToastPresentation.test.ts index 072456fa..43844408 100644 --- a/apps/desktop/src/renderer/components/app/prToastPresentation.test.ts +++ b/apps/desktop/src/renderer/components/app/prToastPresentation.test.ts @@ -21,22 +21,142 @@ const baseEvent = { reviewStatus: "requested" as const, }; -describe("prToastPresentation", () => { +describe("getPrToastTone", () => { + it("maps checks_failing to danger", () => { + expect(getPrToastTone("checks_failing")).toBe("danger"); + }); + + it("maps changes_requested to danger", () => { + expect(getPrToastTone("changes_requested")).toBe("danger"); + }); + + it("maps review_requested to warning", () => { + expect(getPrToastTone("review_requested")).toBe("warning"); + }); + + it("maps merge_ready to success", () => { + expect(getPrToastTone("merge_ready")).toBe("success"); + }); + + it("returns info for unknown kinds", () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect(getPrToastTone("some_future_kind" as any)).toBe("info"); + }); +}); + +describe("getPrToastHeadline", () => { it("uses the PR title as the toast headline", () => { expect(getPrToastHeadline(baseEvent)).toBe("Fix lanes tab"); }); - it("returns informative summary text instead of duplicating the title", () => { + it("falls back to title when prTitle is null", () => { + const event = { ...baseEvent, prTitle: null as unknown as string }; + expect(getPrToastHeadline(event)).toBe("Checks failing"); + }); + + it("falls back to title when prTitle is empty", () => { + const event = { ...baseEvent, prTitle: "" }; + expect(getPrToastHeadline(event)).toBe("Checks failing"); + }); + + it("falls back to title when prTitle is whitespace-only", () => { + const event = { ...baseEvent, prTitle: " " }; + expect(getPrToastHeadline(event)).toBe("Checks failing"); + }); + + it("falls back to generic text when both prTitle and title are empty", () => { + const event = { ...baseEvent, prTitle: "", title: "" }; + expect(getPrToastHeadline(event)).toBe("Pull request #82"); + }); + + it("falls back to generic text when both prTitle and title are null", () => { + const event = { ...baseEvent, prTitle: null as unknown as string, title: null as unknown as string }; + expect(getPrToastHeadline(event)).toBe("Pull request #82"); + }); +}); + +describe("getPrToastSummary", () => { + it("returns informative summary text", () => { expect(getPrToastSummary(baseEvent)).toBe("One or more required CI checks failed on this pull request."); }); - it("deduplicates repeated lane and branch labels in metadata", () => { - expect(getPrToastMeta(baseEvent, "fix-lanes-tab")).toEqual(["fix-lanes-tab", "fix-lanes-tab -> main", "ade-dev/ade"]); + it("falls back to default text when message is empty", () => { + const event = { ...baseEvent, message: "" }; + expect(getPrToastSummary(event)).toBe("Pull request status changed."); }); - it("maps PR notification kinds to toast tones", () => { - expect(getPrToastTone("checks_failing")).toBe("danger"); - expect(getPrToastTone("review_requested")).toBe("warning"); - expect(getPrToastTone("merge_ready")).toBe("success"); + it("falls back to default text when message is whitespace-only", () => { + const event = { ...baseEvent, message: " " }; + expect(getPrToastSummary(event)).toBe("Pull request status changed."); + }); + + it("falls back to default text when message is null", () => { + const event = { ...baseEvent, message: null as unknown as string }; + expect(getPrToastSummary(event)).toBe("Pull request status changed."); + }); +}); + +describe("getPrToastMeta", () => { + it("returns lane name, branch label, and repo label", () => { + expect(getPrToastMeta(baseEvent, "fix-lanes-tab")).toEqual([ + "fix-lanes-tab", + "fix-lanes-tab -> main", + "ade-dev/ade", + ]); + }); + + it("omits lane name when it is null", () => { + const meta = getPrToastMeta(baseEvent, null); + expect(meta).toEqual(["fix-lanes-tab -> main", "ade-dev/ade"]); + }); + + it("omits lane name when it is empty", () => { + const meta = getPrToastMeta(baseEvent, ""); + expect(meta).toEqual(["fix-lanes-tab -> main", "ade-dev/ade"]); + }); + + it("omits branch label when both headBranch and baseBranch are missing", () => { + const event = { ...baseEvent, headBranch: null as unknown as string, baseBranch: null as unknown as string }; + const meta = getPrToastMeta(event, "my-lane"); + expect(meta).toEqual(["my-lane", "ade-dev/ade"]); + }); + + it("shows only headBranch when baseBranch is missing", () => { + const event = { ...baseEvent, baseBranch: null as unknown as string }; + const meta = getPrToastMeta(event, "my-lane"); + expect(meta).toContain("fix-lanes-tab"); + expect(meta.some((item) => item.includes("->"))).toBe(false); + }); + + it("shows only baseBranch when headBranch is missing", () => { + const event = { ...baseEvent, headBranch: null as unknown as string }; + const meta = getPrToastMeta(event, "my-lane"); + expect(meta).toContain("main"); + }); + + it("omits repo label when repoOwner and repoName are missing", () => { + const event = { ...baseEvent, repoOwner: null as unknown as string, repoName: null as unknown as string }; + const meta = getPrToastMeta(event, "my-lane"); + expect(meta.some((item) => item.includes("/"))).toBe(false); + }); + + it("deduplicates identical items", () => { + // If lane name equals branch label, Set dedup should prevent double entry + const event = { ...baseEvent, headBranch: "my-lane", baseBranch: null as unknown as string }; + const meta = getPrToastMeta(event, "my-lane"); + const unique = [...new Set(meta)]; + expect(meta).toEqual(unique); + }); + + it("returns empty array when all fields are empty", () => { + const event = { + ...baseEvent, + repoOwner: null as unknown as string, + repoName: null as unknown as string, + headBranch: null as unknown as string, + baseBranch: null as unknown as string, + }; + const meta = getPrToastMeta(event, null); + expect(meta).toEqual([]); }); }); diff --git a/apps/desktop/src/renderer/components/lanes/AttachLaneDialog.tsx b/apps/desktop/src/renderer/components/lanes/AttachLaneDialog.tsx index ccb9d0c9..40f25d1e 100644 --- a/apps/desktop/src/renderer/components/lanes/AttachLaneDialog.tsx +++ b/apps/desktop/src/renderer/components/lanes/AttachLaneDialog.tsx @@ -1,6 +1,7 @@ -import * as Dialog from "@radix-ui/react-dialog"; import { Link, WarningCircle } from "@phosphor-icons/react"; import { Button } from "../ui/Button"; +import { LaneDialogShell } from "./LaneDialogShell"; +import { SECTION_CLASS_NAME, LABEL_CLASS_NAME, INPUT_CLASS_NAME } from "./laneDialogTokens"; export function AttachLaneDialog({ open, @@ -24,70 +25,80 @@ export function AttachLaneDialog({ onSubmit: () => void; }) { return ( - - - - -
- - - Attach Existing Worktree - - + +
+
+
+ ADE will keep the existing files where they are and start tracking the worktree as a lane in this project.
-

- Link an existing git worktree into ADE without moving files. The path must be the root of a worktree from this repository. -

-
-
- - setAttachName(e.target.value)} - placeholder="e.g. bugfix/from-other-worktree" - className="h-10 w-full rounded border border-border/20 bg-surface-recessed shadow-card px-3 text-sm outline-none placeholder:text-muted-fg" - autoFocus - disabled={busy} - /> -
-
- - setAttachPath(e.target.value)} - placeholder="/absolute/path/to/existing/worktree" - className="h-10 w-full rounded border border-border/20 bg-surface-recessed px-3 font-mono text-xs outline-none placeholder:text-muted-fg" - disabled={busy} - /> -

- Example: /Users/you/repo-worktrees/feature-auth -

-
-
- {error ? ( -
- - {error} -
- ) : null} -
-
+ +
+ +
+ +
+ +
+ Example: /Users/you/repo-worktrees/feature-auth
- - - +
+ + {error ? ( +
+ + {error} +
+ ) : null} + +
+ + +
+
+ ); } diff --git a/apps/desktop/src/renderer/components/lanes/CreateLaneDialog.tsx b/apps/desktop/src/renderer/components/lanes/CreateLaneDialog.tsx index b6193d91..7ecf1471 100644 --- a/apps/desktop/src/renderer/components/lanes/CreateLaneDialog.tsx +++ b/apps/desktop/src/renderer/components/lanes/CreateLaneDialog.tsx @@ -1,8 +1,10 @@ -import * as Dialog from "@radix-ui/react-dialog"; +import { GitBranch, Plus, StackSimple } from "@phosphor-icons/react"; import { Button } from "../ui/Button"; import type { LaneSummary, LaneEnvInitProgress, LaneTemplate } from "../../../shared/types"; import type { LaneBranchOption } from "./laneUtils"; import { LaneEnvInitProgressPanel } from "./LaneEnvInitProgress"; +import { LaneDialogShell } from "./LaneDialogShell"; +import { SECTION_CLASS_NAME, LABEL_CLASS_NAME, INPUT_CLASS_NAME, SELECT_CLASS_NAME } from "./laneDialogTokens"; function buttonLabel(busy: boolean | undefined, createAsChild: boolean, parentLaneId: string, baseBranch: string): string { if (busy) return "Setting up lane..."; @@ -53,138 +55,207 @@ export function CreateLaneDialog({ setSelectedTemplateId: (id: string) => void; onNavigateToTemplates?: () => void; }) { + const localBranches = createBranches.filter((branch) => !branch.isRemote); + const selectedTemplate = templates.find((template) => template.id === selectedTemplateId) ?? null; + return ( - - - - -
- Create lane - -
-
+ +
+
+ +
+ +
+
-
Name
- setCreateLaneName(e.target.value)} - placeholder="e.g. feature/auth-refresh" - className="mt-1 h-10 w-full rounded border border-border/15 bg-surface-recessed shadow-card px-3 text-sm outline-none placeholder:text-muted-fg" - autoFocus - disabled={busy} - /> -
-
-
-
Template
- {onNavigateToTemplates && ( - - )} +
Template
+
+ Optional automation for dependency install, file copy, and lane setup.
- {templates.length > 0 ? ( - - ) : ( -
- - No templates yet — templates copy folders, install deps, and configure lanes automatically. - -
- )}
-
+ {templates.length > 0 ? ( + <> + setCreateParentLaneId(event.target.value)} - className="h-10 w-full rounded border border-border/15 bg-surface-recessed shadow-card px-3 text-sm outline-none" + > + + {templates.map((template) => ( + + ))} + +
+ {selectedTemplate?.description ?? "Create a lane with the default environment setup."} +
+ + ) : ( +
+ No templates yet. Create one to copy folders, install dependencies, and configure lanes automatically. +
+ )} +
+ +
+
+ + {createAsChild ? : } + +
+
Starting point
+
+ Choose whether the new lane starts from primary or from another lane in the stack. +
+
+
- ) : ( -
-
Base branch on primary
- -
- Lane will be created from primary/{createBaseBranch || "..."} -
+
Child lane
+
Stack the lane under an existing lane and inherit its branch.
+
- )} -
- {error && ( -
- {error} + + {createAsChild ? ( + + ) : ( + + )}
- )} -
- -
- {envInitProgress && } - - - +
+ + {error ? ( +
+ {error} +
+ ) : null} + +
+ + +
+ + {envInitProgress ? : null} +
+ ); } diff --git a/apps/desktop/src/renderer/components/lanes/LaneDialogShell.tsx b/apps/desktop/src/renderer/components/lanes/LaneDialogShell.tsx new file mode 100644 index 00000000..0aaf3122 --- /dev/null +++ b/apps/desktop/src/renderer/components/lanes/LaneDialogShell.tsx @@ -0,0 +1,59 @@ +import * as Dialog from "@radix-ui/react-dialog"; +import type { ComponentType, ReactNode } from "react"; +import { Button } from "../ui/Button"; + +export function LaneDialogShell({ + open, + onOpenChange, + title, + description, + icon: Icon, + widthClassName, + busy = false, + children, +}: { + open: boolean; + onOpenChange: (open: boolean) => void; + title: string; + description?: string; + icon?: ComponentType<{ size?: number | string; className?: string }>; + widthClassName?: string; + busy?: boolean; + children: ReactNode; +}) { + return ( + { if (!busy || next) onOpenChange(next); }}> + + + +
+
+
+ + {Icon ? ( + + + + ) : null} + {title} + + {description ? ( + + {description} + + ) : null} +
+ + + +
+ {children} + + + + ); +} diff --git a/apps/desktop/src/renderer/components/lanes/LaneWorkPane.tsx b/apps/desktop/src/renderer/components/lanes/LaneWorkPane.tsx index 57022fdd..cbc06659 100644 --- a/apps/desktop/src/renderer/components/lanes/LaneWorkPane.tsx +++ b/apps/desktop/src/renderer/components/lanes/LaneWorkPane.tsx @@ -1,8 +1,8 @@ import { ChatCircleText, Command, Terminal } from "@phosphor-icons/react"; import type { WorkDraftKind } from "../../state/appStore"; -import { WorkViewArea } from "../terminals/WorkViewArea"; import { EmptyState } from "../ui/EmptyState"; import { SANS_FONT } from "./laneDesignTokens"; +import { WorkViewArea } from "../terminals/WorkViewArea"; import { useLaneWorkSessions } from "./useLaneWorkSessions"; const ENTRY_OPTIONS: Array<{ @@ -11,9 +11,9 @@ const ENTRY_OPTIONS: Array<{ icon: typeof ChatCircleText; color: string; }> = [ - { kind: "chat", label: "New chat", icon: ChatCircleText, color: "#8B5CF6" }, - { kind: "cli", label: "CLI tool", icon: Command, color: "#F97316" }, - { kind: "shell", label: "New shell", icon: Terminal, color: "#22C55E" }, + { kind: "chat", label: "New Chat", icon: ChatCircleText, color: "#8B5CF6" }, + { kind: "cli", label: "CLI Tool", icon: Command, color: "#F97316" }, + { kind: "shell", label: "New Shell", icon: Terminal, color: "#22C55E" }, ]; export function LaneWorkPane({ @@ -74,7 +74,7 @@ export function LaneWorkPane({ {work.visibleSessions.length} open - {work.loading ? Refreshing... : null} + {work.loading ? Refreshing… : null}
diff --git a/apps/desktop/src/renderer/components/lanes/LanesPage.tsx b/apps/desktop/src/renderer/components/lanes/LanesPage.tsx index f14e3ca2..698466aa 100644 --- a/apps/desktop/src/renderer/components/lanes/LanesPage.tsx +++ b/apps/desktop/src/renderer/components/lanes/LanesPage.tsx @@ -1073,7 +1073,7 @@ export function LanesPage() { })(); await refreshLanes(); - navigate(`/lanes?laneId=${encodeURIComponent(lane.id)}`); + navigate(`/lanes?laneId=${encodeURIComponent(lane.id)}&focus=single`); createEnvInitLaneIdRef.current = lane.id; const envProgress = selectedTemplateId @@ -1108,7 +1108,7 @@ export function LanesPage() { setAttachName(""); setAttachPath(""); setAttachError(null); - navigate(`/lanes?laneId=${encodeURIComponent(lane.id)}`); + navigate(`/lanes?laneId=${encodeURIComponent(lane.id)}&focus=single`); } catch (err) { setAttachError(err instanceof Error ? err.message : String(err)); } finally { @@ -1388,27 +1388,26 @@ export function LanesPage() { NEW LANE {addLaneDropdownOpen ? ( -
+
) : null} diff --git a/apps/desktop/src/renderer/components/lanes/laneDialogTokens.ts b/apps/desktop/src/renderer/components/lanes/laneDialogTokens.ts new file mode 100644 index 00000000..adf0b0a3 --- /dev/null +++ b/apps/desktop/src/renderer/components/lanes/laneDialogTokens.ts @@ -0,0 +1,8 @@ +/** Shared Tailwind class-name tokens used across lane dialog components. */ + +export const SECTION_CLASS_NAME = "rounded-xl border border-white/[0.06] bg-white/[0.03] p-4 shadow-card"; +export const LABEL_CLASS_NAME = "text-[11px] font-semibold uppercase tracking-[0.14em] text-muted-fg/70"; +export const INPUT_CLASS_NAME = + "mt-2 h-11 w-full rounded-lg border border-white/[0.06] bg-white/[0.03] px-3 text-sm text-fg outline-none transition-colors placeholder:text-muted-fg/45 focus:border-accent/40"; +export const SELECT_CLASS_NAME = + "mt-2 h-11 w-full rounded-lg border border-white/[0.06] bg-white/[0.03] px-3 text-sm text-fg outline-none transition-colors focus:border-accent/40"; diff --git a/apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.ts b/apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.ts index 7ffd1f84..102370c5 100644 --- a/apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.ts +++ b/apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.ts @@ -1,10 +1,10 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import type { TerminalSessionSummary } from "../../../shared/types"; import { useAppStore, type WorkDraftKind, type WorkProjectViewState, type WorkViewMode } from "../../state/appStore"; -import { shouldRefreshSessionListForChatEvent } from "../../lib/chatSessionEvents"; import { listSessionsCached } from "../../lib/sessionListCache"; -import { isRunOwnedSession } from "../../lib/sessions"; import { sessionStatusBucket } from "../../lib/terminalAttention"; +import { shouldRefreshSessionListForChatEvent } from "../../lib/chatSessionEvents"; +import { isRunOwnedSession } from "../../lib/sessions"; const DEFAULT_LANE_WORK_STATE: WorkProjectViewState = { openItemIds: [], diff --git a/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.issueResolver.test.tsx b/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.issueResolver.test.tsx index 07d80ad5..8cf5c622 100644 --- a/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.issueResolver.test.tsx +++ b/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.issueResolver.test.tsx @@ -165,6 +165,7 @@ function renderPane(args: { onNavigate?: (path: string) => void; activity?: PrActivityEvent[]; statusOverrides?: Partial; + mergeMethod?: "merge" | "squash" | "rebase"; }) { const issueResolutionStart = vi.fn().mockResolvedValue({ sessionId: "session-1", @@ -177,6 +178,16 @@ function renderPane(args: { }); const getReviewThreads = vi.fn().mockResolvedValue(args.reviewThreads); const writeClipboardText = vi.fn().mockResolvedValue(undefined); + const land = vi.fn().mockResolvedValue({ + prId: "pr-80", + prNumber: 80, + success: true, + mergeCommitSha: "sha-merge", + branchDeleted: false, + laneArchived: false, + error: null, + }); + const onRefresh = vi.fn().mockResolvedValue(undefined); Object.assign(window, { ade: { prs: { @@ -197,6 +208,7 @@ function renderPane(args: { getReviewThreads, issueResolutionStart, issueResolutionPreviewPrompt, + land, openInGitHub: vi.fn().mockResolvedValue(undefined), }, app: { @@ -211,6 +223,8 @@ function renderPane(args: { issueResolutionPreviewPrompt, getReviewThreads, writeClipboardText, + land, + onRefresh, ...render( , ), @@ -270,6 +284,54 @@ describe("PrDetailPane issue resolver CTA", () => { }); }); + it("keeps the merge readiness checks row in a running state while failed checks are still in flight", async () => { + renderPane({ + checks: [ + makeCheck({ name: "ci / unit", conclusion: "success" }), + makeCheck({ name: "ci / e2e", conclusion: "failure" }), + makeCheck({ name: "ci / lint", status: "in_progress", conclusion: null }), + ], + reviewThreads: [], + }); + + await waitFor(() => { + expect(screen.getByText("Some checks failing")).toBeTruthy(); + expect(screen.getByText("1/3 checks passing, 1 still running")).toBeTruthy(); + expect(screen.getAllByLabelText("CI running").length).toBeGreaterThan(0); + }); + }); + + it("lets the operator attempt a bypass merge and uses the selected merge method", async () => { + const user = userEvent.setup(); + const { land, onRefresh } = renderPane({ + checks: [makeCheck()], + reviewThreads: [], + mergeMethod: "squash", + statusOverrides: { + checksStatus: "failing", + reviewStatus: "changes_requested", + isMergeable: false, + mergeConflicts: false, + }, + }); + + const mergeButton = await screen.findByRole("button", { name: /merge pull request/i }); + expect((mergeButton as HTMLButtonElement).disabled).toBe(true); + + await user.click(screen.getByRole("button", { name: /create merge commit/i })); + await user.click(screen.getByRole("checkbox", { name: /attempt merge anyway if github allows bypass rules/i })); + + const bypassButton = screen.getByRole("button", { name: /attempt merge anyway/i }); + expect((bypassButton as HTMLButtonElement).disabled).toBe(false); + + await user.click(bypassButton); + + await waitFor(() => { + expect(land).toHaveBeenCalledWith({ prId: "pr-80", method: "merge" }); + expect(onRefresh).toHaveBeenCalled(); + }); + }); + it("launches the issue resolver chat and navigates to the work session", async () => { const user = userEvent.setup(); const onNavigate = vi.fn(); diff --git a/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx b/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx index d6d51e1d..33d17275 100644 --- a/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx +++ b/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx @@ -15,7 +15,7 @@ import type { } from "../../../../shared/types"; import { getPrIssueResolutionAvailability } from "../../../../shared/prIssueResolution"; import { COLORS, MONO_FONT, SANS_FONT, LABEL_STYLE, cardStyle, inlineBadge, outlineButton, primaryButton, dangerButton } from "../../lanes/laneDesignTokens"; -import { getPrChecksBadge, getPrReviewsBadge, getPrStateBadge, InlinePrBadge } from "../shared/prVisuals"; +import { getPrChecksBadge, getPrReviewsBadge, getPrStateBadge, InlinePrBadge, PrCiRunningIndicator } from "../shared/prVisuals"; import { PrIssueResolverModal } from "../shared/PrIssueResolverModal"; import { PrLaneCleanupBanner } from "../shared/PrLaneCleanupBanner"; import { formatTimeAgo, formatTimestampFull } from "../shared/prFormatters"; @@ -287,6 +287,77 @@ function fileStatusLabel(status: string): string { return FILE_STATUS_LABELS[status] ?? "?"; } +type ChecksSummary = { + passing: number; + failing: number; + pending: number; + total: number; + allChecksPassed: boolean; + someChecksFailing: boolean; + checksRunning: boolean; +}; + +function summarizeChecks(checks: PrCheck[]): ChecksSummary { + const passing = checks.filter((check) => check.conclusion === "success").length; + const failing = checks.filter((check) => check.conclusion === "failure").length; + const pending = checks.filter((check) => check.status !== "completed").length; + return { + passing, + failing, + pending, + total: checks.length, + allChecksPassed: checks.length > 0 && failing === 0 && pending === 0, + someChecksFailing: failing > 0, + checksRunning: pending > 0, + }; +} + +function getChecksRowVisuals(summary: ChecksSummary): { color: string; title: string; description: string } { + const { passing, pending, total, allChecksPassed, someChecksFailing, checksRunning } = summary; + + if (allChecksPassed) { + return { + color: COLORS.success, + title: "All checks have passed", + description: `${passing} successful check${passing !== 1 ? "s" : ""}`, + }; + } + if (someChecksFailing) { + return { + color: COLORS.danger, + title: "Some checks failing", + description: checksRunning + ? `${passing}/${total} checks passing, ${pending} still running` + : `${passing}/${total} checks passing`, + }; + } + if (total === 0) { + return { + color: COLORS.textMuted, + title: "No checks", + description: "No status checks are required", + }; + } + return { + color: COLORS.warning, + title: "Checks in progress", + description: `${pending} check${pending !== 1 ? "s" : ""} pending`, + }; +} + +function getChecksRowIcon(summary: ChecksSummary): React.ReactNode { + if (summary.allChecksPassed) { + return ; + } + if (summary.someChecksFailing) { + return ; + } + if (summary.total === 0) { + return ; + } + return ; +} + // ---- Props ---- type PrDetailPaneProps = { pr: PrWithConflicts; @@ -425,10 +496,10 @@ export function PrDetailPane({ }; // ---- Actions ---- - const handleMerge = () => { + const handleMerge = (method: MergeMethod) => { setActionResult(null); return runAction(async () => { - const res = await window.ade.prs.land({ prId: pr.id, method: mergeMethod }); + const res = await window.ade.prs.land({ prId: pr.id, method }); setActionResult(res); await onRefresh(); }); @@ -968,10 +1039,11 @@ function CommentMenu({ url }: { url: string | null }) { } // ---- Merge readiness status row ---- -function MergeStatusRow({ color, icon, title, description, children, expandable, expanded, onToggle }: { +function MergeStatusRow({ color, icon, title, titleAccessory, description, children, expandable, expanded, onToggle }: { color: string; icon: React.ReactNode; title: string; + titleAccessory?: React.ReactNode; description: string; children?: React.ReactNode; expandable?: boolean; @@ -991,6 +1063,7 @@ function MergeStatusRow({ color, icon, title, description, children, expandable,
{title} + {titleAccessory} {expandable && ( expanded ? : )} @@ -1049,7 +1122,7 @@ type OverviewTabProps = { setReviewBody: (v: string) => void; reviewEvent: "APPROVE" | "REQUEST_CHANGES" | "COMMENT"; setReviewEvent: (v: "APPROVE" | "REQUEST_CHANGES" | "COMMENT") => void; - onMerge: () => void; + onMerge: (method: MergeMethod) => void; onAddComment: () => void; onUpdateBody: () => void; onSetLabels: (labels: string[]) => void; @@ -1069,6 +1142,16 @@ function OverviewTab(props: OverviewTabProps) { const { pr, detail, status, checks, reviews, comments, detailBusy, aiSummary, aiSummaryBusy, actionBusy, mergeMethod, activity, lanes } = props; const [checksExpanded, setChecksExpanded] = React.useState(false); const [localMergeMethod, setLocalMergeMethod] = React.useState(mergeMethod); + const [allowBlockedMerge, setAllowBlockedMerge] = React.useState(false); + + React.useEffect(() => { + setLocalMergeMethod(mergeMethod); + }, [mergeMethod]); + + // Reset bypass opt-in when the selected PR changes + React.useEffect(() => { + setAllowBlockedMerge(false); + }, [pr.id]); // Sort comments chronologically (oldest first, like GitHub) const sortedComments = React.useMemo( @@ -1081,18 +1164,36 @@ function OverviewTab(props: OverviewTabProps) { ); // Checks summary - const passing = checks.filter(c => c.conclusion === "success").length; - const failing = checks.filter(c => c.conclusion === "failure").length; - const pending = checks.filter(c => c.status !== "completed").length; - const allChecksPassed = checks.length > 0 && failing === 0 && pending === 0; - const someChecksPending = pending > 0; - const someChecksFailing = failing > 0 && !someChecksPending; + const checksSummary = summarizeChecks(checks); + const { allChecksPassed, someChecksFailing, checksRunning } = checksSummary; + const checksRowVisuals = getChecksRowVisuals(checksSummary); // Review status from pr const reviewStatus = pr.reviewStatus; // Merge readiness - const canMerge = status?.isMergeable && !status?.mergeConflicts && pr.state === "open"; + const canMerge = Boolean(status?.isMergeable) && !status?.mergeConflicts && pr.state === "open"; + const canAttemptBlockedMerge = Boolean(status) && !status?.isMergeable && !status?.mergeConflicts && pr.state === "open"; + const isBypassMerge = allowBlockedMerge && canAttemptBlockedMerge; + const mergeActionEnabled = canMerge || isBypassMerge; + const mergeActionLabel = actionBusy + ? (isBypassMerge ? "Attempting merge..." : "Merging...") + : (isBypassMerge ? "Attempt merge anyway" : "Merge pull request"); + // Derive merge button styling from the merge/bypass state in one place: + const mergeAccentColor = canMerge ? COLORS.success : isBypassMerge ? COLORS.warning : null; + const mergeActionBackground = mergeAccentColor + ? `linear-gradient(135deg, ${mergeAccentColor} 0%, ${canMerge ? "#16a34a" : "#d97706"} 100%)` + : COLORS.recessedBg; + const mergeActionBorderColor = mergeAccentColor ?? COLORS.border; + const mergeActionShadow = mergeAccentColor + ? `0 2px 16px ${mergeAccentColor}${canMerge ? "40" : "35"}, 0 0 0 1px ${mergeAccentColor}${canMerge ? "30" : "25"}` + : "none"; + + React.useEffect(() => { + if (!canAttemptBlockedMerge) { + setAllowBlockedMerge(false); + } + }, [canAttemptBlockedMerge]); return (
@@ -1370,27 +1471,11 @@ function OverviewTab(props: OverviewTabProps) { {/* Checks status */} - : someChecksFailing - ? - : - } - title={ - allChecksPassed ? "All checks have passed" - : someChecksPending ? "Checks in progress" - : someChecksFailing ? "Some checks failing" - : checks.length === 0 ? "No checks" : "Checks in progress" - } - description={ - allChecksPassed ? `${passing} successful check${passing !== 1 ? "s" : ""}` - : someChecksPending && failing > 0 ? `${pending} pending, ${failing} failing` - : someChecksPending ? `${pending} check${pending !== 1 ? "s" : ""} pending` - : someChecksFailing ? `${passing}/${checks.length} checks passing` - : checks.length === 0 ? "No status checks are required" : `${pending} check${pending !== 1 ? "s" : ""} pending` - } + color={checksRowVisuals.color} + icon={getChecksRowIcon(checksSummary)} + title={checksRowVisuals.title} + titleAccessory={checksRunning && checksSummary.total > 0 ? : undefined} + description={checksRowVisuals.description} expandable={checks.length > 0} expanded={checksExpanded} onToggle={() => setChecksExpanded(!checksExpanded)} @@ -1441,14 +1526,20 @@ function OverviewTab(props: OverviewTabProps) { description={ status?.isMergeable && !status?.mergeConflicts ? "This branch has no conflicts with the base branch" : status?.mergeConflicts ? "This branch has conflicts that must be resolved" - : status && !status.isMergeable ? "Required conditions have not been met" + : status && !status.isMergeable ? "Required conditions have not been met. If GitHub offers bypass rules for your account, you can still attempt the merge below." : "Waiting for merge status check" } /> {/* Merge action area */} {(pr.state === "open" || pr.state === "draft") && ( -
+
{/* Merge method selector */}
{(["squash", "merge", "rebase"] as const).map((m) => { @@ -1474,28 +1565,60 @@ function OverviewTab(props: OverviewTabProps) { })}
+ {canAttemptBlockedMerge && ( + + )} +
{pr.state === "open" && ( diff --git a/apps/desktop/src/renderer/components/prs/shared/prVisuals.test.ts b/apps/desktop/src/renderer/components/prs/shared/prVisuals.test.ts index 89a0bd4d..7a74510b 100644 --- a/apps/desktop/src/renderer/components/prs/shared/prVisuals.test.ts +++ b/apps/desktop/src/renderer/components/prs/shared/prVisuals.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { derivePrActivityState, getPrEdgeColor } from "./prVisuals"; +import { derivePrActivityState, formatCompactCount, getPrCiDotColor, getPrEdgeColor, getPrReviewDotColor } from "./prVisuals"; describe("prVisuals", () => { afterEach(() => { @@ -55,4 +55,56 @@ describe("prVisuals", () => { expect(getPrEdgeColor({ state: "draft", checksStatus: "none", reviewStatus: "none" })).toBe("#A78BFA"); expect(getPrEdgeColor({ state: "open", checksStatus: "passing", reviewStatus: "changes_requested" })).toBe("#EF4444"); }); + + it("returns info color for getPrEdgeColor when ciRunning flag is explicitly set", () => { + // ciRunning should take priority over other statuses (except merged, draft, changes_requested) + expect(getPrEdgeColor({ state: "open", checksStatus: "passing", reviewStatus: "approved", ciRunning: true })).toBe("#3B82F6"); + expect(getPrEdgeColor({ state: "open", checksStatus: "failing", reviewStatus: "approved", ciRunning: true })).toBe("#3B82F6"); + // But merged still wins + expect(getPrEdgeColor({ state: "merged", checksStatus: "pending", reviewStatus: "approved", ciRunning: true })).toBe("#22C55E"); + // And changes_requested still wins + expect(getPrEdgeColor({ state: "open", checksStatus: "passing", reviewStatus: "changes_requested", ciRunning: true })).toBe("#EF4444"); + }); + + describe("getPrCiDotColor", () => { + it("returns info color when ciRunning is true regardless of checksStatus", () => { + expect(getPrCiDotColor({ checksStatus: "passing", ciRunning: true })).toBe("#3B82F6"); + expect(getPrCiDotColor({ checksStatus: "failing", ciRunning: true })).toBe("#3B82F6"); + expect(getPrCiDotColor({ checksStatus: "none", ciRunning: true })).toBe("#3B82F6"); + }); + + it("returns info color for pending checksStatus even without ciRunning flag", () => { + expect(getPrCiDotColor({ checksStatus: "pending" })).toBe("#3B82F6"); + }); + + it("returns danger for failing, success for passing, and muted for none", () => { + expect(getPrCiDotColor({ checksStatus: "failing" })).toBe("#EF4444"); + expect(getPrCiDotColor({ checksStatus: "passing" })).toBe("#22C55E"); + expect(getPrCiDotColor({ checksStatus: "none" })).toBe("#8B8B9A"); + }); + }); + + describe("getPrReviewDotColor", () => { + it("returns the correct color for each review status", () => { + expect(getPrReviewDotColor({ reviewStatus: "changes_requested" })).toBe("#EF4444"); + expect(getPrReviewDotColor({ reviewStatus: "approved" })).toBe("#22C55E"); + expect(getPrReviewDotColor({ reviewStatus: "requested" })).toBe("#F59E0B"); + expect(getPrReviewDotColor({ reviewStatus: "none" })).toBe("#8B8B9A"); + }); + }); + + describe("formatCompactCount", () => { + it("returns the number as a string for values under 1000", () => { + expect(formatCompactCount(0)).toBe("0"); + expect(formatCompactCount(42)).toBe("42"); + expect(formatCompactCount(999)).toBe("999"); + }); + + it("returns a compact 'k' suffix for values at or above 1000", () => { + expect(formatCompactCount(1000)).toBe("1k"); + expect(formatCompactCount(1500)).toBe("1.5k"); + expect(formatCompactCount(2345)).toBe("2.3k"); + expect(formatCompactCount(10000)).toBe("10k"); + }); + }); }); diff --git a/apps/desktop/src/renderer/components/prs/shared/prVisuals.tsx b/apps/desktop/src/renderer/components/prs/shared/prVisuals.tsx index a6958b48..b57fe818 100644 --- a/apps/desktop/src/renderer/components/prs/shared/prVisuals.tsx +++ b/apps/desktop/src/renderer/components/prs/shared/prVisuals.tsx @@ -1,6 +1,7 @@ import React from "react"; +import { CircleNotch } from "@phosphor-icons/react"; import type { PrChecksStatus, PrReviewStatus, PrState } from "../../../../shared/types"; -import { COLORS, inlineBadge } from "../../lanes/laneDesignTokens"; +import { COLORS, SANS_FONT, inlineBadge } from "../../lanes/laneDesignTokens"; export type PrActivityState = "active" | "idle" | "stale"; @@ -92,3 +93,54 @@ export function InlinePrBadge(props: { label: string; color: string; bg: string; return {label}; } +export function PrCiRunningIndicator(props: { + showLabel?: boolean; + label?: string; + color?: string; + size?: number; + title?: string; +}) { + const { + showLabel = false, + label = "running", + color = COLORS.warning, + size = 10, + title = "CI checks are still running", + } = props; + + if (!showLabel) { + return ( + + + + ); + } + + return ( + + + {label} + + ); +} diff --git a/apps/desktop/src/renderer/components/prs/state/PrsContext.tsx b/apps/desktop/src/renderer/components/prs/state/PrsContext.tsx index 2039be39..56b16b26 100644 --- a/apps/desktop/src/renderer/components/prs/state/PrsContext.tsx +++ b/apps/desktop/src/renderer/components/prs/state/PrsContext.tsx @@ -196,6 +196,10 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { // Rebase state const [rebaseNeeds, setRebaseNeeds] = useState([]); const [autoRebaseStatuses, setAutoRebaseStatuses] = useState([]); + const rebaseNeedsRef = React.useRef([]); + const autoRebaseStatusesRef = React.useRef([]); + React.useEffect(() => { rebaseNeedsRef.current = rebaseNeeds; }, [rebaseNeeds]); + React.useEffect(() => { autoRebaseStatusesRef.current = autoRebaseStatuses; }, [autoRebaseStatuses]); // Queue state const [queueStates, setQueueStates] = useState>({}); @@ -248,13 +252,13 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { // Concurrency guard for refresh const refreshInFlight = React.useRef(false); const prsRef = React.useRef([]); - prsRef.current = prs; const mergeContextByPrIdRef = React.useRef>({}); - mergeContextByPrIdRef.current = mergeContextByPrId; + React.useEffect(() => { prsRef.current = prs; }, [prs]); + React.useEffect(() => { mergeContextByPrIdRef.current = mergeContextByPrId; }, [mergeContextByPrId]); // Refs for detail polling const selectedPrIdRef = React.useRef(null); - selectedPrIdRef.current = selectedPrId; + React.useEffect(() => { selectedPrIdRef.current = selectedPrId; }, [selectedPrId]); const detailFetchInProgress = React.useRef(false); const refreshMergeContexts = useCallback(async (prIds: string[]) => { @@ -315,12 +319,20 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { try { await window.ade.prs.refresh().catch(() => {}); const shouldLoadWorkflowState = activeTab !== "normal"; - const [prList, laneList, queueStateList] = await Promise.all([ + const [prList, laneList, queueStateList, refreshedRebaseNeeds, refreshedAutoRebaseStatuses] = await Promise.all([ window.ade.prs.listWithConflicts(), window.ade.lanes.list({ includeStatus: true }), shouldLoadWorkflowState ? window.ade.prs.listQueueStates({ includeCompleted: true, limit: 50 }) : Promise.resolve([] as QueueLandingState[]), + window.ade.rebase.scanNeeds().catch((err) => { + console.warn("[PrsContext] Failed to refresh rebase needs:", err); + return rebaseNeedsRef.current; + }), + window.ade.lanes.listAutoRebaseStatuses().catch((err) => { + console.warn("[PrsContext] Failed to refresh auto-rebase statuses:", err); + return autoRebaseStatusesRef.current; + }), ]); const changedPrIds = diffPrIds(prsRef.current, prList); @@ -328,6 +340,8 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { // to avoid unnecessary re-render cascades in child components. setPrs((prev) => (jsonEqual(prev, prList) ? prev : prList)); setLanes((prev) => (jsonEqual(prev, laneList) ? prev : laneList)); + setRebaseNeeds((prev) => (jsonEqual(prev, refreshedRebaseNeeds) ? prev : refreshedRebaseNeeds)); + setAutoRebaseStatuses((prev) => (jsonEqual(prev, refreshedAutoRebaseStatuses) ? prev : refreshedAutoRebaseStatuses)); setQueueStates((prev) => { const next = Object.fromEntries(queueStateList.map((state) => [state.groupId, state] as const)); return jsonEqual(prev, next) ? prev : next; @@ -360,13 +374,10 @@ export function PrsProvider({ children }: { children: React.ReactNode }) { void refreshQueueStates([...affectedQueueGroupIds]); } - // Refresh rebase needs and auto-rebase statuses alongside the main data - window.ade.rebase.scanNeeds().then((needs) => { - setRebaseNeeds((prev) => (jsonEqual(prev, needs) ? prev : needs)); - }).catch(() => {}); - window.ade.lanes.listAutoRebaseStatuses().then((statuses) => { - setAutoRebaseStatuses((prev) => (jsonEqual(prev, statuses) ? prev : statuses)); - }).catch(() => {}); + // NOTE: Rebase needs and auto-rebase statuses are already fetched in the + // Promise.all batch above (refreshedRebaseNeeds / refreshedAutoRebaseStatuses) + // and applied via setRebaseNeeds / setAutoRebaseStatuses, so no additional + // fire-and-forget fetch is needed here. } catch (err) { setError(err instanceof Error ? err.message : String(err)); } finally { diff --git a/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.test.tsx b/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.test.tsx index 033349b7..ae0dbc13 100644 --- a/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.test.tsx +++ b/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.test.tsx @@ -98,9 +98,9 @@ describe("GitHubTab", () => { beforeEach(() => { mockUsePrs.mockReturnValue({ prs: [ - { id: "pr-open" }, - { id: "pr-merged" }, - { id: "pr-queue" }, + { id: "pr-open", checksStatus: "pending", reviewStatus: "requested", additions: 12, deletions: 3 }, + { id: "pr-merged", checksStatus: "passing", reviewStatus: "approved", additions: 5, deletions: 1 }, + { id: "pr-queue", checksStatus: "passing", reviewStatus: "approved", additions: 7, deletions: 2 }, ] satisfies Partial[], mergeContextByPrId: { "pr-queue": { groupType: "queue", groupId: "queue-group-1", members: [] }, @@ -180,4 +180,12 @@ describe("GitHubTab", () => { expect(screen.getByTestId("queue-context").textContent).toContain("queue-group-1"); }); }); + + it("shows a running CI indicator for PR cards with pending checks", async () => { + renderTab(); + + await waitFor(() => { + expect(screen.getAllByLabelText("CI running").length).toBeGreaterThan(0); + }); + }); }); diff --git a/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.tsx b/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.tsx index 8d18fa05..c4bcb46e 100644 --- a/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.tsx +++ b/apps/desktop/src/renderer/components/prs/tabs/GitHubTab.tsx @@ -6,6 +6,7 @@ import { EmptyState } from "../../ui/EmptyState"; import { COLORS, LABEL_STYLE, MONO_FONT, SANS_FONT, cardStyle, inlineBadge, outlineButton, primaryButton } from "../../lanes/laneDesignTokens"; import { PrDetailPane } from "../detail/PrDetailPane"; import { formatTimestampShort, formatTimeAgoCompact } from "../shared/prFormatters"; +import { PrCiRunningIndicator } from "../shared/prVisuals"; import { usePrs } from "../state/PrsContext"; type GitHubTabProps = { @@ -761,6 +762,7 @@ export function GitHubTab({ lanes, mergeMethod, selectedPrId, onSelectPr, onRefr const sc = stateColor(item.state); const linkedPr = item.linkedPrId ? prsByIdMap.get(item.linkedPrId) ?? null : null; const ci = ciDotColor(linkedPr); + const ciRunning = linkedPr?.checksStatus === "pending"; const review = reviewIndicator(linkedPr); const ago = formatTimeAgoCompact(item.updatedAt); return ( @@ -856,9 +858,10 @@ export function GitHubTab({ lanes, mergeMethod, selectedPrId, onSelectPr, onRefr )} {ci ? ( - + CI + {ciRunning ? : null} ) : null} {review ? ( diff --git a/apps/desktop/src/renderer/components/prs/tabs/NormalTab.test.tsx b/apps/desktop/src/renderer/components/prs/tabs/NormalTab.test.tsx new file mode 100644 index 00000000..d68c03a7 --- /dev/null +++ b/apps/desktop/src/renderer/components/prs/tabs/NormalTab.test.tsx @@ -0,0 +1,188 @@ +// @vitest-environment jsdom + +import React from "react"; +import { MemoryRouter } from "react-router-dom"; +import { cleanup, render, screen, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { LaneSummary, PrMergeContext, PrWithConflicts } from "../../../../shared/types"; + +const mockUsePrs = vi.fn(); + +vi.mock("../state/PrsContext", () => ({ + usePrs: () => mockUsePrs(), +})); + +vi.mock("../detail/PrDetailPane", () => ({ + PrDetailPane: ({ pr }: { pr: { id: string } }) => ( +
{pr.id}
+ ), +})); + +vi.mock("../shared/IntegrationPrContextPanel", () => ({ + IntegrationPrContextPanel: () => null, +})); + +import { NormalTab } from "./NormalTab"; + +function makePr(overrides: Partial = {}): PrWithConflicts { + return { + id: "pr-1", + laneId: "lane-1", + projectId: "proj-1", + repoOwner: "ade-dev", + repoName: "ade", + githubPrNumber: 101, + githubUrl: "https://github.com/ade-dev/ade/pull/101", + githubNodeId: "PR_101", + title: "Test PR", + state: "open", + baseBranch: "main", + headBranch: "feature/test", + checksStatus: "passing", + reviewStatus: "approved", + additions: 10, + deletions: 3, + lastSyncedAt: "2026-03-24T00:00:00.000Z", + createdAt: "2026-03-24T00:00:00.000Z", + updatedAt: "2026-03-24T00:00:00.000Z", + conflictAnalysis: null, + ...overrides, + }; +} + +function makeLane(overrides: Partial = {}): LaneSummary { + return { + id: "lane-1", + name: "feature/test", + description: "Test lane", + laneType: "worktree", + baseRef: "main", + branchRef: "feature/test", + worktreePath: "/tmp/lane-1", + attachedRootPath: null, + parentLaneId: null, + childCount: 0, + stackDepth: 0, + parentStatus: null, + isEditProtected: false, + status: { dirty: false, ahead: 0, behind: 0, remoteBehind: -1, rebaseInProgress: false }, + color: null, + icon: null, + tags: [], + folder: null, + createdAt: "2026-03-24T00:00:00.000Z", + archivedAt: null, + ...overrides, + }; +} + +describe("NormalTab", () => { + beforeEach(() => { + mockUsePrs.mockReturnValue({ + detailStatus: null, + detailChecks: [], + detailReviews: [], + detailComments: [], + detailBusy: false, + setActiveTab: vi.fn(), + setSelectedRebaseItemId: vi.fn(), + }); + + Object.assign(window, { + ade: { + prs: { + getDetail: vi.fn().mockResolvedValue({ + prId: "pr-1", + body: "", + labels: [], + assignees: [], + requestedReviewers: [], + author: { login: "octocat", avatarUrl: null }, + isDraft: false, + milestone: null, + linkedIssues: [], + }), + getFiles: vi.fn().mockResolvedValue([]), + getActionRuns: vi.fn().mockResolvedValue([]), + getActivity: vi.fn().mockResolvedValue([]), + getReviewThreads: vi.fn().mockResolvedValue([]), + land: vi.fn(), + openInGitHub: vi.fn(), + }, + app: { + openExternal: vi.fn(), + }, + }, + }); + }); + + afterEach(() => { + cleanup(); + }); + + function renderTab(overrides: { + prs?: PrWithConflicts[]; + lanes?: LaneSummary[]; + mergeContextByPrId?: Record; + selectedPrId?: string | null; + } = {}) { + const onSelectPr = vi.fn(); + render( + + + , + ); + return { onSelectPr }; + } + + it("shows the CI running indicator for PRs with pending checks", async () => { + renderTab({ + prs: [makePr({ checksStatus: "pending" })], + }); + + await waitFor(() => { + expect(screen.getAllByLabelText("CI running").length).toBeGreaterThan(0); + }); + }); + + it("does not show the CI running indicator for PRs with passing checks", async () => { + renderTab({ + prs: [makePr({ checksStatus: "passing" })], + }); + + await waitFor(() => { + expect(screen.getByText("Test PR")).toBeTruthy(); + }); + + expect(screen.queryByLabelText("CI running")).toBeNull(); + }); + + it("shows the CI running indicator only for PRs that have pending checks when mixed", async () => { + renderTab({ + prs: [ + makePr({ id: "pr-1", checksStatus: "passing", title: "Passing PR", githubPrNumber: 101 }), + makePr({ id: "pr-2", laneId: "lane-2", checksStatus: "pending", title: "Pending PR", githubPrNumber: 102 }), + ], + lanes: [ + makeLane(), + makeLane({ id: "lane-2", name: "feature/pending" }), + ], + }); + + await waitFor(() => { + expect(screen.getByText("Passing PR")).toBeTruthy(); + expect(screen.getByText("Pending PR")).toBeTruthy(); + }); + + // Exactly one CI running indicator for the pending PR + expect(screen.getAllByLabelText("CI running")).toHaveLength(1); + }); +}); diff --git a/apps/desktop/src/renderer/components/prs/tabs/NormalTab.tsx b/apps/desktop/src/renderer/components/prs/tabs/NormalTab.tsx index 395066cd..87937523 100644 --- a/apps/desktop/src/renderer/components/prs/tabs/NormalTab.tsx +++ b/apps/desktop/src/renderer/components/prs/tabs/NormalTab.tsx @@ -12,7 +12,7 @@ import { PrDetailPane } from "../detail/PrDetailPane"; import { usePrs } from "../state/PrsContext"; import { IntegrationPrContextPanel } from "../shared/IntegrationPrContextPanel"; import { COLORS, MONO_FONT, LABEL_STYLE, outlineButton } from "../../lanes/laneDesignTokens"; -import { getPrChecksBadge, getPrReviewsBadge, getPrStateBadge, InlinePrBadge } from "../shared/prVisuals"; +import { getPrChecksBadge, getPrReviewsBadge, getPrStateBadge, InlinePrBadge, PrCiRunningIndicator } from "../shared/prVisuals"; import { formatTimeAgoCompact } from "../shared/prFormatters"; function statusDotColor(state: string): string { @@ -259,7 +259,10 @@ export function NormalTab({ prs, lanes, mergeContextByPrId, mergeMethod, selecte
- + + + {pr.checksStatus === "pending" ? : null} + {/* Diff stats mini */} diff --git a/apps/desktop/src/renderer/components/prs/tabs/queueWorkflowModel.test.ts b/apps/desktop/src/renderer/components/prs/tabs/queueWorkflowModel.test.ts index f24305f8..d8610108 100644 --- a/apps/desktop/src/renderer/components/prs/tabs/queueWorkflowModel.test.ts +++ b/apps/desktop/src/renderer/components/prs/tabs/queueWorkflowModel.test.ts @@ -85,7 +85,7 @@ describe("queueWorkflowModel", () => { } satisfies PrStatus, memberSummary: null, }), - ).toEqual(["GitHub has not marked the current PR as mergeable yet."]); + ).toEqual(["GitHub has not marked the current PR as mergeable yet. Manual land can still succeed if GitHub allows a bypass merge."]); }); it("advises the operator to rebase the next lane after a successful land", () => { diff --git a/apps/desktop/src/renderer/components/prs/tabs/queueWorkflowModel.ts b/apps/desktop/src/renderer/components/prs/tabs/queueWorkflowModel.ts index 83401480..e47cd184 100644 --- a/apps/desktop/src/renderer/components/prs/tabs/queueWorkflowModel.ts +++ b/apps/desktop/src/renderer/components/prs/tabs/queueWorkflowModel.ts @@ -93,7 +93,7 @@ export function buildManualLandWarnings(args: { if (reviewStatus === "changes_requested") warnings.push("The current PR has requested changes."); if (args.status?.mergeConflicts) warnings.push("GitHub reports merge conflicts on the current PR."); if (args.status && !args.status.isMergeable && !args.status.mergeConflicts) { - warnings.push("GitHub has not marked the current PR as mergeable yet."); + warnings.push("GitHub has not marked the current PR as mergeable yet. Manual land can still succeed if GitHub allows a bypass merge."); } return warnings; diff --git a/apps/desktop/src/renderer/state/appStore.ts b/apps/desktop/src/renderer/state/appStore.ts index 8e544906..16cd6649 100644 --- a/apps/desktop/src/renderer/state/appStore.ts +++ b/apps/desktop/src/renderer/state/appStore.ts @@ -276,7 +276,9 @@ export const useAppStore = create((set, get) => ({ continue; } const laneId = scopeKey.slice(projectKey.length + 2); - if (allowed.has(laneId)) nextLaneWorkViews[scopeKey] = viewState; + if (allowed.has(laneId)) { + nextLaneWorkViews[scopeKey] = viewState; + } } return { lanes, diff --git a/apps/desktop/src/shared/types/prs.ts b/apps/desktop/src/shared/types/prs.ts index 6aa0fd7f..829bec17 100644 --- a/apps/desktop/src/shared/types/prs.ts +++ b/apps/desktop/src/shared/types/prs.ts @@ -151,16 +151,16 @@ export type PrEventPayload = prId: string; prNumber: number; title: string; + prTitle: string; + repoOwner: string; + repoName: string; + baseBranch: string; + headBranch: string; githubUrl: string; message: string; state: PrState; checksStatus: PrChecksStatus; reviewStatus: PrReviewStatus; - prTitle: string | null; - repoOwner: string | null; - repoName: string | null; - headBranch: string | null; - baseBranch: string | null; } | { type: "queue-step"; diff --git a/docs/architecture/SYSTEM_OVERVIEW.md b/docs/architecture/SYSTEM_OVERVIEW.md index bf3e7246..c6dae636 100644 --- a/docs/architecture/SYSTEM_OVERVIEW.md +++ b/docs/architecture/SYSTEM_OVERVIEW.md @@ -222,7 +222,7 @@ Missions remain ADE's structured multi-worker execution system, but the mission ### PRs and GitHub -The PR system still supports local simulation, stacked workflows, and integration proposals, but GitHub snapshot loading is now cached and integration simulation is manually triggered instead of auto-running on tab entry. PR issue resolution is available: agents can be launched from the PR detail surface to fix failing CI checks and address unresolved review threads, with dedicated workflow tools for refreshing issue state, rerunning checks, and managing review threads. PR polling now runs on a 60-second default interval (up from 25s) and uses fingerprint-based change detection to avoid cascading re-renders when nothing changed. Rebase suggestions are queue-aware: the conflict service fetches queue target tracking branches and resolves rebase overrides so that queued PRs rebase against the correct comparison ref rather than always using the lane's base branch. +The PR system still supports local simulation, stacked workflows, and integration proposals, but GitHub snapshot loading is now cached and integration simulation is manually triggered instead of auto-running on tab entry. PR issue resolution is available: agents can be launched from the PR detail surface to fix failing CI checks and address unresolved review threads, with dedicated workflow tools for refreshing issue state, rerunning checks, and managing review threads. PR polling now runs on a 60-second default interval (up from 25s) and uses fingerprint-based change detection to avoid cascading re-renders when nothing changed. Rebase suggestions are queue-aware: the conflict service fetches queue target tracking branches and resolves rebase overrides so that queued PRs rebase against the correct comparison ref rather than always using the lane's base branch. The PR detail pane supports merge bypass for blocked PRs (opt-in checkbox when branch protection prevents merge but the user may have bypass permissions), a CI running indicator for in-progress checks, and PR notification toasts with tone-aware styling and structured metadata. ### Workspace Graph diff --git a/docs/features/LANES.md b/docs/features/LANES.md index 365fd33e..9e490920 100644 --- a/docs/features/LANES.md +++ b/docs/features/LANES.md @@ -2,7 +2,7 @@ > Roadmap reference: `docs/final-plan/README.md` is the canonical future plan and sequencing source. -> Last updated: 2026-03-08 +> Last updated: 2026-03-24 --- @@ -132,7 +132,7 @@ The left pane is a scrollable list of all active lanes for the current project. **Stack graph** (bottom of left pane): A lightweight visual representation of parent-child lane relationships (including clear connections back to the Primary lane). A one-click “Open canvas” action jumps to the full Workspace Graph for deeper exploration. -**Create Lane button**: Opens a dialog to create a new lane. The user provides a name and optionally selects a parent lane (for stacking) and a base ref. +**Create Lane button**: Opens a dropdown menu offering "Create new lane" or "Add existing worktree as lane". Both options open a dialog built on the shared `LaneDialogShell` component. The create dialog lets the user choose a name, optional template, and starting point (from primary with a base branch selector, or as a child of an existing lane). The attach dialog accepts a lane name and an absolute worktree path. ### Center Pane — Lane Detail Area @@ -264,7 +264,7 @@ The inspector is a collapsible sidebar on the right edge of the Lanes tab. It pr | Service | Responsibility | |---------|---------------| -| `laneService` | CRUD operations for lanes. Creates/removes worktrees via git. Computes lane status by aggregating dirty state, ahead/behind, and other signals. Manages lane metadata in the database. Supports primary, worktree, and attached lane types. Provides rebase (recursive rebase), reparent, stack chain, and appearance management. | +| `laneService` | CRUD operations for lanes. Creates/removes worktrees via git. Computes lane status by aggregating dirty state, ahead/behind, and other signals. Manages lane metadata in the database. Supports primary, worktree, and attached lane types. Provides rebase (recursive rebase with remote tracking branch resolution for primary parents, dirty-worktree guard), reparent, stack chain, and appearance management. | | `rebaseSuggestionService` | Monitors stacked lanes for parent-advanced state. Generates rebase suggestions with dismiss/defer lifecycle. Emits real-time suggestion events to the renderer. | | `gitService` | All git operations: stage, unstage, discard, commit, stash, fetch, sync (merge/rebase), push, conflict state detection (merge/rebase in-progress, continue, abort). Operates on a specified worktree path. Returns structured results with success/failure and output. | | `diffService` | Computes working tree diffs (unstaged changes) and index diffs (staged changes). Per-file diff content for the Monaco viewer. Handles binary file detection and large file truncation. | @@ -297,7 +297,7 @@ The inspector is a collapsible sidebar on the right edge of the Lanes tab. It pr | Channel | Signature | Description | |---------|-----------|-------------| -| `ade.lanes.rebaseStart` | `(args: RebaseStartArgs) => RebaseStartResult` | Rebase a lane (rebase onto parent), optionally recursive | +| `ade.lanes.rebaseStart` | `(args: RebaseStartArgs) => RebaseStartResult` | Rebase a lane onto its parent. For primary parents, resolves the remote tracking branch (upstream or `origin/`) as the rebase target before falling back to the local HEAD. Refuses to rebase dirty worktrees. Optionally recursive. | | `ade.lanes.listRebaseSuggestions` | `() => RebaseSuggestion[]` | List lanes whose parent has advanced (rebase recommended) | | `ade.lanes.dismissRebaseSuggestion` | `(args: { laneId: string }) => void` | Dismiss a rebase suggestion for the current parent HEAD | | `ade.lanes.deferRebaseSuggestion` | `(args: { laneId: string; minutes: number }) => void` | Defer a rebase suggestion for N minutes | diff --git a/docs/features/PULL_REQUESTS.md b/docs/features/PULL_REQUESTS.md index 1dfad6c1..a3b2c087 100644 --- a/docs/features/PULL_REQUESTS.md +++ b/docs/features/PULL_REQUESTS.md @@ -98,6 +98,26 @@ That is the main reason the normal PR and GitHub views feel less heavy than befo --- +## Merge bypass for blocked PRs + +When GitHub reports a PR as not mergeable (e.g. due to branch protection rules), ADE now offers an explicit opt-in to attempt the merge anyway. This covers cases where the user's account has permission to bypass branch protection requirements even though the GitHub API reports `isMergeable: false`. + +The UI surfaces a checkbox in the merge action area when the PR is open, has no merge conflicts, but is flagged as not mergeable. Checking it enables the merge button in a warning state. The merge request still goes through the normal GitHub merge API — GitHub itself decides whether the bypass is allowed. + +--- + +## CI running indicator + +PR list rows (Normal tab, GitHub tab) and the PR detail merge readiness panel now show a spinning indicator when CI checks are still running. The `PrCiRunningIndicator` component lives in `prVisuals.tsx` and supports both icon-only and labeled variants. The detail pane's check status row also uses it as a title accessory. + +--- + +## PR notifications + +Notification messages emitted by the polling service are now generic (not PR-specific) to work better as system notifications. The title no longer includes the PR number. The event payload now includes `prTitle`, `repoOwner`, `repoName`, `baseBranch`, and `headBranch` fields so consumers can format context-aware messages themselves. + +--- + ## Current product contract The current PR experience follows these rules: @@ -107,6 +127,7 @@ The current PR experience follows these rules: - load workflow state only when the user is actually in a workflow view - never auto-run expensive integration simulation just from tab selection - make GitHub revisits warm through layered caching +- include rebase needs and auto-rebase statuses in the main refresh batch That preserves the full PR feature set while keeping the common browse-and-review path much cheaper. @@ -146,6 +167,8 @@ The PR service exposes review thread data through a dedicated GraphQL-backed `ge Rebase suggestions for queued PRs are now queue-aware. The conflict service calls `fetchQueueTargetTrackingBranches()` before scanning rebase needs, then uses `resolveQueueRebaseOverride()` per lane to determine the correct comparison ref. When a lane belongs to an active merge queue, the rebase targets the queue's tracking branch rather than the lane's static base branch. Queue group context is propagated into the rebase need for display in the rebase UI. AI-assisted rebase (`rebaseLane`) also respects the queue override, and the rebase request now accepts `modelId`, `reasoningEffort`, and `permissionMode` parameters for finer control over the AI rebase agent. +For non-queued stacked lanes, the conflict service now uses `resolveLaneRebaseTarget()` to determine the comparison ref. When the parent lane is a primary lane, the comparison ref resolves to `origin/` (the remote tracking branch) rather than the local HEAD. This keeps conflict prediction and rebase suggestions consistent with the lane service's own rebase behavior, which targets the remote tracking branch for primary parents. + --- ## Queue workflow model