diff --git a/apps/cli/src/commands/log.ts b/apps/cli/src/commands/log.ts index d4a1ffe1..bf487923 100644 --- a/apps/cli/src/commands/log.ts +++ b/apps/cli/src/commands/log.ts @@ -21,6 +21,7 @@ interface PRInfo { number: number; state: "OPEN" | "CLOSED" | "MERGED"; url: string; + title: string; } interface LogFlags { @@ -67,22 +68,21 @@ export async function log( return; } - // Get tracked bookmarks with OPEN PRs - const trackedBookmarks = engine.getTrackedBookmarks().filter((bookmark) => { - const meta = engine.getMeta(bookmark); - if (!meta?.prInfo) return true; - return meta.prInfo.state === "OPEN"; - }); + // Get all tracked bookmarks (show all, not just OPEN PRs) + const trackedBookmarks = engine.getTrackedBookmarks(); - // Build revset: tracked bookmarks + their ancestors down to trunk + WC + // Build revset: trunk + mutable tracked bookmarks + fork points + WC let revset: string; if (trackedBookmarks.length === 0) { - revset = `${trunkName}:: & ::@`; + revset = `${trunkName} | @`; } else { const bookmarkRevsets = trackedBookmarks .map((b) => `bookmarks(exact:"${b}")`) .join(" | "); - revset = `(${trunkName}:: & ::(${bookmarkRevsets})) | @`; + // Show: trunk, mutable tracked bookmarks, their fork points (parents that are ancestors of trunk), and WC + const mutableBookmarks = `((${bookmarkRevsets}) & mutable())`; + const forkPoints = `((${mutableBookmarks})- & ::${trunkName})`; + revset = `${trunkName} | ${mutableBookmarks} | ${forkPoints} | @`; } // Run jj log with our template @@ -108,6 +108,7 @@ export async function log( // Build enhancement data const prInfoMap = buildPRInfoMap(engine, trackedBookmarks); const modifiedBookmarks = await getModifiedBookmarks(cwd); + const behindTrunkChanges = await getBehindTrunkChanges(cwd); // Check if empty state (just on trunk with empty WC) const lines = result.value.stdout.split("\n"); @@ -129,6 +130,7 @@ export async function log( prInfoMap, modifiedBookmarks, trackedBookmarks, + behindTrunkChanges, ); message(output); @@ -152,6 +154,7 @@ function renderEnhancedOutput( prInfoMap: Map, modifiedBookmarks: Set, trackedBookmarks: string[], + behindTrunkChanges: Set, ): string { const lines = rawOutput.split("\n"); const output: string[] = []; @@ -161,27 +164,21 @@ function renderEnhancedOutput( let currentIsTracked = false; let currentIsModified = false; let currentIsTrunk = false; - let wcParentName: string | null = null; + let currentIsForkPoint = false; // Immutable commit included only for graph connectivity + let currentIsBehindTrunk = false; // Mutable commit whose parent is not current trunk let pendingHints: string[] = []; // Buffer hints to output after COMMIT - // First pass: find WC parent for modify hint - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; + // Check if WC parent is a tracked bookmark (for modify hint) + // We only show "arr modify" if WC is on top of a tracked change + const wcParentBookmark: string | null = null; + for (const line of lines) { if (line.includes("HINT:empty")) { - // Find the next CHANGE line to get parent name - for (let j = i + 1; j < lines.length; j++) { - const nextLine = lines[j]; - const changeMatch = nextLine.match( - /CHANGE:([^|]+)\|([^|]*)\|([^|]*)\|([^|]*)\|([^|]*)\|([^|]*)\|/, - ); - if (changeMatch) { - const bookmarks = changeMatch[5]; - const description = changeMatch[6]; - const bookmark = parseBookmark(bookmarks, trunkName); - wcParentName = bookmark || description || changeMatch[1].slice(0, 8); - break; - } - } + // WC is empty - check if parent is a tracked bookmark by looking at graph structure + // The parent in jj graph is indicated by the line connecting @ to the next commit + // But we can't reliably parse this from output, so check tracked bookmarks + // If there's exactly one tracked bookmark that's a direct parent of @, use it + // For now, we'll be conservative and not show modify hint unless we're certain + // TODO: Query jj for @- to get actual parent break; } } @@ -235,6 +232,19 @@ function renderEnhancedOutput( currentIsTracked = bookmarks.some((b) => trackedBookmarks.includes(b)); currentIsModified = bookmarks.some((b) => modifiedBookmarks.has(b)); currentIsTrunk = isTrunk; + // Fork point: immutable commit that's not trunk (included for graph connectivity) + currentIsForkPoint = isImmutable && !isTrunk; + currentIsBehindTrunk = behindTrunkChanges.has(changeId); + + // Skip rendering fork points - just keep graph lines + if (currentIsForkPoint) { + // Only output the graph connector line + const connectorOnly = graphPrefix.replace(/[◆○@]/g, "│"); + if (connectorOnly.trim()) { + output.push(connectorOnly); + } + break; + } // Replace the marker in graphPrefix with our styled version // jj uses: @ for WC, ○ for mutable, ◆ for immutable @@ -260,6 +270,7 @@ function renderEnhancedOutput( const shortId = formatChangeId(changeId, changeIdPrefix); const badges: string[] = []; + if (currentIsBehindTrunk) badges.push(yellow("behind trunk")); if (currentIsModified) badges.push(yellow("local changes")); if (hasConflict) badges.push(yellow("conflicts")); const badgeStr = @@ -273,6 +284,9 @@ function renderEnhancedOutput( } case "TIME:": { + // Skip for fork points + if (currentIsForkPoint) break; + const timestamp = new Date(data); const timeStr = formatRelativeTime(timestamp); output.push(`${graphPrefix}${dim(timeStr)}`); @@ -286,12 +300,15 @@ function renderEnhancedOutput( case "HINT:": { if (data === "empty") { // Buffer hints to output after COMMIT line + // Use a clean "│ " prefix, not the graph prefix which may have ~ terminators + const hintPrefix = "│ "; pendingHints.push( - `${graphPrefix}${arr(COMMANDS.create)} ${dim('"message"')} ${dim("to save as new change")}`, + `${hintPrefix}${arr(COMMANDS.create)} ${dim('"message"')} ${dim("to save as new change")}`, ); - if (wcParentName) { + // Only show modify hint if WC parent is a tracked bookmark + if (wcParentBookmark) { pendingHints.push( - `${graphPrefix}${arr(COMMANDS.modify)} ${dim(`to update ${wcParentName}`)}`, + `${hintPrefix}${arr(COMMANDS.modify)} ${dim(`to update ${wcParentBookmark}`)}`, ); } } @@ -299,18 +316,30 @@ function renderEnhancedOutput( } case "PR:": { - const [bookmarksStr, description] = data.split("|"); + // Skip for fork points + if (currentIsForkPoint) break; + + const [bookmarksStr] = data.split("|"); const bookmark = parseBookmark(bookmarksStr, trunkName); - if (bookmark && bookmark !== trunkName && currentIsTracked) { + // Don't show PR info for trunk or if the change is immutable (already merged into trunk) + if ( + bookmark && + bookmark !== trunkName && + currentIsTracked && + !currentIsTrunk + ) { const prInfo = prInfoMap.get(bookmark); if (prInfo) { - output.push(`${graphPrefix}${formatPRLine(prInfo, description)}`); - output.push(`${graphPrefix}${cyan(prInfo.url)}`); - if (currentIsModified) { - pendingHints.push( - `${graphPrefix}${arr(COMMANDS.submit)} ${dim("to push local changes")}`, - ); + // Don't show merged/closed PRs that are now part of trunk + if (prInfo.state === "OPEN") { + output.push(`${graphPrefix}${formatPRLine(prInfo)}`); + output.push(`${graphPrefix}${cyan(prInfo.url)}`); + if (currentIsModified) { + pendingHints.push( + `${graphPrefix}${arr(COMMANDS.submit)} ${dim("to push local changes")}`, + ); + } } } else { output.push(`${graphPrefix}${dim("Not submitted")}`); @@ -323,10 +352,21 @@ function renderEnhancedOutput( } case "COMMIT:": { + // Skip for fork points + if (currentIsForkPoint) break; + const [commitId, commitIdPrefix, description] = data.split("|"); const commitIdFormatted = formatCommitId(commitId, commitIdPrefix); - // For trunk, we need to add the │ connector since there's no PR section - const prefix = currentIsTrunk ? "│ " : graphPrefix; + // Ensure we always have a │ prefix - jj may give us empty/wrong prefix for some lines + // Especially for lines after merges or when WC is a child + let prefix = graphPrefix; + if ( + !prefix.includes("│") && + !prefix.includes("├") && + !prefix.includes("╯") + ) { + prefix = "│ "; + } output.push( `${prefix}${commitIdFormatted} ${dim(`- ${description || "(no description)"}`)}`, ); @@ -382,6 +422,7 @@ function buildPRInfoMap( number: meta.prInfo.number, state: meta.prInfo.state, url: meta.prInfo.url, + title: meta.prInfo.title, }); } } @@ -401,6 +442,34 @@ async function getModifiedBookmarks(cwd: string): Promise> { return modifiedBookmarks; } +/** + * Get change IDs that are "behind trunk" (mutable but not descendants of current trunk). + */ +async function getBehindTrunkChanges(cwd: string): Promise> { + const result = await runJJ( + [ + "log", + "-r", + "mutable() ~ trunk()::", + "--no-graph", + "-T", + 'change_id.short() ++ "\\n"', + ], + cwd, + ); + + const behindChanges = new Set(); + if (result.ok) { + for (const line of result.value.stdout.split("\n")) { + const changeId = line.trim(); + if (changeId) { + behindChanges.add(changeId); + } + } + } + return behindChanges; +} + function formatChangeId(changeId: string, prefix: string): string { const short = changeId.slice(0, 8); if (prefix && short.startsWith(prefix)) { @@ -417,12 +486,12 @@ function formatCommitId(commitId: string, prefix: string): string { return cyan(short); } -function formatPRLine(prInfo: PRInfo, description: string): string { +function formatPRLine(prInfo: PRInfo): string { const stateColor = prInfo.state === "MERGED" ? magenta : prInfo.state === "OPEN" ? green : red; const stateLabel = prInfo.state.charAt(0) + prInfo.state.slice(1).toLowerCase(); - return `${stateColor(`PR #${prInfo.number}`)} ${dim(`(${stateLabel})`)} ${description}`; + return `${stateColor(`PR #${prInfo.number}`)} ${dim(`(${stateLabel})`)} ${prInfo.title}`; } function formatRelativeTime(date: Date): string { diff --git a/apps/cli/src/commands/restack.ts b/apps/cli/src/commands/restack.ts index 36e54227..ff59bfa1 100644 --- a/apps/cli/src/commands/restack.ts +++ b/apps/cli/src/commands/restack.ts @@ -1,11 +1,13 @@ import { restack as coreRestack } from "@array/core/commands/restack"; +import type { ArrContext } from "@array/core/engine"; import { formatSuccess, message, status } from "../utils/output"; import { unwrap } from "../utils/run"; -export async function restack(): Promise { +export async function restack(ctx: ArrContext): Promise { status("Restacking all changes onto trunk..."); - const result = unwrap(await coreRestack()); + const trackedBookmarks = ctx.engine.getTrackedBookmarks(); + const result = unwrap(await coreRestack({ trackedBookmarks })); if (result.restacked === 0) { message("All stacks already up to date with trunk"); diff --git a/apps/cli/src/commands/submit.ts b/apps/cli/src/commands/submit.ts index 38f3ae34..1479ffa5 100644 --- a/apps/cli/src/commands/submit.ts +++ b/apps/cli/src/commands/submit.ts @@ -33,7 +33,7 @@ export async function submit( process.exit(1); } - status("Submitting stack as linked PRs..."); + status("Submitting active stack as linked PRs..."); blank(); const result = unwrap( diff --git a/apps/cli/src/commands/sync.ts b/apps/cli/src/commands/sync.ts index ebe3e215..27a62de2 100644 --- a/apps/cli/src/commands/sync.ts +++ b/apps/cli/src/commands/sync.ts @@ -1,7 +1,7 @@ import { restack as coreRestack } from "@array/core/commands/restack"; import { sync as coreSync } from "@array/core/commands/sync"; import type { ArrContext, Engine } from "@array/core/engine"; -import { type MergedChange, reparentAndCleanup } from "@array/core/stacks"; +import { deleteBookmark } from "@array/core/jj"; import { COMMANDS } from "../registry"; import { arr, @@ -16,50 +16,71 @@ import { import { confirm } from "../utils/prompt"; import { unwrap } from "../utils/run"; -interface CleanupStats { - cleanedUp: number; - reparented: number; - prBasesUpdated: number; +interface StaleBookmark { + bookmark: string; + prNumber: number; + title: string; + state: "MERGED" | "CLOSED"; } /** - * Prompt user for each merged/closed PR and cleanup if confirmed. - * Reparents children to grandparent before cleanup. + * Find tracked bookmarks with MERGED or CLOSED PRs that should be cleaned up. */ -async function promptAndCleanupMerged( - pending: MergedChange[], - engine: Engine, -): Promise { - if (pending.length === 0) { - return { cleanedUp: 0, reparented: 0, prBasesUpdated: 0 }; +function findStaleTrackedBookmarks(engine: Engine): StaleBookmark[] { + const stale: StaleBookmark[] = []; + const trackedBookmarks = engine.getTrackedBookmarks(); + + for (const bookmark of trackedBookmarks) { + const meta = engine.getMeta(bookmark); + if (meta?.prInfo) { + if (meta.prInfo.state === "MERGED" || meta.prInfo.state === "CLOSED") { + stale.push({ + bookmark, + prNumber: meta.prInfo.number, + title: meta.prInfo.title, + state: meta.prInfo.state, + }); + } + } } + return stale; +} + +/** + * Prompt user to clean up stale bookmarks (merged/closed PRs). + * This untrracks from arr and deletes the jj bookmark if it exists. + */ +async function promptAndCleanupStale( + stale: StaleBookmark[], + engine: Engine, +): Promise { + if (stale.length === 0) return 0; + let cleanedUp = 0; - let reparented = 0; - let prBasesUpdated = 0; - for (const change of pending) { - const prLabel = magenta(`PR #${change.prNumber}`); - const branchLabel = dim(`(${change.bookmark})`); - const desc = change.description || "(no description)"; - const stateLabel = change.reason === "merged" ? "merged" : "closed"; + for (const item of stale) { + const prLabel = magenta(`PR #${item.prNumber}`); + const branchLabel = dim(`(${item.bookmark})`); + const stateLabel = item.state === "MERGED" ? "merged" : "closed"; const confirmed = await confirm( - `Delete ${stateLabel} branch ${prLabel} ${branchLabel}: ${desc}?`, + `Clean up ${stateLabel} ${prLabel} ${branchLabel}: ${item.title}?`, { default: true }, ); if (confirmed) { - const result = await reparentAndCleanup(change, engine); - if (result.ok) { - cleanedUp++; - reparented += result.value.reparentedChildren.length; - prBasesUpdated += result.value.prBasesUpdated; - } + // Untrack from arr + engine.untrack(item.bookmark); + + // Delete the jj bookmark if it exists + await deleteBookmark(item.bookmark); + + cleanedUp++; } } - return { cleanedUp, reparented, prBasesUpdated }; + return cleanedUp; } export async function sync(ctx: ArrContext): Promise { @@ -68,55 +89,33 @@ export async function sync(ctx: ArrContext): Promise { const result = unwrap(await coreSync({ engine: ctx.engine })); // Check if anything actually happened - const hadChanges = - result.fetched || - result.rebased || - result.merged.length > 0 || - result.empty.length > 0 || - result.hasConflicts; - - if (!hadChanges && result.pendingCleanup.length === 0) { + const hadChanges = result.fetched || result.rebased || result.hasConflicts; + + if (!hadChanges) { message(formatSuccess("Already up to date")); } else { - if (result.fetched && !result.hasConflicts && result.merged.length === 0) { + if (result.fetched && !result.hasConflicts) { message(formatSuccess("Synced with remote")); } if (result.hasConflicts) { warning("Rebase resulted in conflicts"); hint(`Resolve conflicts and run ${arr(COMMANDS.sync)} again`); } - - if (result.merged.length > 0) { - message( - formatSuccess(`Cleaned up ${result.merged.length} merged change(s)`), - ); - } - - if (result.empty.length > 0) { - hint(`Removed ${result.empty.length} empty change(s)`); - } } - // Prompt for each merged/closed PR cleanup - const cleanupStats = await promptAndCleanupMerged( - result.pendingCleanup, + // Find and prompt to clean up stale bookmarks (merged/closed PRs) + const staleBookmarks = findStaleTrackedBookmarks(ctx.engine); + const cleanedUpCount = await promptAndCleanupStale( + staleBookmarks, ctx.engine, ); - if (cleanupStats.cleanedUp > 0) { - const prLabel = cleanupStats.cleanedUp === 1 ? "PR" : "PRs"; - message(formatSuccess(`Cleaned up ${cleanupStats.cleanedUp} ${prLabel}`)); - - if (cleanupStats.reparented > 0) { - const childLabel = cleanupStats.reparented === 1 ? "child" : "children"; - hint(`Reparented ${cleanupStats.reparented} ${childLabel} to new parent`); - } - - if (cleanupStats.prBasesUpdated > 0) { - const baseLabel = - cleanupStats.prBasesUpdated === 1 ? "PR base" : "PR bases"; - hint(`Updated ${cleanupStats.prBasesUpdated} ${baseLabel} on GitHub`); - } + if (cleanedUpCount > 0) { + message( + formatSuccess( + `Cleaned up ${cleanedUpCount} ${cleanedUpCount === 1 ? "branch" : "branches"}`, + ), + ); } if (result.updatedComments > 0) { @@ -132,7 +131,8 @@ export async function sync(ctx: ArrContext): Promise { ); if (confirmed) { status("Restacking and pushing..."); - const restackResult = unwrap(await coreRestack()); + const trackedBookmarks = ctx.engine.getTrackedBookmarks(); + const restackResult = unwrap(await coreRestack({ trackedBookmarks })); if (restackResult.restacked > 0) { message( diff --git a/apps/cli/src/registry.ts b/apps/cli/src/registry.ts index 608fb9c9..f6546420 100644 --- a/apps/cli/src/registry.ts +++ b/apps/cli/src/registry.ts @@ -125,7 +125,7 @@ export const HANDLERS: Record = { bottom: () => bottom(), log: (p, ctx) => log(ctx!, { debug: !!p.flags.debug }), sync: (_p, ctx) => sync(ctx!), - restack: () => restack(), + restack: (_p, ctx) => restack(ctx!), checkout: (p) => checkout(p.args[0]), delete: (p, ctx) => deleteChange(p.args[0], ctx!, { yes: !!p.flags.yes || !!p.flags.y }), diff --git a/packages/core/src/commands/restack.ts b/packages/core/src/commands/restack.ts index 5a053891..db9a6b68 100644 --- a/packages/core/src/commands/restack.ts +++ b/packages/core/src/commands/restack.ts @@ -12,47 +12,69 @@ interface RestackResult { pushed: string[]; } +interface RestackOptions { + /** Tracked bookmarks to consider for restacking */ + trackedBookmarks: string[]; +} + /** - * Check if there are mutable changes not based on current trunk. + * Find tracked bookmarks that are behind trunk (not based on current trunk tip). */ -async function getStacksBehindTrunk(): Promise> { - const result = await runJJ([ - "log", - "-r", - "roots(mutable() ~ trunk()..)", - "--no-graph", - "-T", - `change_id ++ "\\n"`, - ]); - if (!result.ok) return result; - const roots = result.value.stdout - .split("\n") - .filter((line) => line.trim() !== ""); - return ok(roots.length); +async function getBookmarksBehindTrunk( + trackedBookmarks: string[], +): Promise> { + if (trackedBookmarks.length === 0) { + return ok([]); + } + + const behindBookmarks: string[] = []; + + for (const bookmark of trackedBookmarks) { + // Check if this bookmark exists and is not a descendant of trunk + const result = await runJJ([ + "log", + "-r", + `bookmarks(exact:"${bookmark}") & mutable() ~ trunk()::`, + "--no-graph", + "-T", + `change_id ++ "\\n"`, + ]); + + if (result.ok && result.value.stdout.trim()) { + behindBookmarks.push(bookmark); + } + } + + return ok(behindBookmarks); } /** - * Rebase all mutable stacks onto trunk. + * Rebase tracked bookmarks that are behind trunk. */ -async function restackAll(): Promise> { - const countResult = await getStacksBehindTrunk(); - if (!countResult.ok) return countResult; +async function restackTracked( + trackedBookmarks: string[], +): Promise> { + const behindResult = await getBookmarksBehindTrunk(trackedBookmarks); + if (!behindResult.ok) return behindResult; - if (countResult.value === 0) { + const behind = behindResult.value; + if (behind.length === 0) { return ok({ restacked: 0 }); } - // Use mutable config for rebase on potentially pushed commits - const result = await runJJWithMutableConfigVoid([ - "rebase", - "-s", - "roots(mutable())", - "-d", - "trunk()", - ]); - if (!result.ok) return result; - - return ok({ restacked: countResult.value }); + // Rebase each behind bookmark onto trunk + for (const bookmark of behind) { + const result = await runJJWithMutableConfigVoid([ + "rebase", + "-b", + bookmark, + "-d", + "trunk()", + ]); + if (!result.ok) return result; + } + + return ok({ restacked: behind.length }); } /** @@ -76,15 +98,19 @@ async function pushAllUnpushed(): Promise> { } /** - * Fetch from remote, restack all changes onto trunk, and push rebased bookmarks. + * Fetch from remote, restack tracked bookmarks onto trunk, and push rebased bookmarks. */ -export async function restack(): Promise> { +export async function restack( + options: RestackOptions, +): Promise> { + const { trackedBookmarks } = options; + // Fetch latest first const fetchResult = await runJJ(["git", "fetch"]); if (!fetchResult.ok) return fetchResult; - // Restack all changes onto trunk - const restackResult = await restackAll(); + // Restack only tracked bookmarks that are behind trunk + const restackResult = await restackTracked(trackedBookmarks); if (!restackResult.ok) return restackResult; // Push all unpushed bookmarks @@ -97,10 +123,10 @@ export async function restack(): Promise> { }); } -export const restackCommand: Command = { +export const restackCommand: Command = { meta: { name: "restack", - description: "Rebase all stacks onto trunk and push updated bookmarks", + description: "Rebase tracked stacks onto trunk and push updated bookmarks", category: "workflow", }, run: restack, diff --git a/packages/core/src/commands/sync.ts b/packages/core/src/commands/sync.ts index be513ddf..5ff3b145 100644 --- a/packages/core/src/commands/sync.ts +++ b/packages/core/src/commands/sync.ts @@ -1,18 +1,11 @@ import type { Engine } from "../engine"; -import { - getTrunk, - list, - runJJ, - runJJWithMutableConfigVoid, - status, -} from "../jj"; +import { getTrunk, runJJ, runJJWithMutableConfigVoid, status } from "../jj"; import { ok, type Result } from "../result"; import { findMergedChanges, type MergedChange, updateStackComments, } from "../stacks"; -import type { AbandonedChange } from "../types"; import { syncPRInfo } from "./sync-pr-info"; import type { Command } from "./types"; @@ -20,9 +13,7 @@ interface SyncResult { fetched: boolean; rebased: boolean; hasConflicts: boolean; - merged: AbandonedChange[]; - empty: AbandonedChange[]; - /** Changes with merged PRs pending cleanup - caller should prompt before cleanup */ + /** Changes with merged/closed PRs pending cleanup - caller should prompt before cleanup */ pendingCleanup: MergedChange[]; updatedComments: number; stacksBehind: number; @@ -136,62 +127,56 @@ export async function sync(options: SyncOptions): Promise> { const trunk = await getTrunk(); await runJJ(["bookmark", "set", trunk, "-r", `${trunk}@origin`]); - // Rebase onto trunk (use mutable config for potentially pushed commits) - const rebaseResult = await runJJWithMutableConfigVoid([ - "rebase", - "-s", - "roots(trunk()..@)", - "-d", - "trunk()", - ]); + // Rebase WC onto new trunk if it's behind (empty WC on old trunk ancestor) + await runJJWithMutableConfigVoid(["rebase", "-r", "@", "-d", "trunk()"]); + + // Rebase only tracked bookmarks onto trunk (not all mutable commits) + // This prevents rebasing unrelated orphaned commits from the repo history + const trackedBookmarks = engine.getTrackedBookmarks(); + let rebaseOk = true; + let rebaseError: string | undefined; + + for (const bookmark of trackedBookmarks) { + // Only rebase if bookmark exists and is mutable + const checkResult = await runJJ([ + "log", + "-r", + `bookmarks(exact:"${bookmark}") & mutable()`, + "--no-graph", + "-T", + "change_id", + ]); + + if (checkResult.ok && checkResult.value.stdout.trim()) { + const result = await runJJWithMutableConfigVoid([ + "rebase", + "-b", + bookmark, + "-d", + "trunk()", + ]); + if (!result.ok) { + rebaseOk = false; + rebaseError = result.error.message; + break; + } + } + } // Check for conflicts let hasConflicts = false; - if (rebaseResult.ok) { + if (rebaseOk) { const statusResult = await status(); if (statusResult.ok) { hasConflicts = statusResult.value.workingCopy.hasConflicts; } } else { - hasConflicts = rebaseResult.error.message.includes("conflict"); - } - - // Find empty changes, but exclude the current working copy - const emptyResult = await list({ revset: "(trunk()..@) & empty() & ~@" }); - const abandoned: AbandonedChange[] = []; - - if (emptyResult.ok) { - for (const change of emptyResult.value) { - // If change has bookmarks, check cached PR info from engine - if (change.bookmarks.length > 0) { - const hasOpenPR = change.bookmarks.some((bookmark) => { - const meta = engine.getMeta(bookmark); - return meta?.prInfo?.state === "OPEN"; - }); - - // Skip if there's still an open PR - if (hasOpenPR) { - continue; - } - } - - const abandonResult = await runJJWithMutableConfigVoid([ - "abandon", - change.changeId, - ]); - if (abandonResult.ok) { - const reason = change.description.trim() !== "" ? "merged" : "empty"; - abandoned.push({ changeId: change.changeId, reason }); - } - } + hasConflicts = rebaseError?.includes("conflict") ?? false; } - // Clean up orphaned bookmarks + // Clean up orphaned bookmarks (bookmarks with no target) await cleanupOrphanedBookmarks(); - const merged = abandoned.filter((a) => a.reason === "merged"); - const empty = abandoned.filter((a) => a.reason === "empty"); - // Find changes with merged PRs - don't auto-cleanup, let caller prompt const mergedResult = await findMergedChanges(); const pendingCleanup = mergedResult.ok ? mergedResult.value : []; @@ -206,10 +191,8 @@ export async function sync(options: SyncOptions): Promise> { return ok({ fetched: true, - rebased: rebaseResult.ok, + rebased: rebaseOk, hasConflicts, - merged, - empty, pendingCleanup, updatedComments, stacksBehind, diff --git a/packages/core/src/stacks/merge.ts b/packages/core/src/stacks/merge.ts index 26505560..c10629d9 100644 --- a/packages/core/src/stacks/merge.ts +++ b/packages/core/src/stacks/merge.ts @@ -131,24 +131,19 @@ export async function mergeStack( } } - const baseUpdates: Promise>[] = []; - for (const prItem of prs) { - if (prItem.baseRefName !== trunk) { - baseUpdates.push(updatePR(prItem.prNumber, { base: trunk })); - } - } - if (baseUpdates.length > 0) { - const updateResults = await Promise.all(baseUpdates); - for (const result of updateResults) { - if (!result.ok) return result; - } - } - for (let i = 0; i < prs.length; i++) { const prItem = prs[i]; const nextPR = prs[i + 1]; callbacks?.onMerging?.(prItem, nextPR); + + // Update base to trunk right before merging this PR + // (Don't do this upfront for all PRs - that can cause GitHub to auto-close them) + if (prItem.baseRefName !== trunk) { + const baseUpdateResult = await updatePR(prItem.prNumber, { base: trunk }); + if (!baseUpdateResult.ok) return baseUpdateResult; + } + callbacks?.onWaiting?.(prItem); const mergeableResult = await waitForMergeable(prItem.prNumber, {