diff --git a/apps/ade-cli/src/adeRpcServer.test.ts b/apps/ade-cli/src/adeRpcServer.test.ts index 99f1a0012..f3307a4b6 100644 --- a/apps/ade-cli/src/adeRpcServer.test.ts +++ b/apps/ade-cli/src/adeRpcServer.test.ts @@ -3261,6 +3261,87 @@ describe("adeRpcServer", () => { expect(fixture.runtime.conflictService.getLaneStatus).toHaveBeenCalledWith({ laneId: "lane-1" }); }); + it("projects projectless Linear lane issues and link rows instead of dropping them", async () => { + const fixture = createRuntime(); + const projectlessIssue = { + id: "issue-69", + identifier: "ADE-69", + title: "Projectless issue linked to a lane", + description: null, + url: "https://linear.app/ade-linear/issue/ADE-69/projectless", + projectId: "", + projectSlug: "", + projectName: null, + teamId: "team-ade", + teamKey: "ADE", + teamName: "ADE", + stateId: "state-backlog", + stateName: "Backlog", + stateType: "backlog", + priority: 2, + priorityLabel: "high", + labels: ["bug"], + assigneeId: null, + assigneeName: null, + creatorId: null, + creatorName: null, + dueDate: null, + estimate: null, + branchName: "ade-69-projectless", + createdAt: "2026-05-31T08:32:17.115Z", + updatedAt: "2026-05-31T08:32:17.115Z", + }; + (fixture.runtime.laneService.list as any).mockResolvedValueOnce([ + { + id: "lane-1", + name: "ADE-69 Projectless issue linked to a lane", + laneType: "worktree", + parentLaneId: null, + baseRef: "main", + branchRef: "ade-69-projectless", + worktreePath: "/tmp/project/.ade/worktrees/ade-69", + archivedAt: null, + stackDepth: 0, + linearIssue: projectlessIssue, + linearIssueLinks: [ + { + id: "link-69", + laneId: "lane-1", + issue: projectlessIssue, + role: "primary", + source: "lane_create", + includeInPr: true, + closeOnMerge: false, + evidence: null, + createdAt: "2026-05-31T08:32:17.115Z", + updatedAt: "2026-05-31T08:32:17.115Z", + }, + ], + status: { dirty: false, ahead: 0, behind: 0 }, + }, + ]); + const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" }); + + await initialize(handler); + const response = await callTool(handler, "get_lane_status", { laneId: "lane-1" }); + + expect(response?.isError).toBeUndefined(); + expect(response.structuredContent.lane.linearIssue).toMatchObject({ + id: "issue-69", + identifier: "ADE-69", + projectId: "", + projectSlug: "", + }); + expect(response.structuredContent.lane.linearIssueLinks).toEqual([ + expect.objectContaining({ + id: "link-69", + role: "primary", + source: "lane_create", + issue: expect.objectContaining({ identifier: "ADE-69", projectId: "" }), + }), + ]); + }); + it("routes check_conflicts with a single laneId", async () => { const fixture = createRuntime(); const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" }); @@ -3349,6 +3430,54 @@ describe("adeRpcServer", () => { expect((fixture.runtime.laneService.create as any).mock.calls[0][0].linearIssue).not.toHaveProperty("secretToken"); }); + it("accepts projectless Linear issue data when creating a linked lane", async () => { + const fixture = createRuntime(); + const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" }); + + await initialize(handler, { callerId: "orchestrator", role: "orchestrator" }); + const response = await callTool(handler, "create_lane", { + name: "projectless-linear-lane", + linearIssue: { + id: "issue-projectless", + identifier: "ADE-69", + title: "Projectless Linear issue", + description: null, + url: null, + projectId: "", + projectSlug: "", + projectName: null, + teamId: "team-ade", + teamKey: "ADE", + teamName: "ADE", + stateId: "state-backlog", + stateName: "Backlog", + stateType: "backlog", + priority: 2, + priorityLabel: "high", + labels: [], + assigneeId: null, + assigneeName: null, + creatorId: null, + creatorName: null, + dueDate: null, + estimate: null, + createdAt: "2026-05-31T08:32:17.115Z", + updatedAt: "2026-05-31T08:32:17.115Z", + }, + }); + + expect(response?.isError).toBeUndefined(); + expect(fixture.runtime.laneService.create).toHaveBeenCalledWith( + expect.objectContaining({ + linearIssue: expect.objectContaining({ + identifier: "ADE-69", + projectId: "", + projectSlug: "", + }), + }), + ); + }); + it("routes simulate_integration as a read-only dry-merge", async () => { const fixture = createRuntime(); const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" }); diff --git a/apps/ade-cli/src/adeRpcServer.ts b/apps/ade-cli/src/adeRpcServer.ts index 75876857d..0d05042e9 100644 --- a/apps/ade-cli/src/adeRpcServer.ts +++ b/apps/ade-cli/src/adeRpcServer.ts @@ -38,6 +38,7 @@ import { type ComputerUseBackendStyle, type ComputerUseArtifactOwner, type LaneLinearIssue, + type LaneLinearIssueLink, type MergeMethod, type AppNavigationRequest, } from "../../desktop/src/shared/types"; @@ -1934,8 +1935,8 @@ function parseLaneLinearIssue(value: unknown, field = "linearIssue"): LaneLinear title: assertNonEmptyString(issue.title, `${field}.title`), description: assertOptionalStringOrNull(issue.description, `${field}.description`), url: assertOptionalStringOrNull(issue.url, `${field}.url`), - projectId: assertNonEmptyString(issue.projectId, `${field}.projectId`), - projectSlug: assertNonEmptyString(issue.projectSlug, `${field}.projectSlug`), + projectId: asTrimmedString(issue.projectId), + projectSlug: asTrimmedString(issue.projectSlug), projectName: assertOptionalStringOrNull(issue.projectName, `${field}.projectName`), teamId: assertNonEmptyString(issue.teamId, `${field}.teamId`), teamKey: assertNonEmptyString(issue.teamKey, `${field}.teamKey`), @@ -1967,6 +1968,46 @@ function projectLaneLinearIssue(value: unknown): LaneLinearIssue | null { } } +function projectLaneLinearIssueLink(value: unknown): LaneLinearIssueLink | null { + const link = safeObject(value); + if (Object.keys(link).length === 0) return null; + const issue = projectLaneLinearIssue(link.issue); + if (!issue) return null; + const role = asOptionalTrimmedString(link.role); + const source = asOptionalTrimmedString(link.source); + const laneId = asOptionalTrimmedString(link.laneId); + const evidenceRecord = safeObject(link.evidence); + const evidence = Object.keys(evidenceRecord).length + ? { + chatSessionId: asOptionalTrimmedString(evidenceRecord.chatSessionId), + commitSha: asOptionalTrimmedString(evidenceRecord.commitSha), + prId: asOptionalTrimmedString(evidenceRecord.prId), + } + : null; + return { + id: asOptionalTrimmedString(link.id) ?? "", + laneId: laneId ?? "", + issue, + role: role === "primary" || role === "worked" || role === "referenced" || role === "inferred" + ? role + : "worked", + source: source === "lane_create" + || source === "lane_link" + || source === "chat_attach" + || source === "linear_open_issue" + || source === "commit" + || source === "pr_body" + || source === "manual" + ? source + : "manual", + includeInPr: link.includeInPr !== false, + closeOnMerge: link.closeOnMerge === true, + evidence, + createdAt: asOptionalTrimmedString(link.createdAt) ?? "", + updatedAt: asOptionalTrimmedString(link.updatedAt) ?? "", + }; +} + function assertNonEmptyString(value: unknown, field: string): string { const text = asTrimmedString(value); if (!text.length) { @@ -2786,6 +2827,11 @@ function resolveSpawnContextFile(args: { } function mapLaneSummary(lane: Record): Record { + const linearIssueLinks = Array.isArray(lane.linearIssueLinks) + ? lane.linearIssueLinks + .map(projectLaneLinearIssueLink) + .filter((link): link is LaneLinearIssueLink => Boolean(link)) + : []; return { id: lane.id, name: lane.name, @@ -2797,6 +2843,7 @@ function mapLaneSummary(lane: Record): Record archivedAt: lane.archivedAt, stackDepth: lane.stackDepth, linearIssue: projectLaneLinearIssue(lane.linearIssue), + linearIssueLinks, status: lane.status }; } diff --git a/apps/ade-cli/src/cli.test.ts b/apps/ade-cli/src/cli.test.ts index def3cdcfe..40647b84c 100644 --- a/apps/ade-cli/src/cli.test.ts +++ b/apps/ade-cli/src/cli.test.ts @@ -11,6 +11,7 @@ import { graphWaitState, isEphemeralRuntimeSocketPath, isFailedServiceManagerResult, + machineRuntimeMismatchReason, parseCliArgs, readRuntimeIdleExitMs, renderLaneGraph, @@ -32,6 +33,7 @@ function baseResolveOpts(): Omit< role: "external", headless: true, requireSocket: false, + socketPath: null, pretty: false, text: false, timeoutMs: 15_000, @@ -75,6 +77,38 @@ describe("ADE CLI", () => { } }); + it("parses socket mode with an optional socket path override", () => { + const spaced = parseCliArgs([ + "--socket", + "/tmp/ade-runtime.sock", + "--project-root", + "/tmp/project", + "linear", + "graphql", + "--query", + "query { viewer { id } }", + ]); + expect(spaced.options.requireSocket).toBe(true); + expect(spaced.options.headless).toBe(false); + expect(spaced.options.socketPath).toBe("/tmp/ade-runtime.sock"); + expect(spaced.command.slice(0, 2)).toEqual(["linear", "graphql"]); + + const joined = parseCliArgs([ + "--socket=tcp://127.0.0.1:8787", + "linear", + "issue", + "ADE-69", + ]); + expect(joined.options.requireSocket).toBe(true); + expect(joined.options.socketPath).toBe("tcp://127.0.0.1:8787"); + expect(joined.command).toEqual(["linear", "issue", "ADE-69"]); + + const flagOnly = parseCliArgs(["--socket", "linear", "issue", "ADE-69"]); + expect(flagOnly.options.requireSocket).toBe(true); + expect(flagOnly.options.socketPath).toBeNull(); + expect(flagOnly.command).toEqual(["linear", "issue", "ADE-69"]); + }); + it("maps ade code to the terminal Work chat launcher", () => { const parsed = parseCliArgs([ "--project-root", @@ -171,6 +205,26 @@ describe("ADE CLI", () => { expect(readRuntimeIdleExitMs({ ADE_RUNTIME_IDLE_EXIT_MS: "nope" } as NodeJS.ProcessEnv)).toBeNull(); }); + it("allows explicit runtime socket overrides across build hashes", () => { + const runtimeInfo = { + version: "0.0.0", + buildHash: "other-build", + defaultRole: "agent", + packageChannel: null, + projectRoot: null, + pid: 123, + }; + + expect( + machineRuntimeMismatchReason(runtimeInfo, "expected-build", "agent"), + ).toBe("build hash changed"); + expect( + machineRuntimeMismatchReason(runtimeInfo, "expected-build", "agent", { + enforceBuildCompatibility: false, + }), + ).toBeNull(); + }); + it("marks failed service manager results as CLI failures", () => { expect( isFailedServiceManagerResult({ @@ -980,6 +1034,7 @@ describe("ADE CLI", () => { role: "agent", headless: false, requireSocket: false, + socketPath: null, pretty: true, text: true, timeoutMs: 1000, @@ -1060,6 +1115,48 @@ describe("ADE CLI", () => { expect(graph).toContain("\\- sibling (id: sibling) [feature-2]"); }); + it("renders linked Linear issue identifiers in lane graph and detail text", () => { + const lane = { + id: "lane-linear", + name: "ADE-69 Fix linked lane", + branchRef: "ade-69-fix-linked-lane", + linearIssue: { + id: "issue-69", + identifier: "ADE-69", + title: "Fix linked lane", + }, + linearIssueLinks: [ + { + id: "link-70", + issue: { id: "issue-70", identifier: "ADE-70", title: "Follow-up" }, + }, + ], + }; + + expect(renderLaneGraph({ lanes: [lane] })).toContain( + "\\- ADE-69 Fix linked lane (id: lane-linear) [ade-69-fix-linked-lane] {ADE-69, ADE-70}", + ); + + const detail = formatOutput( + { lane }, + { + projectRoot: null, + workspaceRoot: null, + role: "agent", + headless: false, + requireSocket: false, + socketPath: null, + pretty: false, + text: true, + timeoutMs: 1000, + }, + "lane-detail", + ); + expect(detail).toContain("linear issue"); + expect(detail).toContain("ADE-69: Fix linked lane"); + expect(detail).toContain("ADE-70: Follow-up"); + }); + it("accepts --option=value syntax equivalently to --option value", () => { const spaced = parseCliArgs([ "--project-root", @@ -2539,7 +2636,7 @@ describe("ADE CLI", () => { ).toThrow(/missing both "id" and "identifier"/); }); - it("maps Linear quick view to the typed RPC tool", () => { + it("maps Linear quick view through the runtime-owned issue tracker action", () => { const plan = buildCliPlan(["linear", "quick-view", "--text"]); expect(plan.kind).toBe("execute"); @@ -2547,24 +2644,32 @@ describe("ADE CLI", () => { expect(plan.label).toBe("Linear quick view"); expect(plan.formatter).toBe("linear-quick-view"); expect(plan.steps[0]?.params).toEqual({ - name: "getLinearQuickView", - arguments: {}, + name: "run_ade_action", + arguments: { + domain: "linear_issue_tracker", + action: "getQuickView", + args: {}, + }, }); }); - it("maps Linear picker data to the typed RPC tool", () => { + it("maps Linear picker data through the runtime-owned issue tracker action", () => { const plan = buildCliPlan(["linear", "picker-data", "--text"]); expect(plan.kind).toBe("execute"); if (plan.kind !== "execute") return; expect(plan.label).toBe("Linear picker data"); expect(plan.steps[0]?.params).toEqual({ - name: "getLinearIssuePickerData", - arguments: {}, + name: "run_ade_action", + arguments: { + domain: "linear_issue_tracker", + action: "getIssuePickerData", + args: {}, + }, }); }); - it("maps Linear search-issues filters to the typed RPC tool", () => { + it("maps Linear search-issues filters through the runtime-owned issue tracker action", () => { const plan = buildCliPlan([ "linear", "search-issues", @@ -2583,13 +2688,32 @@ describe("ADE CLI", () => { if (plan.kind !== "execute") return; expect(plan.label).toBe("Linear search issues"); expect(plan.steps[0]?.params).toEqual({ - name: "searchLinearIssues", + name: "run_ade_action", arguments: { - projectId: "proj-1", - stateTypes: ["started", "unstarted"], - query: "auth", - first: 25, - includeArchived: true, + domain: "linear_issue_tracker", + action: "searchIssues", + args: { + projectId: "proj-1", + stateTypes: ["started", "unstarted"], + query: "auth", + first: 25, + includeArchived: true, + }, + }, + }); + }); + + it("maps Linear issue comments to the scalar issue tracker action", () => { + const plan = buildCliPlan(["linear", "issue-comments", "ADE-69"]); + + expect(plan.kind).toBe("execute"); + if (plan.kind !== "execute") return; + expect(plan.steps[0]?.params).toEqual({ + name: "run_ade_action", + arguments: { + domain: "linear_issue_tracker", + action: "fetchIssueComments", + arg: "ADE-69", }, }); }); diff --git a/apps/ade-cli/src/cli.ts b/apps/ade-cli/src/cli.ts index cc8e5884a..a7d021d7b 100644 --- a/apps/ade-cli/src/cli.ts +++ b/apps/ade-cli/src/cli.ts @@ -59,6 +59,7 @@ type GlobalOptions = { role: "cto" | "orchestrator" | "agent" | "external" | "evaluator"; headless: boolean; requireSocket: boolean; + socketPath: string | null; pretty: boolean; text: boolean; timeoutMs: number; @@ -2335,6 +2336,7 @@ function parseCliArgs(argv: string[]): ParsedCli { role: resolveAdeDefaultRole(process.env.ADE_DEFAULT_ROLE, "agent"), headless: parseBooleanEnv(process.env.ADE_CLI_HEADLESS), requireSocket: false, + socketPath: null, pretty: true, text: false, timeoutMs: 10 * 60 * 1000, @@ -2394,6 +2396,17 @@ function parseCliArgs(argv: string[]): ParsedCli { if (inGlobalPrefix && token === "--socket") { options.requireSocket = true; options.headless = false; + const maybeSocketPath = argv[index + 1] ?? ""; + if (looksLikeSocketPathOverride(maybeSocketPath)) { + options.socketPath = maybeSocketPath; + index += 1; + } + continue; + } + if (inGlobalPrefix && token.startsWith("--socket=")) { + options.requireSocket = true; + options.headless = false; + options.socketPath = requireValue(token.slice("--socket=".length), "--socket"); continue; } if (token === "--compact") { @@ -2445,6 +2458,19 @@ function parseCliArgs(argv: string[]): ParsedCli { return { options, command }; } +function looksLikeSocketPathOverride(value: string): boolean { + if (!value || value.startsWith("-")) return false; + return ( + value.startsWith("tcp://") || + value.startsWith("/") || + value.startsWith("./") || + value.startsWith("../") || + value.startsWith("~") || + isAdeRuntimeNamedPipePath(value) || + value.endsWith(".sock") + ); +} + function parseRole(value: string): GlobalOptions["role"] { const role = normalizeAdeRuntimeRole(value); if (role) return role; @@ -8855,9 +8881,10 @@ function buildLinearPlan(args: string[]): CliPlan { label: "Linear quick view", formatter: "linear-quick-view", steps: [ - actionCallStep( + actionStep( "result", - "getLinearQuickView", + "linear_issue_tracker", + "getQuickView", collectGenericObjectArgs(args), ), ], @@ -8868,9 +8895,10 @@ function buildLinearPlan(args: string[]): CliPlan { kind: "execute", label: "Linear picker data", steps: [ - actionCallStep( + actionStep( "result", - "getLinearIssuePickerData", + "linear_issue_tracker", + "getIssuePickerData", collectGenericObjectArgs(args), ), ], @@ -8913,9 +8941,10 @@ function buildLinearPlan(args: string[]): CliPlan { kind: "execute", label: "Linear search issues", steps: [ - actionCallStep( + actionStep( "result", - "searchLinearIssues", + "linear_issue_tracker", + "searchIssues", collectGenericObjectArgs(args, input), ), ], @@ -8928,10 +8957,11 @@ function buildLinearPlan(args: string[]): CliPlan { kind: "execute", label: "Linear issue comments", steps: [ - actionCallStep( + actionScalarStep( "result", - "getLinearIssueComments", - collectGenericObjectArgs(args, { issueId }), + "linear_issue_tracker", + "fetchIssueComments", + issueId, ), ], }; @@ -9750,8 +9780,9 @@ function commandExists(command: string): boolean { return result.status === 0 && result.stdout.trim().length > 0; } -function resolveAdeCodeSocketPath(projectRoot: string): string { +function resolveAdeCodeSocketPath(projectRoot: string, socketPathOverride?: string | null): string { return ( + socketPathOverride?.trim() || process.env.ADE_RPC_URL?.trim() || process.env.ADE_RPC_SOCKET_PATH?.trim() || process.env.ADE_RUNTIME_SOCKET_PATH?.trim() || @@ -9771,7 +9802,7 @@ function buildAdeCodeArgs(rest: string[], options: GlobalOptions): string[] { ...(options.requireSocket ? [ "--socket", - resolveAdeCodeSocketPath(roots.projectRoot), + resolveAdeCodeSocketPath(roots.projectRoot, options.socketPath), "--require-socket", ] : []), @@ -10981,7 +11012,9 @@ async function createConnection( const { resolveAdeLayout } = await import("../../desktop/src/shared/adeLayout"); const layout = resolveAdeLayout(roots.projectRoot); + const socketPathOverride = options.socketPath?.trim() || null; const legacySocketPath = + socketPathOverride || process.env.ADE_RPC_URL?.trim() || process.env.ADE_RPC_SOCKET_PATH?.trim() || layout.socketPath; @@ -10990,8 +11023,8 @@ async function createConnection( if (!options.headless) { let socketClient: SocketJsonRpcClient | null = null; try { - const machineSocketPath = await resolveMachineRuntimeSocketPath(); - socketClient = await connectMachineRuntimeDaemon(options); + const machineSocketPath = await resolveMachineRuntimeSocketPath(socketPathOverride); + socketClient = await connectMachineRuntimeDaemon(options, socketPathOverride); let activeProjectId: string | null = null; const connection: CliConnection = { mode: "runtime-socket", @@ -11290,29 +11323,34 @@ function machineRuntimeMismatchReason( runtimeInfo: MachineRuntimeInfo, expectedBuildHash: string | null, expectedDefaultRole: GlobalOptions["role"], + options: { enforceBuildCompatibility?: boolean } = {}, ): string | null { - const runtimeVersion = runtimeInfo.version; - const sourceCliTalkingToReleasedRuntime = - VERSION === PLACEHOLDER_VERSION && - Boolean(runtimeVersion) && - runtimeVersion !== PLACEHOLDER_VERSION; - if (VERSION !== PLACEHOLDER_VERSION) { - const versionMatches = runtimeVersion === VERSION; - const placeholderBuildMatches = - runtimeVersion === PLACEHOLDER_VERSION && - expectedBuildHash != null && - runtimeInfo.buildHash === expectedBuildHash; - if (!versionMatches && !placeholderBuildMatches) { - return `version ${runtimeVersion ?? "missing"} does not match CLI version ${VERSION}`; + const enforceBuildCompatibility = + options.enforceBuildCompatibility ?? true; + if (enforceBuildCompatibility) { + const runtimeVersion = runtimeInfo.version; + const sourceCliTalkingToReleasedRuntime = + VERSION === PLACEHOLDER_VERSION && + Boolean(runtimeVersion) && + runtimeVersion !== PLACEHOLDER_VERSION; + if (VERSION !== PLACEHOLDER_VERSION) { + const versionMatches = runtimeVersion === VERSION; + const placeholderBuildMatches = + runtimeVersion === PLACEHOLDER_VERSION && + expectedBuildHash != null && + runtimeInfo.buildHash === expectedBuildHash; + if (!versionMatches && !placeholderBuildMatches) { + return `version ${runtimeVersion ?? "missing"} does not match CLI version ${VERSION}`; + } } - } - if ( - !sourceCliTalkingToReleasedRuntime && - expectedBuildHash && - runtimeInfo.buildHash !== expectedBuildHash - ) { - return runtimeInfo.buildHash ? "build hash changed" : "build hash missing"; + if ( + !sourceCliTalkingToReleasedRuntime && + expectedBuildHash && + runtimeInfo.buildHash !== expectedBuildHash + ) { + return runtimeInfo.buildHash ? "build hash changed" : "build hash missing"; + } } if (!canRuntimeDefaultRoleServe(runtimeInfo.defaultRole, expectedDefaultRole)) { return `default role ${runtimeInfo.defaultRole ?? "missing"} cannot serve CLI role ${expectedDefaultRole}`; @@ -11431,7 +11469,9 @@ async function connectMachineRuntimeDaemon( const label = "ADE runtime daemon socket"; const allowSpawn = connectOptions.allowSpawn ?? !options.requireSocket; const isTcpSocket = socketPath.startsWith("tcp://"); - const expectedBuildHash = isTcpSocket + const hasExplicitSocketOverride = Boolean(socketPathOverride?.trim()); + const enforceBuildCompatibility = !hasExplicitSocketOverride; + const expectedBuildHash = isTcpSocket || !enforceBuildCompatibility ? null : await resolveExpectedMachineRuntimeBuildHash(); try { @@ -11448,6 +11488,7 @@ async function connectMachineRuntimeDaemon( runtimeInfo, expectedBuildHash, options.role, + { enforceBuildCompatibility }, ); if (mismatch) { if (!allowSpawn || isTcpSocket) { @@ -11476,6 +11517,7 @@ async function connectMachineRuntimeDaemon( restartedInfo, expectedBuildHash, options.role, + { enforceBuildCompatibility }, ); if (restartedMismatch) { await shutdownMachineRuntimeDaemon(restarted); @@ -11504,6 +11546,7 @@ async function connectMachineRuntimeDaemon( runtimeInfo, expectedBuildHash, options.role, + { enforceBuildCompatibility }, ); if (mismatch) { await shutdownMachineRuntimeDaemon(client); @@ -12294,8 +12337,10 @@ function renderLaneGraph(result: unknown): string { const archived = asString(lane.archivedAt) ? " archived" : ""; const id = asString(lane.id); const idSuffix = id ? ` (id: ${id})` : ""; + const linearIdentifiers = linearIssueIdentifiers(lane); + const linearSuffix = linearIdentifiers.length ? ` {${linearIdentifiers.join(", ")}}` : ""; lines.push( - `${prefix}${isLast ? "\\- " : "|- "}${name}${idSuffix}${branch ? ` [${branch}]` : ""}${status ? ` ${status}` : ""}${archived}`, + `${prefix}${isLast ? "\\- " : "|- "}${name}${idSuffix}${branch ? ` [${branch}]` : ""}${linearSuffix}${status ? ` ${status}` : ""}${archived}`, ); const children = id ? (byParent.get(id) ?? []) : []; children.forEach((child, index) => @@ -12311,6 +12356,45 @@ function renderLaneGraph(result: unknown): string { return lines.join("\n"); } +function linearIssueIdentifiers(lane: JsonObject): string[] { + const identifiers: string[] = []; + const seen = new Set(); + const add = (issue: unknown): void => { + if (!isRecord(issue)) return; + const identifier = asString(issue.identifier) ?? asString(issue.id); + if (!identifier || seen.has(identifier)) return; + seen.add(identifier); + identifiers.push(identifier); + }; + add(lane.linearIssue); + const links = Array.isArray(lane.linearIssueLinks) ? lane.linearIssueLinks : []; + for (const link of links) { + if (!isRecord(link)) continue; + add(link.issue); + } + return identifiers; +} + +function linearIssueLabels(lane: JsonObject): string[] { + const labels: string[] = []; + const seen = new Set(); + const add = (issue: unknown): void => { + if (!isRecord(issue)) return; + const identifier = asString(issue.identifier) ?? asString(issue.id); + if (!identifier || seen.has(identifier)) return; + seen.add(identifier); + const title = asString(issue.title); + labels.push(title ? `${identifier}: ${title}` : identifier); + }; + add(lane.linearIssue); + const links = Array.isArray(lane.linearIssueLinks) ? lane.linearIssueLinks : []; + for (const link of links) { + if (!isRecord(link)) continue; + add(link.issue); + } + return labels; +} + function truncateCell(value: string, width = 42): string { const normalized = value.replace(/\s+/g, " ").trim(); if (normalized.length <= width) return normalized; @@ -12562,11 +12646,14 @@ function formatActionsList(value: unknown): string { function formatLaneDetail(value: unknown): string { const root = isRecord(value) ? value : {}; const lane = firstRecord(value, ["lane"]) ?? (isRecord(value) ? value : {}); + const linearLabels = linearIssueLabels(lane); return renderKeyValues("ADE lane", [ ["id", lane.id], ["name", lane.name], ["branch", lane.branchRef ?? lane.branch], ["base", lane.baseBranch ?? lane.baseRef], + ["linear issue", linearLabels[0] ?? null], + ["linear links", linearLabels.length > 1 ? linearLabels.slice(1).join(", ") : null], ["status", lane.status ?? root.rebaseStatus], ["worktree", lane.worktreePath], ]); @@ -14143,6 +14230,16 @@ async function runCli( } } +async function writeProcessOutput( + stream: NodeJS.WriteStream, + output: string, +): Promise { + if (!output.length) return; + await new Promise((resolve) => { + stream.write(output, () => resolve()); + }); +} + async function main(): Promise { const writeDiagnostic = (...args: unknown[]) => { process.stderr.write( @@ -14154,36 +14251,46 @@ async function main(): Promise { console.warn = writeDiagnostic; try { const result = await runCli(process.argv.slice(2)); - process.stdout.write(result.output); + await writeProcessOutput(process.stdout, result.output); process.exitCode = result.exitCode; } catch (error) { const fallback = maybeRunBuiltCliFallback(error, process.argv.slice(2)); if (fallback) { - if (fallback.stderr.length) process.stderr.write(fallback.stderr); - if (fallback.stdout.length) process.stdout.write(fallback.stdout); + await writeProcessOutput(process.stderr, fallback.stderr); + await writeProcessOutput(process.stdout, fallback.stdout); process.exitCode = fallback.exitCode; return; } if (error instanceof CliUsageError) { - process.stderr.write(`ade: ${error.message}\nRun 'ade help'.\n`); + await writeProcessOutput( + process.stderr, + `ade: ${error.message}\nRun 'ade help'.\n`, + ); process.exitCode = 2; return; } if (error instanceof CliToolError) { - process.stderr.write(`ade: ${error.message}\n`); + await writeProcessOutput(process.stderr, `ade: ${error.message}\n`); if (error.details !== undefined) { - process.stderr.write(`${JSON.stringify(error.details, null, 2)}\n`); + await writeProcessOutput( + process.stderr, + `${JSON.stringify(error.details, null, 2)}\n`, + ); } process.exitCode = 1; return; } if (error instanceof CliExecutionError) { - process.stderr.write(`ade: ${error.message}\n`); - process.stderr.write(`${JSON.stringify(error.details, null, 2)}\n`); + await writeProcessOutput(process.stderr, `ade: ${error.message}\n`); + await writeProcessOutput( + process.stderr, + `${JSON.stringify(error.details, null, 2)}\n`, + ); process.exitCode = 1; return; } - process.stderr.write( + await writeProcessOutput( + process.stderr, `ade: ${error instanceof Error ? error.stack || error.message : String(error)}\n`, ); process.exitCode = 1; @@ -14191,7 +14298,9 @@ async function main(): Promise { } if (/(^|[/\\])cli\.(?:ts|js|cjs)$/.test(process.argv[1] ?? "")) { - void main(); + void main().finally(() => { + process.exit(typeof process.exitCode === "number" ? process.exitCode : 0); + }); } export { @@ -14203,6 +14312,7 @@ export { graphWaitState, isEphemeralRuntimeSocketPath, isFailedServiceManagerResult, + machineRuntimeMismatchReason, parseCliArgs, readRuntimeIdleExitMs, renderLaneGraph, diff --git a/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts b/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts index a15a6028a..5659936a6 100644 --- a/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts +++ b/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts @@ -333,7 +333,7 @@ function buildFallbackInventoryItems(args: { source, type: "review_thread", externalId: `review-thread:${thread.id}`, - state: thread.isResolved || thread.isOutdated ? "fixed" : "new", + state: thread.isResolved ? "fixed" : "new", round: 0, filePath: thread.path, line: thread.line, diff --git a/apps/desktop/src/main/services/automations/githubPollingService.test.ts b/apps/desktop/src/main/services/automations/githubPollingService.test.ts index c6d196c8a..bdd99033f 100644 --- a/apps/desktop/src/main/services/automations/githubPollingService.test.ts +++ b/apps/desktop/src/main/services/automations/githubPollingService.test.ts @@ -627,6 +627,66 @@ describe("githubPollingService — cursor format", () => { expect(githubService.listIssueComments.mock.calls[0]?.[3]?.since).toBe("2026-04-23T10:15:00Z"); }); + it("emits first-seen issue comments after downtime using the durable repo cursor", async () => { + const { service, dispatchCalls, githubService } = makeHarness({ + initialCursor: "acme/ade=2026-04-23T10:00:00Z", + issuesByCall: [[{ + number: 10, + updatedAt: "2026-04-23T10:30:00Z", + createdAt: "2026-04-22T09:00:00Z", + comments: 1, + }]], + pullsByCall: [[]], + commentsByCall: [[{ id: 101, body: "comment during downtime", createdAt: "2026-04-23T10:20:00Z" }]], + }); + + await service.pollNow(); + + expect(githubService.listIssueComments.mock.calls[0]?.[3]?.since).toBe("2026-04-23T10:00:00Z"); + expect(dispatchCalls.filter((call) => call.triggerType === "github.issue_commented")).toHaveLength(1); + }); + + it("emits first-seen PR comments after downtime using the durable repo cursor", async () => { + const { service, dispatchCalls, githubService } = makeHarness({ + initialCursor: "acme/ade=2026-04-23T10:00:00Z", + issuesByCall: [[]], + pullsByCall: [[{ + number: 42, + updatedAt: "2026-04-23T10:30:00Z", + createdAt: "2026-04-22T09:00:00Z", + comments: 1, + }]], + commentsByCall: [[{ id: 201, body: "pr comment during downtime", createdAt: "2026-04-23T10:20:00Z" }]], + }); + + await service.pollNow(); + + expect(githubService.listIssueComments.mock.calls[0]?.[3]?.since).toBe("2026-04-23T10:00:00Z"); + expect(dispatchCalls.filter((call) => call.triggerType === "github.pr_commented")).toHaveLength(1); + }); + + it("emits only first-seen PR reviews newer than the durable repo cursor", async () => { + const { service, dispatchCalls } = makeHarness({ + initialCursor: "acme/ade=2026-04-23T10:00:00Z", + issuesByCall: [[]], + pullsByCall: [[{ + number: 42, + updatedAt: "2026-04-23T10:30:00Z", + createdAt: "2026-04-22T09:00:00Z", + }]], + reviewsByCall: [[ + { id: 1, body: "old review", submittedAt: "2026-04-23T09:50:00Z" }, + { id: 2, body: "new review", submittedAt: "2026-04-23T10:20:00Z" }, + ]], + }); + + await service.pollNow(); + + const reviews = dispatchCalls.filter((call) => call.triggerType === "github.pr_review_submitted"); + expect(reviews).toHaveLength(1); + expect(reviews[0]?.rawPayload).toMatchObject({ reviewId: 2 }); + }); + it("does not skip comments that share the same created_at timestamp", async () => { const { service, dispatchCalls } = makeHarness({ issuesByCall: [ diff --git a/apps/desktop/src/main/services/automations/githubPollingService.ts b/apps/desktop/src/main/services/automations/githubPollingService.ts index e7c73c6f7..128ce3103 100644 --- a/apps/desktop/src/main/services/automations/githubPollingService.ts +++ b/apps/desktop/src/main/services/automations/githubPollingService.ts @@ -327,8 +327,8 @@ export function createGithubPollingService(args: GithubPollingServiceArgs) { summary: `Issue #${issue.number} opened: ${issue.title}`, }); } - if (since === undefined && (issue.comments ?? 0) > 0) { - await pollComments(repo, issue.number, since, ctx, /* isPr */ false, /* emit */ false); + if ((issue.comments ?? 0) > 0) { + await pollComments(repo, issue.number, since, ctx, /* isPr */ false, /* emit */ since !== undefined); } snapshotByRepo.set(issue.number, { labels: currentLabels, @@ -422,14 +422,10 @@ export function createGithubPollingService(args: GithubPollingServiceArgs) { summary: `PR #${pr.number} opened: ${pr.title}`, }); } - if (since === undefined) { - if ((pr.comments ?? 0) > 0) { - await pollComments(repo, pr.number, since, ctx, /* isPr */ true, /* emit */ false); - } - await pollReviews(repo, pr.number, ctx, /* emit */ false); - } else { - await pollReviews(repo, pr.number, ctx); + if ((pr.comments ?? 0) > 0) { + await pollComments(repo, pr.number, since, ctx, /* isPr */ true, /* emit */ since !== undefined); } + await pollReviews(repo, pr.number, ctx, /* emit */ since !== undefined, since); snapshotByRepo.set(pr.number, currentSnapshot); continue; } @@ -509,6 +505,7 @@ export function createGithubPollingService(args: GithubPollingServiceArgs) { prNumber: number, ctx: AutomationTriggerPrContext, emit = true, + since?: string, ) => { const key = `${repoSlug(repo)}:${prNumber}`; const cursor = reviewCursors.get(key); @@ -517,6 +514,7 @@ export function createGithubPollingService(args: GithubPollingServiceArgs) { const submittedAt = review.submitted_at ?? ""; if (!submittedAt) continue; if (cursor && submittedAt <= cursor) continue; + if (!cursor && since && submittedAt <= since) continue; if (emit) { await dispatch(repo, "github.pr_review_submitted", `${repoSlug(repo)}#${prNumber}:review:${review.id}`, { pr: { ...ctx, body: review.body ?? undefined }, diff --git a/apps/desktop/src/main/services/conflicts/conflictService.test.ts b/apps/desktop/src/main/services/conflicts/conflictService.test.ts index ccb45c072..e93b7f443 100644 --- a/apps/desktop/src/main/services/conflicts/conflictService.test.ts +++ b/apps/desktop/src/main/services/conflicts/conflictService.test.ts @@ -127,6 +127,7 @@ function insertPrediction(args: { laneASha: string; overlaps: string[]; status?: "clean" | "conflict" | "unknown"; + predictedAt?: string; }) { args.db.run( ` @@ -145,7 +146,7 @@ function insertPrediction(args: { JSON.stringify(args.overlaps), args.laneASha, args.laneBSha ?? null, - "2026-02-15T18:50:00.000Z", + args.predictedAt ?? "2026-02-15T18:50:00.000Z", "2026-02-15T20:00:00.000Z" ] ); @@ -193,6 +194,48 @@ async function setupAdditionalInstructionsResolver(prefix: string, projectId: st } describe("conflictService conflict context integrity", () => { + it("marks risk matrix predictions stale when the lane HEAD SHA changed", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-conflicts-stale-sha-")); + const { laneHeadSha } = seedRepoWithLaneWork(repoRoot); + const baseHeadSha = git(repoRoot, ["rev-parse", "main"]); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + const projectId = "proj-stale-sha"; + try { + await seedProjectAndLane(db, projectId, repoRoot); + insertPrediction({ + db, + projectId, + laneAId: "lane-1", + laneASha: "old-lane-head", + laneBSha: baseHeadSha, + overlaps: ["src/a.ts"], + predictedAt: new Date().toISOString(), + }); + const laneSummary = createLaneSummary(repoRoot); + const service = createConflictService({ + db, + logger: createLogger(), + projectId, + projectRoot: repoRoot, + laneService: { + list: async () => [laneSummary], + getLaneBaseAndBranch: () => ({ worktreePath: repoRoot, baseRef: "main", branchRef: "feature/lane-1" }), + } as any, + projectConfigService: { + get: () => ({ effective: { providerMode: "subscription" } }), + } as any, + }); + + const matrix = await service.getRiskMatrix(); + + expect(laneHeadSha).not.toBe("old-lane-head"); + expect(matrix.find((entry) => entry.laneAId === "lane-1" && entry.laneBId === "lane-1")?.stale).toBe(true); + } finally { + db.close(); + fs.rmSync(repoRoot, { recursive: true, force: true }); + } + }); + it("passes relevant file contexts into subscription conflict proposal jobs", async () => { const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-conflicts-ctx-")); const { laneHeadSha } = seedRepoWithLaneWork(repoRoot); diff --git a/apps/desktop/src/main/services/conflicts/conflictService.ts b/apps/desktop/src/main/services/conflicts/conflictService.ts index bdd721b42..d2e7115d8 100644 --- a/apps/desktop/src/main/services/conflicts/conflictService.ts +++ b/apps/desktop/src/main/services/conflicts/conflictService.ts @@ -1495,6 +1495,28 @@ export function createConflictService({ overlaps: BatchOverlapEntry[]; }> => { const latest = getLatestRows(); + const currentHeadShaByLaneId = new Map(); + const currentBaseShaByLaneId = new Map(); + await Promise.all(lanes.map(async (lane) => { + const [headSha, baseSha] = await Promise.all([ + readHeadSha(lane.worktreePath, "HEAD").catch(() => null), + readHeadSha(projectRoot, lane.baseRef).catch(() => null), + ]); + currentHeadShaByLaneId.set(lane.id, headSha); + currentBaseShaByLaneId.set(lane.id, baseSha); + })); + const predictionIsStale = (row: ConflictPredictionRow | undefined): boolean => { + if (!row) return true; + if (isStalePrediction(row.predicted_at)) return true; + const laneASha = currentHeadShaByLaneId.get(row.lane_a_id) ?? null; + if (!row.lane_a_sha || !laneASha || row.lane_a_sha !== laneASha) return true; + if (row.lane_b_id == null) { + const baseSha = currentBaseShaByLaneId.get(row.lane_a_id) ?? null; + return !row.lane_b_sha || !baseSha || row.lane_b_sha !== baseSha; + } + const laneBSha = currentHeadShaByLaneId.get(row.lane_b_id) ?? null; + return !row.lane_b_sha || !laneBSha || row.lane_b_sha !== laneBSha; + }; const matrix: RiskMatrixEntry[] = []; const overlapEntries: BatchOverlapEntry[] = []; @@ -1509,7 +1531,7 @@ export function createConflictService({ overlapCount: overlapFiles.length, hasConflict: (row?.status ?? "unknown") === "conflict" || conflicting.length > 0, computedAt: row?.predicted_at ?? null, - stale: isStalePrediction(row?.predicted_at) + stale: predictionIsStale(row) }); overlapEntries.push({ laneAId: lane.id, @@ -1533,7 +1555,7 @@ export function createConflictService({ overlapCount: overlapFiles.length, hasConflict: (row?.status ?? "unknown") === "conflict" || conflicting.length > 0, computedAt: row?.predicted_at ?? null, - stale: isStalePrediction(row?.predicted_at) + stale: predictionIsStale(row) }); overlapEntries.push({ laneAId: laneA.id, diff --git a/apps/desktop/src/main/services/git/git.test.ts b/apps/desktop/src/main/services/git/git.test.ts index d81bf4d1b..5c2161d3e 100644 --- a/apps/desktop/src/main/services/git/git.test.ts +++ b/apps/desktop/src/main/services/git/git.test.ts @@ -67,7 +67,7 @@ describe("runGitOrThrow", () => { const gitDir = git(worktreeRoot, ["rev-parse", "--absolute-git-dir"]); const lockPath = path.join(gitDir, "index.lock"); fs.writeFileSync(lockPath, "", "utf8"); - const staleDate = new Date(Date.now() - 60_000); + const staleDate = new Date(Date.now() - 5 * 60_000); fs.utimesSync(lockPath, staleDate, staleDate); await runGitOrThrow(["add", "-A", "--", "."], { cwd: worktreeRoot, timeoutMs: 15_000 }); @@ -110,4 +110,25 @@ describe("runGit", () => { expect(result.exitCode).toBe(0); expect(fs.existsSync(lockPath)).toBe(false); }); + + it("does not rename a recent index.lock", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-git-lock-fresh-")); + git(repoRoot, ["init", "-b", "main"]); + git(repoRoot, ["config", "user.email", "ade@test.local"]); + git(repoRoot, ["config", "user.name", "ADE Test"]); + fs.writeFileSync(path.join(repoRoot, "file.txt"), "hello\n", "utf8"); + git(repoRoot, ["add", "."]); + git(repoRoot, ["commit", "-m", "init"]); + + const gitDir = git(repoRoot, ["rev-parse", "--absolute-git-dir"]); + const lockPath = path.join(gitDir, "index.lock"); + fs.writeFileSync(lockPath, "", "utf8"); + fs.writeFileSync(path.join(repoRoot, "file.txt"), "updated\n", "utf8"); + + const result = await runGit(["add", "-A", "--", "."], { cwd: repoRoot, timeoutMs: 8_000 }); + + expect(result.exitCode).not.toBe(0); + expect(fs.existsSync(lockPath)).toBe(true); + expect(fs.readdirSync(gitDir).some((entry) => entry.startsWith("index.lock.stale-"))).toBe(false); + }); }); diff --git a/apps/desktop/src/main/services/git/git.ts b/apps/desktop/src/main/services/git/git.ts index 5a21d8e2c..23fdf8429 100644 --- a/apps/desktop/src/main/services/git/git.ts +++ b/apps/desktop/src/main/services/git/git.ts @@ -98,7 +98,7 @@ export type GitMergeTreeResult = GitRunResult & { }; const DEFAULT_MAX_OUTPUT_BYTES = 4 * 1024 * 1024; -const STALE_GIT_INDEX_LOCK_MIN_AGE_MS = 15_000; +const STALE_GIT_INDEX_LOCK_MIN_AGE_MS = 2 * 60_000; let mergeTreeMergeBaseSupportPromise: Promise | null = null; const activeGitPids = new Set(); @@ -109,16 +109,31 @@ function extractIndexLockPath(message: string): string | null { return doubleQuoteMatch?.[1] ?? null; } +function isIndexLockHeldByProcess(lockPath: string): boolean { + if (activeGitPids.size > 0) return true; + if (process.platform === "win32") return false; + try { + const out = execFileSync("lsof", [lockPath], { + encoding: "utf8", + timeout: 2_000, + stdio: ["ignore", "pipe", "ignore"], + }); + return out.trim().split(/\r?\n/).length > 1; + } catch { + return false; + } +} + function recoverStaleIndexLock(lockPath: string): boolean { try { const normalizedPath = path.normalize(lockPath); if (path.basename(normalizedPath) !== "index.lock") return false; if (!normalizedPath.includes(`${path.sep}.git${path.sep}`)) return false; - if (activeGitPids.size > 0) return false; if (!fs.existsSync(lockPath)) return false; const stat = fs.statSync(lockPath); if (!stat.isFile()) return false; if (Date.now() - stat.mtimeMs < STALE_GIT_INDEX_LOCK_MIN_AGE_MS) return false; + if (isIndexLockHeldByProcess(lockPath)) return false; fs.renameSync(lockPath, `${lockPath}.stale-${Date.now()}`); return true; } catch (error) { diff --git a/apps/desktop/src/main/services/github/githubService.test.ts b/apps/desktop/src/main/services/github/githubService.test.ts index f8bed4182..6d842b38f 100644 --- a/apps/desktop/src/main/services/github/githubService.test.ts +++ b/apps/desktop/src/main/services/github/githubService.test.ts @@ -581,6 +581,45 @@ describe("githubService issue-domain helpers", () => { expect(mockFetch.mock.calls[3]?.[0]).toContain("page=2"); }); + it("does not evict an ETag cache entry while its conditional request is in flight", async () => { + const service = makeService(); + let protectedCalls = 0; + let resolveProtected304: ((response: Response) => void) | null = null; + mockFetch.mockImplementation((rawUrl: string) => { + const url = new URL(rawUrl); + if (url.pathname === "/protected") { + protectedCalls += 1; + if (protectedCalls === 1) { + return Promise.resolve(jsonResponse(200, { value: "cached" }, { etag: '"protected"' })); + } + return new Promise((resolve) => { + resolveProtected304 = resolve; + }); + } + return Promise.resolve(jsonResponse(200, { value: url.pathname }, { etag: `"${url.pathname}"` })); + }); + + await service.apiRequest({ method: "GET", path: "/protected", token: "ghp_test123" }); + const conditional = service.apiRequest<{ value: string }>({ + method: "GET", + path: "/protected", + token: "ghp_test123", + }); + await Promise.resolve(); + + for (let i = 0; i < 205; i += 1) { + await service.apiRequest({ method: "GET", path: `/repos/acme/repo/issues/${i}`, token: "ghp_test123" }); + } + + const protected304Resolver = resolveProtected304 as ((response: Response) => void) | null; + if (!protected304Resolver) { + throw new Error("Expected protected conditional request to be in flight"); + } + protected304Resolver(jsonResponse(304, {})); + + await expect(conditional).resolves.toMatchObject({ data: { value: "cached" } }); + }); + it("URL-encodes owner/name so special characters don't break the path", async () => { mockFetch.mockResolvedValueOnce(jsonResponse(200, [])); const service = makeService(); diff --git a/apps/desktop/src/main/services/github/githubService.ts b/apps/desktop/src/main/services/github/githubService.ts index 051a65e65..813e02178 100644 --- a/apps/desktop/src/main/services/github/githubService.ts +++ b/apps/desktop/src/main/services/github/githubService.ts @@ -574,6 +574,15 @@ export function createGithubService({ // don't count against GitHub's rate limit, so this dramatically reduces API usage. const etagCache = new Map(); const ETAG_CACHE_MAX_SIZE = 200; + const inFlightConditionalGetKeys = new Set(); + + const evictOldestEtagCacheEntry = (protectedKeys: ReadonlySet): void => { + for (const key of etagCache.keys()) { + if (protectedKeys.has(key)) continue; + etagCache.delete(key); + return; + } + }; const apiRequest = async (args: { method: "GET" | "POST" | "PATCH" | "PUT" | "DELETE"; @@ -604,18 +613,28 @@ export function createGithubService({ // For GET requests, send If-None-Match with cached ETag if available. // GitHub returns 304 Not Modified for free (no rate limit cost). + let sentConditionalGet = false; if (args.method === "GET") { const cached = etagCache.get(urlKey); if (cached) { headers["if-none-match"] = cached.etag; + sentConditionalGet = true; + inFlightConditionalGetKeys.add(urlKey); } } - const response = await fetchGitHub(url.toString(), { - method: args.method, - headers, - body: args.body != null ? JSON.stringify(args.body) : undefined - }); + let response: Response; + try { + response = await fetchGitHub(url.toString(), { + method: args.method, + headers, + body: args.body != null ? JSON.stringify(args.body) : undefined + }); + } finally { + if (sentConditionalGet) { + inFlightConditionalGetKeys.delete(urlKey); + } + } // 304 Not Modified — return cached data (free, no rate limit cost) if (response.status === 304) { @@ -665,9 +684,10 @@ export function createGithubService({ const etag = response.headers.get("etag"); if (etag) { // Evict oldest entries if cache is full - if (etagCache.size >= ETAG_CACHE_MAX_SIZE) { - const firstKey = etagCache.keys().next().value; - if (firstKey) etagCache.delete(firstKey); + while (etagCache.size >= ETAG_CACHE_MAX_SIZE && !etagCache.has(urlKey)) { + const before = etagCache.size; + evictOldestEtagCacheEntry(inFlightConditionalGetKeys); + if (etagCache.size === before) break; } etagCache.set(urlKey, { etag, data, linkHeader }); } diff --git a/apps/desktop/src/main/services/lanes/laneService.test.ts b/apps/desktop/src/main/services/lanes/laneService.test.ts index 8a8343833..1ed239c1b 100644 --- a/apps/desktop/src/main/services/lanes/laneService.test.ts +++ b/apps/desktop/src/main/services/lanes/laneService.test.ts @@ -4065,6 +4065,8 @@ describe("laneService - branchSwitch", () => { "stale", "open", "main", "feature/a", 0, 0, BSW_NOW, BSW_NOW, ], ); + db.run("insert into pull_request_ai_summaries(pr_id, head_sha, summary_json, generated_at) values (?, ?, ?, ?)", ["pr-stale", "abc123", "{}", BSW_NOW]); + db.run("insert into pull_request_snapshots(pr_id, updated_at) values (?, ?)", ["pr-stale", BSW_NOW]); vi.mocked(runGitOrThrow).mockImplementation(async () => ({ exitCode: 0, stdout: "", stderr: "" } as any)); vi.mocked(runGit).mockImplementation(makeRunGitResponder((args) => { @@ -4088,6 +4090,8 @@ describe("laneService - branchSwitch", () => { ["pr-stale"], ); expect(stale).toBeNull(); + expect(db.get<{ count: number }>("select count(1) as count from pull_request_ai_summaries where pr_id = ?", ["pr-stale"])?.count).toBe(0); + expect(db.get<{ count: number }>("select count(1) as count from pull_request_snapshots where pr_id = ?", ["pr-stale"])?.count).toBe(0); } finally { db.close(); fs.rmSync(repoRoot, { recursive: true, force: true }); diff --git a/apps/desktop/src/main/services/lanes/laneService.ts b/apps/desktop/src/main/services/lanes/laneService.ts index da9ec543c..022646f4e 100644 --- a/apps/desktop/src/main/services/lanes/laneService.ts +++ b/apps/desktop/src/main/services/lanes/laneService.ts @@ -6,6 +6,7 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { randomUUID } from "node:crypto"; import type { AdeDb } from "../state/kvDb"; import { getHeadSha, runGit, runGitOrThrow } from "../git/git"; +import { deletePullRequestRowsByIds, deletePullRequestRowsForLane } from "../prs/pullRequestRowCleanup"; import { isWithinDir, normalizeBranchName } from "../shared/utils"; import { fetchRemoteTrackingBranch, resolveQueueRebaseOverride, type QueueRebaseOverride } from "../shared/queueRebase"; import { detectConflictKind } from "../git/gitConflictState"; @@ -2561,14 +2562,7 @@ export function createLaneService({ db.run("update integration_proposals set integration_lane_id = null where integration_lane_id = ? and project_id = ?", [laneId, projectId]); db.run("update integration_proposals set preferred_integration_lane_id = null where preferred_integration_lane_id = ? and project_id = ?", [laneId, projectId]); - db.run("delete from pr_group_members where lane_id = ?", [laneId]); - db.run("delete from pr_group_members where pr_id in (select id from pull_requests where lane_id = ? and project_id = ?)", [laneId, projectId]); - db.run("delete from pull_request_ai_summaries where pr_id in (select id from pull_requests where lane_id = ? and project_id = ?)", [laneId, projectId]); - db.run("delete from pull_request_snapshots where pr_id in (select id from pull_requests where lane_id = ? and project_id = ?)", [laneId, projectId]); - db.run("delete from pr_convergence_state where pr_id in (select id from pull_requests where lane_id = ? and project_id = ?)", [laneId, projectId]); - db.run("delete from pr_pipeline_settings where pr_id in (select id from pull_requests where lane_id = ? and project_id = ?)", [laneId, projectId]); - db.run("delete from pr_issue_inventory where pr_id in (select id from pull_requests where lane_id = ? and project_id = ?)", [laneId, projectId]); - db.run("delete from pull_requests where lane_id = ? and project_id = ?", [laneId, projectId]); + deletePullRequestRowsForLane(db, projectId, laneId); db.run("delete from pr_auto_link_ignores where lane_id = ? and project_id = ?", [laneId, projectId]); db.run("delete from review_run_publications where run_id in (select id from review_runs where lane_id = ? and project_id = ?)", [laneId, projectId]); @@ -3708,21 +3702,8 @@ export function createLaneService({ [row.id, projectId, targetBranchRef], ); if (stalePrRows.length > 0) { - const placeholders = stalePrRows.map(() => "?").join(", "); const stalePrIds = stalePrRows.map((r) => r.id); - db.run(`delete from pr_convergence_state where pr_id in (${placeholders})`, stalePrIds); - db.run(`delete from pr_pipeline_settings where pr_id in (${placeholders})`, stalePrIds); - db.run(`delete from pr_issue_inventory where pr_id in (${placeholders})`, stalePrIds); - db.run(`delete from pr_group_members where pr_id in (${placeholders})`, stalePrIds); - db.run( - ` - delete from pull_requests - where lane_id = ? - and project_id = ? - and head_branch <> ? - `, - [row.id, projectId, targetBranchRef], - ); + deletePullRequestRowsByIds(db, projectId, stalePrIds); } db.run("commit"); } catch (err) { diff --git a/apps/desktop/src/main/services/prs/issueInventoryService.ts b/apps/desktop/src/main/services/prs/issueInventoryService.ts index 30c1eefc0..36d274f77 100644 --- a/apps/desktop/src/main/services/prs/issueInventoryService.ts +++ b/apps/desktop/src/main/services/prs/issueInventoryService.ts @@ -986,7 +986,7 @@ export function createIssueInventoryService(deps: { db: AdeDb }) { const latestReplyLooksResolved = (source === "human" || source === "unknown") && looksLikeResolutionAck(body); - if (thread.isResolved || thread.isOutdated || latestReplyLooksResolved) { + if (thread.isResolved || latestReplyLooksResolved) { if (!existing) continue; upsertItem(prId, externalId, threadData, { state: "fixed", @@ -996,6 +996,9 @@ export function createIssueInventoryService(deps: { db: AdeDb }) { }); continue; } + if (thread.isOutdated && !existing) { + continue; + } const latestCommentAt = latestComment?.updatedAt ?? latestComment?.createdAt ?? null; const hasStoredThreadMetadata = existing != null diff --git a/apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts b/apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts index 8f4eae3ff..dc608fc17 100644 --- a/apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts +++ b/apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts @@ -1365,22 +1365,22 @@ export function createPathToMergeOrchestrator(deps: PathToMergeDeps): PathToMerg // round-trip when the PR is already at base — otherwise every wake fires // a redundant `git fetch` + `git push --force-with-lease` that no-ops on // the remote but pollutes the audit log and wastes round-trips. - let behindBaseBy = 0; + let behindBaseBy: number | null = null; try { const status = await prService.getStatus(prId); - behindBaseBy = status.behindBaseBy ?? 0; + behindBaseBy = status.behindBaseBy ?? null; } catch (err) { logger.debug("ptm.status_fetch_failed", { prId, error: getErrorMessage(err) }); // Fall through; treat as "unknown" → safest is to run the base sync. - behindBaseBy = 1; + behindBaseBy = null; } if (!isAutoConvergeStillEnabled(prId)) { clearTimer(prId); return; } - // ---- Step 1: base-advance conflict check (only when actually behind) ---- - if (behindBaseBy > 0) { + // ---- Step 1: base-advance conflict check (when behind, or when compare is unknown) ---- + if (behindBaseBy == null || behindBaseBy > 0) { const baseSync = await applyConflictStrategy(fresh, "base_advance"); if (!isAutoConvergeStillEnabled(prId)) { clearTimer(prId); diff --git a/apps/desktop/src/main/services/prs/prIssueResolution.test.ts b/apps/desktop/src/main/services/prs/prIssueResolution.test.ts index 80a6b56a4..f90a5a94f 100644 --- a/apps/desktop/src/main/services/prs/prIssueResolution.test.ts +++ b/apps/desktop/src/main/services/prs/prIssueResolution.test.ts @@ -433,6 +433,32 @@ describe("issueInventoryService", () => { expect(insertCalls.length).toBe(0); }); + it("does not mark unresolved outdated tracked threads as fixed", () => { + const db = makeMockDb(); + const existing = makeFakeRow({ + id: "existing-outdated-thread", + external_id: "thread:thread-1", + state: "sent_to_agent", + round: 1, + }); + db.get.mockReturnValue(existing); + db.all.mockReturnValue([existing]); + + const service = createIssueInventoryService({ db }); + service.syncFromPrData( + PR_ID, + [], + [makeReviewThread({ isResolved: false, isOutdated: true })], + [], + ); + + const updateCalls = db.run.mock.calls.filter( + (call: unknown[]) => typeof call[0] === "string" && (call[0] as string).includes("update pr_issue_inventory"), + ); + expect(updateCalls.some((call: unknown[]) => (call[1] as unknown[])[8] === "fixed")).toBe(false); + expect(updateCalls.some((call: unknown[]) => (call[1] as unknown[])[8] === "sent_to_agent")).toBe(true); + }); + it("marks previously tracked resolved threads as fixed", () => { const db = makeMockDb(); db.get.mockReturnValue(makeFakeRow({ diff --git a/apps/desktop/src/main/services/prs/prService.test.ts b/apps/desktop/src/main/services/prs/prService.test.ts index 41e4da59e..98be1abb3 100644 --- a/apps/desktop/src/main/services/prs/prService.test.ts +++ b/apps/desktop/src/main/services/prs/prService.test.ts @@ -1403,6 +1403,53 @@ describe("prService.getStatus", () => { vi.useRealTimers(); } }); + + it("keeps behindBaseBy unknown when GitHub compare fails", async () => { + const row = makePrRow({ id: "pr-status-compare-failed", github_pr_number: 91 }); + const db = makeMockDb(); + installPullRequestRowStore(db, [row]); + const githubService = makeGithubService({ + apiRequest: vi.fn(async (args: { path: string }) => { + if (args.path === "/repos/test-owner/test-repo/pulls/91") { + return { + data: makeGitHubPull({ + number: 91, + html_url: row.github_url, + title: row.title, + mergeable: true, + mergeable_state: "clean", + head: { ref: "my-feature", sha: "head-sha" }, + base: { ref: "main", sha: "base-sha" }, + }), + }; + } + if (args.path === "/repos/test-owner/test-repo/commits/head-sha/status") { + return { data: { state: "success", statuses: [] } }; + } + if (args.path === "/repos/test-owner/test-repo/commits/head-sha/check-runs") { + return { data: { check_runs: [] } }; + } + if (args.path === "/repos/test-owner/test-repo/pulls/91/reviews") { + return { data: [] }; + } + if (args.path === "/repos/test-owner/test-repo/compare/base-sha...head-sha") { + throw new Error("compare unavailable"); + } + throw new Error(`Unexpected GitHub API path: ${args.path}`); + }), + }); + const { service } = buildService({ db, githubService }); + + const status = await service.getStatus("pr-status-compare-failed"); + + expect(status.behindBaseBy).toBeNull(); + const updateCall = db.run.mock.calls.find(([sql]: [unknown]) => + String(sql).includes("update pull_requests") + && String(sql).includes("behind_base_by") + ); + const updateParams = updateCall?.[1] as unknown[] | undefined; + expect(updateParams?.slice(-4, -2)).toEqual([1, null]); + }); }); describe("prService.refresh", () => { @@ -2313,6 +2360,70 @@ describe("prService.delete", () => { }); }); +describe("prService.land", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("does not send a merge request for draft PRs", async () => { + const row = makePrRow({ id: "pr-draft", github_pr_number: 92, state: "draft" }); + const db = makeMockDb(); + installPullRequestRowStore(db, [row]); + const githubService = makeGithubService({ + apiRequest: vi.fn(async (args: { method: string; path: string }) => { + if (args.method === "GET" && args.path === "/repos/test-owner/test-repo/pulls/92") { + return { + data: makeGitHubPull({ + number: 92, + draft: true, + state: "open", + mergeable: true, + mergeable_state: "clean", + }), + }; + } + throw new Error(`Unexpected GitHub API call: ${args.method} ${args.path}`); + }), + }); + const { service } = buildService({ db, githubService }); + + const result = await service.land({ prId: "pr-draft", method: "squash" }); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/draft/i); + expect(githubService.apiRequest).not.toHaveBeenCalledWith(expect.objectContaining({ method: "PUT" })); + }); + + it("short-circuits dirty mergeability before calling GitHub merge", async () => { + const row = makePrRow({ id: "pr-conflict", github_pr_number: 93 }); + const db = makeMockDb(); + installPullRequestRowStore(db, [row]); + const githubService = makeGithubService({ + apiRequest: vi.fn(async (args: { method: string; path: string }) => { + if (args.method === "GET" && args.path === "/repos/test-owner/test-repo/pulls/93") { + return { + data: makeGitHubPull({ + number: 93, + draft: false, + state: "open", + mergeable: false, + mergeable_state: "dirty", + }), + }; + } + throw new Error(`Unexpected GitHub API call: ${args.method} ${args.path}`); + }), + }); + const { service } = buildService({ db, githubService }); + + const result = await service.land({ prId: "pr-conflict", method: "merge" }); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/merge conflicts/i); + expect(githubService.apiRequest).not.toHaveBeenCalledWith(expect.objectContaining({ method: "PUT" })); + }); +}); + describe("prService.createFromLane", () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/apps/desktop/src/main/services/prs/prService.ts b/apps/desktop/src/main/services/prs/prService.ts index 38608af30..10d20a121 100644 --- a/apps/desktop/src/main/services/prs/prService.ts +++ b/apps/desktop/src/main/services/prs/prService.ts @@ -145,6 +145,7 @@ import { publishLinearPrCard } from "../cto/linearLaneCardService"; import { spawn } from "node:child_process"; import { runGit, runGitMergeTree, runGitOrThrow } from "../git/git"; import { shouldAttemptAdminMergeForRestError } from "./pathToMergeOrchestrator"; +import { deletePullRequestRowsByIds } from "./pullRequestRowCleanup"; import { extractFirstJsonObject } from "../ai/utils"; import { buildIntegrationPreflight } from "./integrationPlanning"; import { hasMergeConflictMarkers, parseGitStatusPorcelain } from "./integrationValidation"; @@ -2968,6 +2969,15 @@ export function createPrService({ }; }; + const bestEffort = async (label: string, task: Promise, fallback: T): Promise => { + try { + return await task; + } catch (error) { + logger.warn("prs.best_effort_failed", { label, error: getErrorMessage(error) }); + return fallback; + } + }; + const refreshOne = async (prId: string): Promise => { const row = getRow(prId); if (!row) throw new Error(`PR not found: ${prId}`); @@ -2981,9 +2991,9 @@ export function createPrService({ const [combinedStatus, checkRuns, reviews, compare] = await Promise.all([ headSha ? fetchCombinedStatus(repo, headSha) : Promise.resolve({ state: "", statuses: [] }), - headSha ? fetchCheckRuns(repo, headSha).catch((err) => { console.warn("[prService] fetchCheckRuns failed in refreshOne:", err?.message ?? err); return []; }) : Promise.resolve([]), - fetchReviews(repo, Number(row.github_pr_number)).catch(() => []), - baseSha && headSha ? fetchCompare(repo, baseSha, headSha).catch(() => ({ behindBy: null as number | null })) : Promise.resolve({ behindBy: null as number | null }) + headSha ? bestEffort("refreshOne.fetchCheckRuns", fetchCheckRuns(repo, headSha), [] as any[]) : Promise.resolve([]), + bestEffort("refreshOne.fetchReviews", fetchReviews(repo, Number(row.github_pr_number)), []), + baseSha && headSha ? bestEffort("refreshOne.fetchCompare", fetchCompare(repo, baseSha, headSha), { behindBy: null as number | null }) : Promise.resolve({ behindBy: null as number | null }) ]); const reviewStatesByUser = new Map(); for (const review of reviews) { @@ -3105,9 +3115,9 @@ export function createPrService({ const [combinedStatus, checkRuns, reviews, compare] = await Promise.all([ headSha ? fetchCombinedStatus(repo, headSha) : Promise.resolve({ state: "", statuses: [] }), - headSha ? fetchCheckRuns(repo, headSha).catch((err) => { console.warn("[prService] fetchCheckRuns failed in computeStatus:", err?.message ?? err); return []; }) : Promise.resolve([]), - fetchReviews(repo, summary.githubPrNumber).catch(() => []), - baseSha && headSha ? fetchCompare(repo, baseSha, headSha).catch(() => ({ behindBy: 0 })) : Promise.resolve({ behindBy: 0 }) + headSha ? bestEffort("computeStatus.fetchCheckRuns", fetchCheckRuns(repo, headSha), [] as any[]) : Promise.resolve([]), + bestEffort("computeStatus.fetchReviews", fetchReviews(repo, summary.githubPrNumber), []), + baseSha && headSha ? bestEffort("computeStatus.fetchCompare", fetchCompare(repo, baseSha, headSha), { behindBy: null as number | null }) : Promise.resolve({ behindBy: null as number | null }) ]); const requestedReviewers = Array.isArray(pr?.requested_reviewers) ? pr.requested_reviewers.map((u: any) => asString(u?.login)).filter(Boolean) : []; @@ -3160,8 +3170,8 @@ export function createPrService({ const headSha = asString(pr?.head?.sha); if (!headSha) return []; const [combinedStatus, checkRuns] = await Promise.all([ - fetchCombinedStatus(repo, headSha).catch((err) => { console.warn("[prService] fetchCombinedStatus failed in getChecks:", err?.message ?? err); return { state: "", statuses: [] }; }), - fetchCheckRuns(repo, headSha).catch((err) => { console.warn("[prService] fetchCheckRuns failed in getChecks:", err?.message ?? err); return []; }) + bestEffort("getChecks.fetchCombinedStatus", fetchCombinedStatus(repo, headSha), { state: "", statuses: [] }), + bestEffort("getChecks.fetchCheckRuns", fetchCheckRuns(repo, headSha), [] as any[]), ]); const out: PrCheck[] = []; @@ -3542,30 +3552,9 @@ export function createPrService({ rememberAutoLinkIgnore(row); } - db.run("delete from pr_group_members where pr_id = ?", [row.id]); - db.run( - ` - delete from pr_groups - where project_id = ? - and id in ( - select g.id - from pr_groups g - left join pr_group_members m on m.group_id = g.id - where g.project_id = ? - group by g.id - having count(m.id) = 0 - ) - `, - [projectId, projectId] - ); // Explicitly delete child rows that rely on FK cascade — CRR conversion can // strip checked foreign keys, leaving orphaned rows if we only rely on CASCADE. - db.run("delete from pr_convergence_state where pr_id = ?", [row.id]); - db.run("delete from pr_pipeline_settings where pr_id = ?", [row.id]); - db.run("delete from pr_issue_inventory where pr_id = ?", [row.id]); - db.run("delete from pull_request_ai_summaries where pr_id = ?", [row.id]); - db.run("delete from pull_request_snapshots where pr_id = ?", [row.id]); - db.run("delete from pull_requests where id = ? and project_id = ?", [row.id, projectId]); + deletePullRequestRowsByIds(db, projectId, [row.id]); let laneArchived = false; let laneArchiveError: string | null = null; @@ -4395,6 +4384,26 @@ export function createPrService({ return rawMsg; }; + try { + const latestPull = await fetchPr(repo, Number(row.github_pr_number), { waitForKnownMergeability: true }); + const latestState = toPrState({ + state: asString(latestPull?.state) || row.state || "open", + draft: Boolean(latestPull?.draft), + mergedAt: asString(latestPull?.merged_at) || null, + }); + if (latestState === "draft") { + return finishFailure("PR is draft", "PR is still a draft. Mark it ready for review before merging."); + } + if (latestState !== "open") { + return finishFailure(`PR is ${latestState}`, `PR is ${latestState}; only open PRs can be merged.`); + } + if (mergeConflictsFromPull(latestPull) === true) { + return finishFailure("PR has merge conflicts", "PR has merge conflicts. Rebase or resolve conflicts before merging."); + } + } catch (error) { + return finishFailure(getErrorMessage(error), `Unable to verify mergeability before merging: ${getErrorMessage(error)}`); + } + try { const merge = await githubService.apiRequest({ method: "PUT", diff --git a/apps/desktop/src/main/services/prs/pullRequestRowCleanup.ts b/apps/desktop/src/main/services/prs/pullRequestRowCleanup.ts new file mode 100644 index 000000000..cb039b395 --- /dev/null +++ b/apps/desktop/src/main/services/prs/pullRequestRowCleanup.ts @@ -0,0 +1,55 @@ +type DbLike = { + run(sql: string, params?: unknown[]): unknown; +}; + +function uniqueIds(ids: string[]): string[] { + return [...new Set(ids.map((id) => id.trim()).filter(Boolean))]; +} + +function pruneEmptyPrGroups(db: DbLike, projectId: string): void { + db.run( + ` + delete from pr_groups + where project_id = ? + and id in ( + select g.id + from pr_groups g + left join pr_group_members m on m.group_id = g.id + where g.project_id = ? + group by g.id + having count(m.id) = 0 + ) + `, + [projectId, projectId], + ); +} + +export function deletePullRequestRowsByIds(db: DbLike, projectId: string, prIds: string[]): void { + const ids = uniqueIds(prIds); + if (ids.length === 0) return; + const placeholders = ids.map(() => "?").join(", "); + + db.run(`delete from pr_group_members where pr_id in (${placeholders})`, ids); + db.run(`delete from pr_convergence_state where pr_id in (${placeholders})`, ids); + db.run(`delete from pr_pipeline_settings where pr_id in (${placeholders})`, ids); + db.run(`delete from pr_issue_inventory where pr_id in (${placeholders})`, ids); + db.run(`delete from pull_request_ai_summaries where pr_id in (${placeholders})`, ids); + db.run(`delete from pull_request_snapshots where pr_id in (${placeholders})`, ids); + db.run(`delete from pull_requests where project_id = ? and id in (${placeholders})`, [projectId, ...ids]); + pruneEmptyPrGroups(db, projectId); +} + +export function deletePullRequestRowsForLane(db: DbLike, projectId: string, laneId: string): void { + const params = [laneId, projectId]; + const prSelect = "select id from pull_requests where lane_id = ? and project_id = ?"; + + db.run("delete from pr_group_members where lane_id = ?", [laneId]); + db.run(`delete from pr_group_members where pr_id in (${prSelect})`, params); + db.run(`delete from pr_convergence_state where pr_id in (${prSelect})`, params); + db.run(`delete from pr_pipeline_settings where pr_id in (${prSelect})`, params); + db.run(`delete from pr_issue_inventory where pr_id in (${prSelect})`, params); + db.run(`delete from pull_request_ai_summaries where pr_id in (${prSelect})`, params); + db.run(`delete from pull_request_snapshots where pr_id in (${prSelect})`, params); + db.run("delete from pull_requests where lane_id = ? and project_id = ?", params); + pruneEmptyPrGroups(db, projectId); +} diff --git a/apps/desktop/src/renderer/components/prs/PrRebaseBanner.test.tsx b/apps/desktop/src/renderer/components/prs/PrRebaseBanner.test.tsx new file mode 100644 index 000000000..08e5c35b2 --- /dev/null +++ b/apps/desktop/src/renderer/components/prs/PrRebaseBanner.test.tsx @@ -0,0 +1,80 @@ +// @vitest-environment jsdom + +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; + +import { PrRebaseBanner } from "./PrRebaseBanner"; +import type { RebaseNeed } from "../../../shared/types"; + +afterEach(() => { + cleanup(); + delete (window as any).ade; +}); + +function makeNeed(overrides: Partial = {}): RebaseNeed { + return { + laneId: "lane-1", + laneName: "Lane 1", + kind: "lane_base", + baseBranch: "main", + behindBy: 2, + conflictPredicted: false, + conflictingFiles: [], + prId: null, + groupContext: null, + dismissedAt: null, + deferredUntil: null, + ...overrides, + }; +} + +describe("PrRebaseBanner", () => { + it("reports rebase failures instead of swallowing them", async () => { + (window as any).ade = { + rebase: { + execute: vi.fn(async () => { + throw new Error("rebase failed"); + }), + dismiss: vi.fn(), + }, + }; + + render( + {}} + />, + ); + + fireEvent.click(screen.getByRole("button", { name: /REBASE NOW/i })); + + expect(await screen.findByText("rebase failed")).toBeTruthy(); + }); + + it("refreshes after a successful banner rebase", async () => { + const onRefresh = vi.fn(async () => undefined); + const onRebaseDone = vi.fn(async () => undefined); + (window as any).ade = { + rebase: { + execute: vi.fn(async () => undefined), + dismiss: vi.fn(), + }, + }; + + render( + {}} + onRefresh={onRefresh} + onRebaseDone={onRebaseDone} + />, + ); + + fireEvent.click(screen.getByRole("button", { name: /REBASE NOW/i })); + + await waitFor(() => expect(onRefresh).toHaveBeenCalledTimes(1)); + expect(onRebaseDone).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/desktop/src/renderer/components/prs/PrRebaseBanner.tsx b/apps/desktop/src/renderer/components/prs/PrRebaseBanner.tsx index cc0a14b47..1bbfb4379 100644 --- a/apps/desktop/src/renderer/components/prs/PrRebaseBanner.tsx +++ b/apps/desktop/src/renderer/components/prs/PrRebaseBanner.tsx @@ -8,11 +8,14 @@ type PrRebaseBannerProps = { rebaseNeeds: RebaseNeed[]; autoRebaseStatuses?: AutoRebaseLaneStatus[]; onTabChange: (tab: string) => void; + onRefresh?: () => Promise | void; + onRebaseDone?: () => Promise | void; }; -export function PrRebaseBanner({ laneId, rebaseNeeds, autoRebaseStatuses, onTabChange }: PrRebaseBannerProps) { +export function PrRebaseBanner({ laneId, rebaseNeeds, autoRebaseStatuses, onTabChange, onRefresh, onRebaseDone }: PrRebaseBannerProps) { const [dismissed, setDismissed] = React.useState(false); const [syncBusy, setSyncBusy] = React.useState(false); + const [actionError, setActionError] = React.useState(null); const need = React.useMemo(() => { const laneNeed = findLaneBaseNeed(rebaseNeeds, laneId); @@ -26,6 +29,7 @@ export function PrRebaseBanner({ laneId, rebaseNeeds, autoRebaseStatuses, onTabC // Reset dismissed state when lane changes React.useEffect(() => { setDismissed(false); + setActionError(null); }, [laneId]); if (dismissed) return null; @@ -72,22 +76,29 @@ export function PrRebaseBanner({ laneId, rebaseNeeds, autoRebaseStatuses, onTabC const handleSync = async () => { setSyncBusy(true); + setActionError(null); try { await window.ade.rebase.execute({ laneId, aiAssisted: true }); - } catch { - /* swallow — user can use rebase tab for details */ + await onRefresh?.(); + if (onRebaseDone && onRebaseDone !== onRefresh) { + await onRebaseDone(); + } + } catch (error) { + setActionError(error instanceof Error ? error.message : String(error)); } finally { setSyncBusy(false); } }; const handleDismiss = async () => { + setActionError(null); try { await window.ade.rebase.dismiss(laneId); - } catch { - /* swallow */ + await onRefresh?.(); + setDismissed(true); + } catch (error) { + setActionError(error instanceof Error ? error.message : String(error)); } - setDismissed(true); }; return ( @@ -97,69 +108,76 @@ export function PrRebaseBanner({ laneId, rebaseNeeds, autoRebaseStatuses, onTabC border: "1px solid #F59E0B30", padding: "10px 14px", display: "flex", - alignItems: "center", - justifyContent: "space-between", - gap: 12, + flexDirection: "column", + gap: 8, }} > -
- - - {need.behindBy} commit{need.behindBy !== 1 ? "s" : ""} behind {need.baseBranch} - {need.conflictPredicted - ? " — conflicts predicted, rebase required" - : " — no conflicts, rebase recommended"} - -
-
- - - +
+
+ + + {need.behindBy} commit{need.behindBy !== 1 ? "s" : ""} behind {need.baseBranch} + {need.conflictPredicted + ? " — conflicts predicted, rebase required" + : " — no conflicts, rebase recommended"} + +
+
+ + + +
+ {actionError ? ( +
+ + {actionError} +
+ ) : null}
); } diff --git a/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx b/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx index 2424d760d..04693a7ac 100644 --- a/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx +++ b/apps/desktop/src/renderer/components/prs/detail/PrDetailPane.tsx @@ -2197,10 +2197,13 @@ export function PrDetailPane({ borderBottom: `1px solid ${actionResult.success ? "color-mix(in srgb, var(--color-success) 20%, transparent)" : "color-mix(in srgb, var(--color-error) 20%, transparent)"}`, fontFamily: SANS_FONT, fontSize: 12, color: actionResult.success ? COLORS.success : COLORS.danger, - display: "flex", alignItems: "center", gap: 8, + display: "flex", alignItems: "center", justifyContent: "space-between", gap: 8, }}> - {actionResult.success ? : } - {actionResult.success ? `Merged PR #${actionResult.prNumber}` : `Failed: ${actionResult.error ?? "unknown"}`} +
+ {actionResult.success ? : } + {actionResult.success ? `Merged PR #${actionResult.prNumber}` : `Failed: ${actionResult.error ?? "unknown"}`} +
+ )} @@ -2743,4 +2746,3 @@ function ChecksTab({ ); } - diff --git a/apps/desktop/src/renderer/components/prs/shared/PrDetailMergeRail.test.tsx b/apps/desktop/src/renderer/components/prs/shared/PrDetailMergeRail.test.tsx index af5303484..cf0b9cbf3 100644 --- a/apps/desktop/src/renderer/components/prs/shared/PrDetailMergeRail.test.tsx +++ b/apps/desktop/src/renderer/components/prs/shared/PrDetailMergeRail.test.tsx @@ -109,4 +109,25 @@ describe("PrDetailMergeRail", () => { expect(onDeleteBranch).toHaveBeenCalledTimes(1); expect(screen.getByTestId("pr-merge-merged-banner")).toBeTruthy(); }); + + it("requires confirmation before closing an open PR", () => { + const onClose = vi.fn(); + render( + {}} + onClose={onClose} + />, + ); + + fireEvent.click(screen.getByRole("button", { name: /Close pull request/i })); + expect(onClose).not.toHaveBeenCalled(); + fireEvent.click(screen.getByRole("button", { name: /Click again to close PR/i })); + expect(onClose).toHaveBeenCalledTimes(1); + }); }); diff --git a/apps/desktop/src/renderer/components/prs/shared/PrDetailMergeRail.tsx b/apps/desktop/src/renderer/components/prs/shared/PrDetailMergeRail.tsx index 023cd4b87..910f44d30 100644 --- a/apps/desktop/src/renderer/components/prs/shared/PrDetailMergeRail.tsx +++ b/apps/desktop/src/renderer/components/prs/shared/PrDetailMergeRail.tsx @@ -55,8 +55,10 @@ export const PrDetailMergeRail = memo(function PrDetailMergeRail({ const [menuOpen, setMenuOpen] = useState(false); const [showCliInstructions, setShowCliInstructions] = useState(false); const [deleteBranchArmed, setDeleteBranchArmed] = useState(false); + const [closePrArmed, setClosePrArmed] = useState(false); const menuRef = useRef(null); const deleteBranchArmTimer = useRef | null>(null); + const closePrArmTimer = useRef | null>(null); const handleDeleteBranchClick = useCallback(() => { if (!onDeleteBranch) return; @@ -71,10 +73,28 @@ export const PrDetailMergeRail = memo(function PrDetailMergeRail({ deleteBranchArmTimer.current = setTimeout(() => setDeleteBranchArmed(false), 4000); }, [deleteBranchArmed, onDeleteBranch]); + const handleClosePrClick = useCallback(() => { + if (!onClose) return; + if (closePrArmed) { + if (closePrArmTimer.current) clearTimeout(closePrArmTimer.current); + setClosePrArmed(false); + onClose(); + return; + } + setClosePrArmed(true); + if (closePrArmTimer.current) clearTimeout(closePrArmTimer.current); + closePrArmTimer.current = setTimeout(() => setClosePrArmed(false), 4000); + }, [closePrArmed, onClose]); + useEffect(() => () => { if (deleteBranchArmTimer.current) clearTimeout(deleteBranchArmTimer.current); + if (closePrArmTimer.current) clearTimeout(closePrArmTimer.current); }, []); + useEffect(() => { + setClosePrArmed(false); + }, [pr.id]); + useEffect(() => { setLocalMergeMethod(mergeMethod); }, [mergeMethod]); @@ -360,7 +380,7 @@ export const PrDetailMergeRail = memo(function PrDetailMergeRail({ ) : null} diff --git a/apps/desktop/src/renderer/components/prs/shared/prMergeRailUtils.test.ts b/apps/desktop/src/renderer/components/prs/shared/prMergeRailUtils.test.ts index ef82d164f..ba1505a08 100644 --- a/apps/desktop/src/renderer/components/prs/shared/prMergeRailUtils.test.ts +++ b/apps/desktop/src/renderer/components/prs/shared/prMergeRailUtils.test.ts @@ -94,6 +94,14 @@ describe("prMergeRailUtils", () => { })).toBe(true); }); + it("does not allow draft PR merge attempts even with bypass enabled", () => { + expect(canAttemptMerge({ + pr: makePr({ state: "draft" }), + status: makeStatus({ state: "draft", isMergeable: true }), + bypassRules: true, + })).toBe(false); + }); + it("builds gh merge instructions with optional admin bypass", () => { expect(buildMergeCommandLineInstructions({ repoOwner: "acme", diff --git a/apps/desktop/src/renderer/components/prs/shared/prMergeRailUtils.ts b/apps/desktop/src/renderer/components/prs/shared/prMergeRailUtils.ts index b95a0e90b..4f844a8ac 100644 --- a/apps/desktop/src/renderer/components/prs/shared/prMergeRailUtils.ts +++ b/apps/desktop/src/renderer/components/prs/shared/prMergeRailUtils.ts @@ -67,10 +67,11 @@ export function deriveMergeBlockers(args: { if (status?.mergeConflicts) { blockers.push({ id: "conflicts", label: "This branch has conflicts that must be resolved." }); } - if (status && status.behindBaseBy > 0) { + const behindBaseBy = status?.behindBaseBy ?? 0; + if (behindBaseBy > 0) { blockers.push({ id: "behind", - label: `The head branch is ${status.behindBaseBy} commit${status.behindBaseBy === 1 ? "" : "s"} behind the base branch.`, + label: `The head branch is ${behindBaseBy} commit${behindBaseBy === 1 ? "" : "s"} behind the base branch.`, }); } @@ -160,7 +161,7 @@ export function canAttemptMerge(args: { status: PrStatus | null; bypassRules: boolean; }): boolean { - if (args.pr.state !== "open" && args.pr.state !== "draft") return false; + if (args.pr.state !== "open") return false; if (args.status?.mergeConflicts) return false; if (args.bypassRules) return Boolean(args.status); return Boolean(args.status?.isMergeable); diff --git a/apps/desktop/src/renderer/components/prs/tabs/IntegrationTab.tsx b/apps/desktop/src/renderer/components/prs/tabs/IntegrationTab.tsx index 141d55ac8..9ae331b6b 100644 --- a/apps/desktop/src/renderer/components/prs/tabs/IntegrationTab.tsx +++ b/apps/desktop/src/renderer/components/prs/tabs/IntegrationTab.tsx @@ -1649,13 +1649,20 @@ export function IntegrationTab({ prs, lanes, mergeContextByPrId, mergeMethod, se {/* ---- Rebase banners for source lanes ---- */} {mergeSourcesResolved.map((s) => (
- { - if (tab === "rebase") { - const need = findLaneBaseNeed(rebaseNeeds, s.laneId); - setSelectedRebaseItemId(need ? rebaseNeedItemKey(need) : null); - } - setActiveTab(tab as "normal" | "queue" | "integration" | "rebase"); - }} /> + { + if (tab === "rebase") { + const need = findLaneBaseNeed(rebaseNeeds, s.laneId); + setSelectedRebaseItemId(need ? rebaseNeedItemKey(need) : null); + } + setActiveTab(tab as "normal" | "queue" | "integration" | "rebase"); + }} + />
))} diff --git a/apps/desktop/src/renderer/components/prs/tabs/RebaseTab.tsx b/apps/desktop/src/renderer/components/prs/tabs/RebaseTab.tsx index 9173e00bc..d3ab713b2 100644 --- a/apps/desktop/src/renderer/components/prs/tabs/RebaseTab.tsx +++ b/apps/desktop/src/renderer/components/prs/tabs/RebaseTab.tsx @@ -498,22 +498,24 @@ export function RebaseTab({ const handleDismiss = async () => { if (!selectedNeed) return; + setRebaseError(null); try { await window.ade.rebase.dismiss(selectedNeed.laneId); await onRefresh(); - } catch { - /* swallow */ + } catch (err: unknown) { + setRebaseError(err instanceof Error ? err.message : String(err)); } }; const handleDefer = async () => { if (!selectedNeed) return; const until = new Date(Date.now() + 4 * 60 * 60 * 1000).toISOString(); + setRebaseError(null); try { await window.ade.rebase.defer(selectedNeed.laneId, until); await onRefresh(); - } catch { - /* swallow */ + } catch (err: unknown) { + setRebaseError(err instanceof Error ? err.message : String(err)); } }; diff --git a/apps/desktop/src/renderer/components/review/ReviewPage.tsx b/apps/desktop/src/renderer/components/review/ReviewPage.tsx index 6ee2b1a05..0e008d5c2 100644 --- a/apps/desktop/src/renderer/components/review/ReviewPage.tsx +++ b/apps/desktop/src/renderer/components/review/ReviewPage.tsx @@ -1627,7 +1627,7 @@ export function ReviewPage({ active = true }: { active?: boolean } = {}) { {runs.length === 0 ? (
- No review runs yet in this workspace. Use Launch new review above to start one. + No review runs yet in this workspace. Start a new review from the toolbar.
) : runs.map((run) => { const active = run.id === selectedRunId; diff --git a/apps/desktop/src/shared/types/prs.ts b/apps/desktop/src/shared/types/prs.ts index 46bf75079..ab2a37ef2 100644 --- a/apps/desktop/src/shared/types/prs.ts +++ b/apps/desktop/src/shared/types/prs.ts @@ -60,7 +60,7 @@ export type PrStatus = { reviewStatus: PrReviewStatus; isMergeable: boolean; mergeConflicts: boolean; - behindBaseBy: number; + behindBaseBy: number | null; }; export type PrCheck = {