diff --git a/runtime/drivers/admin/repo_git.go b/runtime/drivers/admin/repo_git.go index 97e3546a24ea..ab1557791795 100644 --- a/runtime/drivers/admin/repo_git.go +++ b/runtime/drivers/admin/repo_git.go @@ -254,10 +254,10 @@ func (r *gitRepo) commitToDefaultBranch(ctx context.Context, message string, for _, err = r.commitAll(repo, message) if err != nil { - if errors.Is(err, git.ErrEmptyCommit) { - return nil // No changes to commit + if !errors.Is(err, git.ErrEmptyCommit) { + return fmt.Errorf("failed to commit changes to edit branch: %w", err) } - return fmt.Errorf("failed to commit changes to edit branch: %w", err) + // continue to push existing commits, if any } err = r.fetchCurrentBranch(ctx) diff --git a/web-admin/src/features/edit-session/CloudRemoteChangeManager.svelte b/web-admin/src/features/edit-session/CloudRemoteChangeManager.svelte new file mode 100644 index 000000000000..09c6279d1b3d --- /dev/null +++ b/web-admin/src/features/edit-session/CloudRemoteChangeManager.svelte @@ -0,0 +1,41 @@ + + + diff --git a/web-admin/src/features/edit-session/EditActions.svelte b/web-admin/src/features/edit-session/EditActions.svelte index eabfffb2be21..f2e0c6fae6c8 100644 --- a/web-admin/src/features/edit-session/EditActions.svelte +++ b/web-admin/src/features/edit-session/EditActions.svelte @@ -9,6 +9,7 @@ import { useRuntimeClient } from "@rilldata/web-common/runtime-client/v2"; import { GitBranch } from "lucide-svelte"; import CommitPopover from "./CommitPopover.svelte"; + import CloudRemoteChangeManager from "./CloudRemoteChangeManager.svelte"; import ExitButton from "./ExitButton.svelte"; import MergePopover from "./MergePopover.svelte"; import PublishPopover from "./PublishPopover.svelte"; @@ -49,6 +50,7 @@ {#if gitStatusLoaded} {#if managedGit} + {:else} diff --git a/web-admin/src/features/edit-session/PublishPopover.svelte b/web-admin/src/features/edit-session/PublishPopover.svelte index 6c16acbddb66..0b50fbe97459 100644 --- a/web-admin/src/features/edit-session/PublishPopover.svelte +++ b/web-admin/src/features/edit-session/PublishPopover.svelte @@ -11,21 +11,24 @@ import { Button } from "@rilldata/web-common/components/button"; import Tooltip from "@rilldata/web-common/components/tooltip/Tooltip.svelte"; import TooltipContent from "@rilldata/web-common/components/tooltip/TooltipContent.svelte"; + import { isMergeConflictError } from "@rilldata/web-common/features/project/deploy/github-utils.ts"; + import MergeConflictResolutionDialog from "@rilldata/web-common/features/project/MergeConflictResolutionDialog.svelte"; import { extractErrorMessage } from "@rilldata/web-common/lib/errors"; import { eventBus } from "@rilldata/web-common/lib/event-bus/event-bus"; import { queryClient } from "@rilldata/web-common/lib/svelte-query/globalQueryClient"; import { createRuntimeServiceGitMergeToBranchMutation, createRuntimeServiceGitPushMutation, - getRuntimeServiceGitStatusQueryKey, } from "@rilldata/web-common/runtime-client"; import { useRuntimeClient } from "@rilldata/web-common/runtime-client/v2"; + import type { ConnectError } from "@connectrpc/connect"; import { Rocket } from "lucide-svelte"; import { buildPostMergeUrl } from "./post-merge-url"; import { goto } from "$app/navigation"; import { fetchDeploymentGithubStatusChanges, getDeploymentGithubStatus, + invalidateGitStatusQueries, } from "@rilldata/web-admin/features/edit-session/selectors.ts"; export let organization: string; @@ -34,7 +37,20 @@ const AUTO_COMMIT_MESSAGE = "Updates from cloud editor"; + type PublishSnapshots = { + hadProdDeployment: boolean; + needsRedeploy: boolean; + preCommitSha: string | undefined; + }; + let isPublishing = false; + let publishMergeConflictDialog = false; + // Captured at click time so the publish flow can resume after a force + // merge without re-reading state that may have changed. `preCommitSha` is + // refreshed before completing the flow because prod's parser may have + // advanced while the user resolved the conflict. + let pendingPublishSnapshots: PublishSnapshots | null = null; + let errorFromGitCommand: ConnectError | null = null; const client = useRuntimeClient(); const gitPushMutation = createRuntimeServiceGitPushMutation(client); @@ -52,6 +68,7 @@ data: { hasLocalChanges, hasChangesOnCurrent, + hasRemoteChanges, alreadyOnPrimary, disabledPerGitStatus, }, @@ -76,6 +93,15 @@ async function handlePublish() { if (!primaryBranch || isPublishing) return; + + // If the remote has commits we don't have locally, stop and ask the + // shared dialog (owned by CloudRemoteChangeManager) to open via + // the bus. After a successful pull the user re-clicks Publish. + if (hasRemoteChanges) { + eventBus.emit("remote-changes-detected"); + return; + } + isPublishing = true; // Snapshot the prod deployment state at click time. Three relevant cases: @@ -87,15 +113,11 @@ // RedeployProject (admin/projects.go:333) handles cases 1 and 2 with a // single RPC: if there's no current deployment it creates one, otherwise // it provisions a fresh one and tears down the old. - const hadProdDeployment = !!prodDeployment; - const needsRedeploy = !prodDeploymentActive; - - // gitStatus tracks localChanges and currentBranch; the mutations below - // change both, so refresh once the flow finishes (success or failure). - const gitStatusQueryKey = getRuntimeServiceGitStatusQueryKey( - client.instanceId, - {}, - ); + const snapshots: PublishSnapshots = { + hadProdDeployment: !!prodDeployment, + needsRedeploy: !prodDeploymentActive, + preCommitSha: $parserShaQuery.data, + }; // Refetch local changes status, we predict this based on file watcher response. // But we dont check if changes flipped to with changes to without changes. @@ -113,6 +135,7 @@ return; } + let mergeResp; try { if (hasLocalChanges) { await $gitPushMutation.mutateAsync({ @@ -120,7 +143,7 @@ force: false, }); } - await $gitMergeMutation.mutateAsync({ + mergeResp = await $gitMergeMutation.mutateAsync({ branch: primaryBranch, force: false, }); @@ -132,12 +155,33 @@ isPublishing = false; return; } finally { - // Either gitPush or gitMerge may have changed localChanges or - // currentBranch (success or partial failure). Invalidate once. - void queryClient.invalidateQueries({ queryKey: gitStatusQueryKey }); + invalidateGitStatusQueries(client, primaryBranch); + } + + // GitMergeToBranch surfaces conflicts via `output` rather than an error; + // unhandled, the user would see a silent failure (the merge didn't land + // but the publish appears to have succeeded). Branch on it explicitly. + if (mergeResp?.output) { + if (isMergeConflictError(mergeResp.output)) { + pendingPublishSnapshots = snapshots; + errorFromGitCommand = null; + publishMergeConflictDialog = true; + } else { + eventBus.emit("notification", { + type: "error", + message: mergeResp.output, + }); + } + isPublishing = false; + return; } - if (needsRedeploy) { + await completePublishFlow(snapshots); + isPublishing = false; + } + + async function completePublishFlow(snapshots: PublishSnapshots) { + if (snapshots.needsRedeploy) { try { // TODO: detect billing/quota errors here and surface a friendly // message, mirroring `getPrettyDeployError` in @@ -171,7 +215,6 @@ detail ? `: ${detail}` : "" }.`, }); - isPublishing = false; return; } } @@ -180,8 +223,8 @@ organization, project, page: $page, - hadProdDeployment, - preCommitSha: $parserShaQuery.data, + hadProdDeployment: snapshots.hadProdDeployment, + preCommitSha: snapshots.preCommitSha, }); const targetWindow = window.open(targetUrl, "_blank"); if (!targetWindow) { @@ -192,7 +235,40 @@ message: "Pop-up was blocked.", }); } + } + + async function handleForcePublishMerge() { + errorFromGitCommand = null; + isPublishing = true; + let resp; + try { + resp = await $gitMergeMutation.mutateAsync({ + branch: primaryBranch, + force: true, + }); + } catch (err) { + errorFromGitCommand = err as ConnectError; + isPublishing = false; + return; + } finally { + invalidateGitStatusQueries(client, primaryBranch); + } + if (resp?.output) { + errorFromGitCommand = { message: resp.output } as ConnectError; + isPublishing = false; + return; + } + + publishMergeConflictDialog = false; + const snapshots = pendingPublishSnapshots; + pendingPublishSnapshots = null; + if (snapshots) { + // Prod's parser may have advanced while the user was on the conflict + // dialog; re-read so the deploying page waits past the correct SHA. + snapshots.preCommitSha = $parserShaQuery.data; + await completePublishFlow(snapshots); + } isPublishing = false; } @@ -216,6 +292,8 @@ Loading project... {:else if !hasLocalChanges} No changes to publish + {:else if hasRemoteChanges} + Remote has updates not in your session. Click to review. {:else if !prodDeployment} Publish your project to production. We'll open a new tab where you can invite teammates while the deployment reconciles. @@ -230,3 +308,10 @@ + + diff --git a/web-admin/src/features/edit-session/selectors.ts b/web-admin/src/features/edit-session/selectors.ts index d811e3e4eb1c..ea101ebe0cd7 100644 --- a/web-admin/src/features/edit-session/selectors.ts +++ b/web-admin/src/features/edit-session/selectors.ts @@ -1,3 +1,4 @@ +import { queryClient as globalQueryClient } from "@rilldata/web-common/lib/svelte-query/globalQueryClient.ts"; import { RuntimeClient } from "@rilldata/web-common/runtime-client/v2"; import { derived } from "svelte/store"; import { @@ -32,6 +33,8 @@ export function getDeploymentGithubStatus( data: { hasLocalChanges: false, hasChangesOnCurrent: false, + hasRemoteChanges: false, + hasLocalCommitsOnCurrent: false, alreadyOnPrimary: false, disabledPerGitStatus: true, }, @@ -49,6 +52,16 @@ export function getDeploymentGithubStatus( ); const hasLocalChanges = hasChangesAgainstCurrent || hasChangesOnCurrent; + // GitPull cascades primary -> current's remote -> local, so collapsing + // both signals into one is sufficient for the dialog flow. + const hasRemoteChanges = Boolean( + currentBranchGitStatusResp.data?.remoteCommits || + primaryBranchGitStatusResp.data?.remoteCommits, + ); + const hasLocalCommitsOnCurrent = Boolean( + currentBranchGitStatusResp.data?.localCommits, + ); + const alreadyOnPrimary = !!primaryBranch && !!currentBranch && currentBranch === primaryBranch; @@ -64,6 +77,8 @@ export function getDeploymentGithubStatus( data: { hasLocalChanges, hasChangesOnCurrent, + hasRemoteChanges, + hasLocalCommitsOnCurrent, alreadyOnPrimary, disabledPerGitStatus, }, @@ -99,3 +114,22 @@ export async function fetchDeploymentGithubStatusChanges( return hasChangesAgainstCurrent || hasChangesAgainstPrimary; } + +export function invalidateGitStatusQueries( + runtimeClient: RuntimeClient, + primaryBranch: string | undefined, +) { + // GitStatus is cached under two keys (see `getDeploymentGithubStatus`): + // one with no `remoteBranch` for the current branch and one keyed by the + // primary branch. Invalidate both so subscribers refetch. + void globalQueryClient.invalidateQueries({ + queryKey: getRuntimeServiceGitStatusQueryKey(runtimeClient.instanceId, {}), + }); + if (primaryBranch) { + void globalQueryClient.invalidateQueries({ + queryKey: getRuntimeServiceGitStatusQueryKey(runtimeClient.instanceId, { + remoteBranch: primaryBranch, + }), + }); + } +} diff --git a/web-common/src/features/project/RemoteProjectManager.svelte b/web-common/src/features/project/RemoteProjectManager.svelte index 19fed45d90c2..5fe3b01c35b9 100644 --- a/web-common/src/features/project/RemoteProjectManager.svelte +++ b/web-common/src/features/project/RemoteProjectManager.svelte @@ -1,125 +1,19 @@ - - - + diff --git a/web-common/src/features/project/RemoteSyncDialogs.svelte b/web-common/src/features/project/RemoteSyncDialogs.svelte new file mode 100644 index 000000000000..ffc94aa32439 --- /dev/null +++ b/web-common/src/features/project/RemoteSyncDialogs.svelte @@ -0,0 +1,145 @@ + + + + + diff --git a/web-common/src/features/project/deploy/github-utils.ts b/web-common/src/features/project/deploy/github-utils.ts index 5af788372821..0efb013d0a71 100644 --- a/web-common/src/features/project/deploy/github-utils.ts +++ b/web-common/src/features/project/deploy/github-utils.ts @@ -16,7 +16,12 @@ export function getGitUrlFromRemote(remote: string | undefined) { const MergeConflictsError = /Your local changes to the following files would be overwritten by merge/; +const PrimaryBranchMergeConflictError = + /use force pull to discard local changes and sync with (remote|primary)/; export function isMergeConflictError(errorMessage: string) { - return MergeConflictsError.test(errorMessage); + return ( + MergeConflictsError.test(errorMessage) || + PrimaryBranchMergeConflictError.test(errorMessage) + ); } diff --git a/web-common/src/lib/create-debouncer.ts b/web-common/src/lib/create-debouncer.ts index a94a15b91a9b..79631484f509 100644 --- a/web-common/src/lib/create-debouncer.ts +++ b/web-common/src/lib/create-debouncer.ts @@ -1,12 +1,18 @@ +type Debounced) => ReturnType> = (( + ...args: Parameters +) => void) & { cancel: () => void }; + export const debounce = ) => ReturnType>( fn: F, delay: number, -) => { +): Debounced => { let timeout: ReturnType; - return function (...args: Parameters) { + const debounced = function (...args: Parameters) { clearTimeout(timeout); timeout = setTimeout(() => { fn.apply(this, args); }, delay); - }; + } as Debounced; + debounced.cancel = () => clearTimeout(timeout); + return debounced; }; diff --git a/web-common/src/lib/event-bus/event-bus.ts b/web-common/src/lib/event-bus/event-bus.ts index b59e51ab0239..bd207140bce2 100644 --- a/web-common/src/lib/event-bus/event-bus.ts +++ b/web-common/src/lib/event-bus/event-bus.ts @@ -17,6 +17,7 @@ export interface Events { "page-content-resized": PageContentResized; "start-chat": string; "rill-yaml-updated": void; + "remote-changes-detected": void; } export const eventBus = new EventEmitter();