diff --git a/apps/ade-cli/src/bootstrap.ts b/apps/ade-cli/src/bootstrap.ts index 933a817b6..d09b68a91 100644 --- a/apps/ade-cli/src/bootstrap.ts +++ b/apps/ade-cli/src/bootstrap.ts @@ -1431,14 +1431,11 @@ export async function createAdeRuntime(args: { prService: headlessLinearServices.prService, aiIntegrationService, }); - const prMergeAutoSettlementService = agentChatService - ? createPrMergeAutoSettlementService({ - db, - sessionService, - agentChatService, - emitEvent: emitPrEvent, - }) - : null; + const prMergeAutoSettlementService = createPrMergeAutoSettlementService({ + db, + sessionService, + emitEvent: emitPrEvent, + }); // GitHub polling fallback. Runtime-bound desktop windows route PR reads to // this daemon instead of the desktop main process, so the daemon must own @@ -1454,7 +1451,7 @@ export async function createAdeRuntime(args: { headlessLinearServices.githubService.getBackgroundRequestPauseUntilMs(), onEvent: emitPrEvent, onPullRequestsSnapshot: (snapshot) => - prMergeAutoSettlementService?.processSnapshot(snapshot), + prMergeAutoSettlementService.processSnapshot(snapshot), onPullRequestsChanged: async ({ changedPrs, changes }) => { if (changedPrs.length > 0) { // Poll results must not start another hot-refresh window; doing so diff --git a/apps/desktop/src/main/main.ts b/apps/desktop/src/main/main.ts index f9fec670c..f999a1f20 100644 --- a/apps/desktop/src/main/main.ts +++ b/apps/desktop/src/main/main.ts @@ -3509,7 +3509,6 @@ app.whenReady().then(async () => { prMergeAutoSettlementServiceRef = createPrMergeAutoSettlementService({ db, sessionService, - agentChatService, emitEvent: emitPrEvent, }); laneTeardownDeps.agentChatService = { diff --git a/apps/desktop/src/main/services/prs/prAsync.test.ts b/apps/desktop/src/main/services/prs/prAsync.test.ts index 98c34067c..ebe2e6000 100644 --- a/apps/desktop/src/main/services/prs/prAsync.test.ts +++ b/apps/desktop/src/main/services/prs/prAsync.test.ts @@ -819,14 +819,14 @@ describe("prMergeAutoSettlementService", () => { }; } - it("settles only eligible lane agent sessions after a newly observed merge", async () => { + it("settles lane agent sessions even when a merge has settlement blockers", async () => { const db = createMemoryDb(); - const settleSessionsWithOutcome = vi.fn((ids: string[]) => ids); + const settledSessionIds = new Set(); + const settleSessionsWithOutcome = vi.fn((ids: string[]) => { + ids.forEach((id) => settledSessionIds.add(id)); + return ids; + }); const emitEvent = vi.fn(); - const getSettlementBlockers = vi.fn(async (sessionId: string) => - sessionId === "cli-blocked" - ? [{ code: "scheduled_work", message: "Complete scheduled work." }] - : []); const service = createPrMergeAutoSettlementService({ db: db as any, sessionService: { @@ -835,24 +835,23 @@ describe("prMergeAutoSettlementService", () => { id: "chat-ready", toolType: "codex-chat", archivedAt: null, - settledAt: null, + settledAt: settledSessionIds.has("chat-ready") ? "2026-03-24T12:01:05.000Z" : null, }, { id: "cli-blocked", toolType: "codex", archivedAt: null, - settledAt: null, + settledAt: settledSessionIds.has("cli-blocked") ? "2026-03-24T12:01:05.000Z" : null, }, { id: "raw-shell", toolType: "shell", archivedAt: null, - settledAt: null, + settledAt: settledSessionIds.has("raw-shell") ? "2026-03-24T12:01:05.000Z" : null, }, ]), settleSessionsWithOutcome, } as any, - agentChatService: { getSettlementBlockers } as any, emitEvent, }); const openPr = createSummary({ state: "open" }); @@ -872,29 +871,115 @@ describe("prMergeAutoSettlementService", () => { polledAt: "2026-03-24T12:01:05.000Z", }); - expect(getSettlementBlockers).toHaveBeenCalledTimes(2); - expect(getSettlementBlockers).toHaveBeenCalledWith( - "chat-ready", - { includeCurrentTurn: true }, - ); expect(settleSessionsWithOutcome).toHaveBeenCalledWith( ["chat-ready"], "PR #101 merged", "2026-03-24T12:01:05.000Z", "pr_merge", ); + expect(settleSessionsWithOutcome).toHaveBeenCalledWith( + ["cli-blocked"], + "PR #101 merged", + "2026-03-24T12:01:05.000Z", + "pr_merge", + ); expect(emitEvent).toHaveBeenCalledWith(expect.objectContaining({ type: "pr-sessions-auto-settled", prId: "pr-1", - settledSessionIds: ["chat-ready"], - settledCount: 1, + settledSessionIds: ["chat-ready", "cli-blocked"], + settledCount: 2, })); await service.processSnapshot({ prs: [mergedPr], polledAt: "2026-03-24T12:02:00.000Z", }); + expect(settleSessionsWithOutcome).toHaveBeenCalledTimes(2); + }); + + it("does not re-settle after reactivation, but settles for a later PR", async () => { + const db = createMemoryDb(); + let settled = false; + const settleSessionsWithOutcome = vi.fn((ids: string[]) => { + settled = true; + return ids; + }); + const service = createPrMergeAutoSettlementService({ + db: db as any, + sessionService: { + list: vi.fn(() => [{ + id: "chat-waiting", + toolType: "claude-chat", + archivedAt: null, + settledAt: settled ? "2026-03-24T12:01:05.000Z" : null, + }]), + settleSessionsWithOutcome, + } as any, + emitEvent: vi.fn(), + }); + const openPr = createSummary({ + state: "open", + githubPrNumber: 101, + chatSessionIds: ["chat-waiting"], + }); + const openSecondPr = createSummary({ + id: "pr-2", + githubPrNumber: 202, + state: "open", + chatSessionIds: ["chat-waiting"], + }); + const mergedPr = createSummary({ + state: "merged", + mergedAt: "2026-03-24T12:01:00.000Z", + chatSessionIds: ["chat-waiting"], + }); + const mergedSecondPr = { + ...openSecondPr, + state: "merged" as const, + mergedAt: "2026-03-24T12:03:00.000Z", + }; + + await service.processSnapshot({ + prs: [openPr, openSecondPr], + polledAt: "2026-03-24T12:00:00.000Z", + }); + await service.processSnapshot({ + prs: [mergedPr, openSecondPr], + polledAt: "2026-03-24T12:01:05.000Z", + }); + + expect(settleSessionsWithOutcome).toHaveBeenCalledTimes(1); + expect(settleSessionsWithOutcome).toHaveBeenLastCalledWith( + ["chat-waiting"], + "PR #101 merged", + "2026-03-24T12:01:05.000Z", + "pr_merge", + ); + expect(getPrMergeAutoSettlementState(db as any)?.handledPrIds).toEqual(["pr-1"]); + + // The user reactivates the chat. The old merged PR is already handled, so + // it must not immediately file the chat again. + settled = false; + await service.processSnapshot({ + prs: [mergedPr, openSecondPr], + polledAt: "2026-03-24T12:02:00.000Z", + }); expect(settleSessionsWithOutcome).toHaveBeenCalledTimes(1); + + // A distinct PR on the same lane gets its own one-shot settlement. + await service.processSnapshot({ + prs: [mergedPr, mergedSecondPr], + polledAt: "2026-03-24T12:03:05.000Z", + }); + + expect(settleSessionsWithOutcome).toHaveBeenLastCalledWith( + ["chat-waiting"], + "PR #202 merged", + "2026-03-24T12:03:05.000Z", + "pr_merge", + ); + expect(settleSessionsWithOutcome).toHaveBeenCalledTimes(2); + expect(getPrMergeAutoSettlementState(db as any)?.handledPrIds).toEqual(["pr-1", "pr-2"]); }); it("baselines old merges and only settles merges observed after re-enabling", async () => { @@ -911,7 +996,6 @@ describe("prMergeAutoSettlementService", () => { }]), settleSessionsWithOutcome, } as any, - agentChatService: { getSettlementBlockers: vi.fn(async () => []) } as any, emitEvent: vi.fn(), }); const oldMerge = createSummary({ @@ -975,7 +1059,6 @@ describe("prMergeAutoSettlementService", () => { it("settles only the chats explicitly linked to a merged PR", async () => { const db = createMemoryDb(); const settleSessionsWithOutcome = vi.fn((ids: string[]) => ids); - const getSettlementBlockers = vi.fn(async () => []); const service = createPrMergeAutoSettlementService({ db: db as any, sessionService: { @@ -985,7 +1068,6 @@ describe("prMergeAutoSettlementService", () => { ]), settleSessionsWithOutcome, } as any, - agentChatService: { getSettlementBlockers } as any, emitEvent: vi.fn(), }); const openPr = createSummary({ state: "open" }); @@ -998,8 +1080,6 @@ describe("prMergeAutoSettlementService", () => { }); await service.processSnapshot({ prs: [mergedPr], polledAt: "2026-03-24T12:01:05.000Z" }); - expect(getSettlementBlockers).toHaveBeenCalledTimes(1); - expect(getSettlementBlockers).toHaveBeenCalledWith("chat-owned", { includeCurrentTurn: true }); expect(settleSessionsWithOutcome).toHaveBeenCalledWith( ["chat-owned"], "PR #101 merged", @@ -1031,7 +1111,6 @@ describe("prMergeAutoSettlementService", () => { }]), settleSessionsWithOutcome, } as any, - agentChatService: { getSettlementBlockers: vi.fn(async () => []) } as any, emitEvent, }); @@ -1081,16 +1160,8 @@ describe("prMergeAutoSettlementService", () => { })); }); - it("honors a concurrent disable while blocker checks are in flight", async () => { + it("honors disabling auto-settle before a merged PR is processed", async () => { const db = createMemoryDb(); - let releaseBlockerCheck: (() => void) | null = null; - const blockerCheckStarted = new Promise((resolve) => { - releaseBlockerCheck = resolve; - }); - let finishBlockerCheck!: () => void; - const blockerCheckFinished = new Promise((resolve) => { - finishBlockerCheck = resolve; - }); const settleSessionsWithOutcome = vi.fn((ids: string[]) => ids); const service = createPrMergeAutoSettlementService({ db: db as any, @@ -1103,13 +1174,6 @@ describe("prMergeAutoSettlementService", () => { }]), settleSessionsWithOutcome, } as any, - agentChatService: { - getSettlementBlockers: vi.fn(async () => { - releaseBlockerCheck?.(); - await blockerCheckFinished; - return []; - }), - } as any, emitEvent: vi.fn(), }); const openPr = createSummary({ state: "open" }); @@ -1121,26 +1185,22 @@ describe("prMergeAutoSettlementService", () => { state: "merged", mergedAt: "2026-03-24T12:01:00.000Z", }); - - const processing = service.processSnapshot({ - prs: [mergedPr], - polledAt: "2026-03-24T12:01:05.000Z", - }); - await blockerCheckStarted; setSessionLifecycleSettings({ db: db as any, settings: { autoSettleLaneSessionsOnPrMerge: false }, - currentPrs: [mergedPr], - now: "2026-03-24T12:01:06.000Z", + currentPrs: [openPr], + now: "2026-03-24T12:01:00.000Z", + }); + await service.processSnapshot({ + prs: [mergedPr], + polledAt: "2026-03-24T12:01:05.000Z", }); - finishBlockerCheck(); - await processing; expect(settleSessionsWithOutcome).not.toHaveBeenCalled(); expect(getSessionLifecycleSettings(db as any).autoSettleLaneSessionsOnPrMerge).toBe(false); expect(getPrMergeAutoSettlementState(db as any)).toEqual({ enabledSince: null, - handledPrIds: ["pr-1"], + handledPrIds: [], }); }); }); diff --git a/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts b/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts index 137bdc0b2..7b8d858c6 100644 --- a/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts +++ b/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts @@ -1,4 +1,3 @@ -import type { createAgentChatService } from "../chat/agentChatService"; import type { createSessionService } from "../sessions/sessionService"; import type { AdeDb } from "../state/kvDb"; import type { PrEventPayload, PrSummary } from "../../../shared/types"; @@ -22,7 +21,6 @@ function isMergeAtOrAfter(mergedAt: string | null | undefined, enabledSince: str export function createPrMergeAutoSettlementService(args: { db: Pick; sessionService: Pick, "list" | "settleSessionsWithOutcome">; - agentChatService: Pick, "getSettlementBlockers">; emitEvent: (event: PrEventPayload) => void; }) { /** @@ -107,10 +105,6 @@ export function createPrMergeAutoSettlementService(args: { const settledSessionIds: string[] = []; for (const session of rows) { - const blockers = await args.agentChatService.getSettlementBlockers( - session.id, - { includeCurrentTurn: true }, - ); const currentSettings = getSessionLifecycleSettings(args.db); const currentState = getPrMergeAutoSettlementState(args.db); if ( @@ -121,18 +115,23 @@ export function createPrMergeAutoSettlementService(args: { ) { break; } - if (blockers.length === 0) { - settledSessionIds.push(...args.sessionService.settleSessionsWithOutcome( - [session.id], - `PR #${pr.githubPrNumber} merged`, - polledAt, - "pr_merge", - )); - } + // A merged PR is an explicit lifecycle decision: file the linked + // session even when it still owns scheduled work, a background task, + // or another normal settlement blocker. Real activity can unsettle it + // again, while handledPrIds prevents this PR from filing it twice. + settledSessionIds.push(...args.sessionService.settleSessionsWithOutcome( + [session.id], + `PR #${pr.githubPrNumber} merged`, + polledAt, + "pr_merge", + )); } const finalSettings = getSessionLifecycleSettings(args.db); const finalState = getPrMergeAutoSettlementState(args.db); + // Mark this PR handled even when its session had background work. The + // merge itself is the explicit override, and a later user reactivation + // belongs to a new lifecycle rather than this already-consumed merge. if ( finalSettings.autoSettleLaneSessionsOnPrMerge && finalState?.enabledSince @@ -145,10 +144,11 @@ export function createPrMergeAutoSettlementService(args: { }); } - // Settling still happens for a PR we are meeting for the first time — - // its sessions really are finished and should be filed. Announcing it - // does not: "PR #977 merged" is news only if you did not already know, - // and a PR that was merged before we ever laid eyes on it is history. + // Filing still happens for a PR we are meeting for the first time — the + // merge is an explicit decision to file its linked sessions. Announcing + // it does not: "PR #977 merged" is news only if you did not already + // know, and a PR that was merged before we ever laid eyes on it is + // history. // // Gated here rather than in the toast so every consumer inherits it — // desktop toasts, mobile push, and anything added later. diff --git a/apps/desktop/src/renderer/state/crossMachineLanes.test.ts b/apps/desktop/src/renderer/state/crossMachineLanes.test.ts index 8d151df81..17b664707 100644 --- a/apps/desktop/src/renderer/state/crossMachineLanes.test.ts +++ b/apps/desktop/src/renderer/state/crossMachineLanes.test.ts @@ -1000,6 +1000,47 @@ describe("foreign payload decoding", () => { expect(decodeForeignSessions("nope")).toEqual([]); }); + it("defaults an unprojected foreign chat to idle without masking explicit activity", () => { + const [quiet, active, waiting, cli] = decodeForeignSessions([ + { + id: "chat-quiet", + laneId: "lane-1", + status: "running", + toolType: "claude-chat", + currentTurnStartedAt: "2026-08-06T12:00:00.000Z", + }, + { + id: "chat-active", + laneId: "lane-1", + status: "running", + toolType: "claude-chat", + runtimeState: "running", + }, + { + id: "chat-waiting", + laneId: "lane-1", + status: "running", + toolType: "claude-chat", + pendingInputItemId: "approval-1", + }, + { + id: "cli-session", + laneId: "lane-1", + status: "running", + toolType: "codex", + }, + ]); + + expect(quiet).toEqual(expect.objectContaining({ + runtimeState: "idle", + currentTurnStartedAt: null, + chatIdleSinceAt: null, + })); + expect(active?.runtimeState).toBe("running"); + expect(waiting?.runtimeState).toBe("waiting-input"); + expect(cli?.runtimeState).toBeUndefined(); + }); + // A half-decoded PR renders "PR #undefined", or a badge whose click is a // silent no-op because the foreign click-through has nowhere to go. Dropping // the row shows no badge, which is honest. diff --git a/apps/desktop/src/renderer/state/crossMachineLanes.ts b/apps/desktop/src/renderer/state/crossMachineLanes.ts index c25dc09fd..ec5a9f63f 100644 --- a/apps/desktop/src/renderer/state/crossMachineLanes.ts +++ b/apps/desktop/src/renderer/state/crossMachineLanes.ts @@ -38,7 +38,7 @@ import type { RemoteRuntimeConnectionStatus, TerminalSessionSummary, } from "../../shared/types"; -import { buildOptimisticChatSessionSummary } from "../lib/sessions"; +import { buildOptimisticChatSessionSummary, isChatToolType } from "../lib/sessions"; import { THIS_MACHINE_ID, THIS_MACHINE_NAME, @@ -887,7 +887,27 @@ export function decodeForeignSessions(result: unknown): TerminalSessionSummary[] if (!isRecord(candidate)) continue; if (typeof candidate.id !== "string" || !candidate.id.trim()) continue; if (typeof candidate.laneId !== "string" || !candidate.laneId.trim()) continue; - sessions.push(candidate as unknown as TerminalSessionSummary); + const session = candidate as unknown as TerminalSessionSummary; + // Older/partial remote runtimes can omit the chat projection fields. A + // persisted chat remains `status: "running"` between turns, so an absent + // runtimeState must not turn a quiet remote chat into an apparently active + // row (which hides Settle from its context menu). Mirror the main-process + // projection fallback, but never override an explicit runtime state. + if ( + candidate.status === "running" + && typeof candidate.toolType === "string" + && isChatToolType(candidate.toolType) + && candidate.runtimeState == null + ) { + sessions.push({ + ...session, + runtimeState: session.pendingInputItemId ? "waiting-input" : "idle", + currentTurnStartedAt: null, + chatIdleSinceAt: session.chatIdleSinceAt ?? null, + }); + } else { + sessions.push(session); + } } return sessions; } diff --git a/docs/features/pull-requests/README.md b/docs/features/pull-requests/README.md index cc8cb23c4..983fa3291 100644 --- a/docs/features/pull-requests/README.md +++ b/docs/features/pull-requests/README.md @@ -163,7 +163,7 @@ Service files (`apps/desktop/src/main/services/prs/`): | `prAsync.test.ts` | Shared bounded-concurrency and async helper coverage, plus the `prMergeAutoSettlementService` regression suite. | | `pullRequestRowCleanup.ts` | The only writer of the detach columns. `detachPullRequestRowsForLane` stamps `detached_at` + the frozen lane identity and provenance when a lane is deleted, lifts `commit_count` / `changed_files` off the snapshot, nulls the bulky snapshot JSON columns, drops lane-scoped group membership, and removes live PR↔chat routing edges. `detachPullRequestRowsByIds` remains an explicit cleanup helper for callers that truly need to detach selected rows; ordinary branch switching retains previous-branch PRs as live lane history. `countLaneProvenance` must run *before* the caller deletes the lane's sessions / artifacts / checkpoints. `deletePullRequestRowsByIds` remains for genuinely destructive paths. See [Multi-PR lane ownership](#multi-pr-lane-ownership-and-chat-edges) and [Detached PR rows](#detached-pr-rows). | | `prPollingService.ts` | Webhook-first PR freshness plus the direct-GitHub safety net. `reconcilePrs(prIds)` coalesces webhook-linked ids and refreshes only those rows immediately. A healthy relay suppresses hot polling and reduces broad refreshes to a 15-minute safety sweep; an unhealthy relay uses the configurable 60 s fallback (clamped to 5 s–5 min) and user-driven hot windows of 15 s for the first minute, then 30 s until the three-minute cap. Empty-cache discovery runs at most every 30 minutes with a healthy relay or 10 minutes without one. Before every network refresh, the poller honors credential cooldown/reset state and preserves the final 500 core/GraphQL requests for foreground actions. It writes `last_polled_at` per PR for delta polling. The ADE daemon owns an instance (created + started + disposed in `apps/ade-cli/src/bootstrap.ts`) for runtime-bound windows; the desktop main process owns the local-bound instance. | -| `prMergeAutoSettlementService.ts` | Applies the enabled lane-PR merge settlement policy after each polling snapshot. It files eligible, unblocked chat and tracked-agent-CLI sessions for a newly discovered merged PR, but emits `pr-sessions-auto-settled` only when the preceding in-memory snapshot contained that PR as open or draft. A first-sight merge — including backfilled history from another machine or the first snapshot after restart — is filed silently, so an imported history cannot generate merge toasts or push notifications. | +| `prMergeAutoSettlementService.ts` | Applies the enabled lane-PR merge settlement policy after each polling snapshot. It files linked chat and tracked-agent-CLI sessions for a newly discovered merged PR even when normal settlement blockers or background work remain: the merge is the explicit override. Each PR is handled once, so user reactivation is not re-filed by that old merge, while another linked PR can file a later lifecycle. It emits `pr-sessions-auto-settled` only when the preceding in-memory snapshot contained that PR as open or draft. A first-sight merge — including backfilled history from another machine or the first snapshot after restart — is filed silently, so an imported history cannot generate merge toasts or push notifications. | | `prChatCards.ts` | Converts bounded PR polling transitions into durable `ade_card` episodes for linked Work chats: CI completion/failure, review received, merge ready, conflicts, and merged. CI jobs are failure-first, capped at three visible rows with `rowsTruncated`, and report an honest `degradedReason` + Retry action when both job/check detail sources fail instead of rendering an empty success state. Desktop-main and daemon-owned pollers call the same emitter, and failures are isolated per PR/session so one cold or malformed chat cannot stop the poll loop. | | `prSummaryService.ts` | AI PR summary generator; caches `PrAiSummary` per `(prId, headSha)` in `pull_request_ai_summaries` so pushes invalidate the cache | | `workflowGraph.ts` | `createWorkflowGraph` — reconstructs the CI pipeline DAG (`PrWorkflowGraph`) behind a swappable `WorkflowGraph` interface. GitHub's jobs API does not return `needs:`, so the graph is built by parsing the workflow YAML that actually ran and joining it to live run state. Parses **only** `jobs..needs` and `jobs..strategy.matrix`, with the existing `yaml` dep. Source order: lane worktree `git show :.github/workflows/` → GitHub Contents API `?ref=` (fork PRs / non-local repos) → `source: "none"` with an `unavailableReason`; it never guesses an edge. A single WORKFLOW degrades to flat swimlanes (not the whole graph) when a job uses a reusable workflow (`uses:`), has a `${{ }}` `name:`, or the YAML will not parse. Matrix legs collapse into one node whose state is the worst leg (failed > running > queued > passed > skipped); `tier` is a cycle-safe longest-path rank over `needs`; `criticalPath` is the longest-duration chain. Running nodes report live elapsed. Parsed YAML is cached per `(repo, headSha)` behind a TTL; the graph itself is always recomputed from live run state. |