diff --git a/src/server/providers/git/mutations.ts b/src/server/providers/git/mutations.ts index da8b465..3fa210e 100644 --- a/src/server/providers/git/mutations.ts +++ b/src/server/providers/git/mutations.ts @@ -254,6 +254,13 @@ export class GitMutations { ); } + private async isAncestor(dir: string, ancestor: string, descendant: string): Promise { + const { stdout } = await this.git.run(['merge-base', ancestor, descendant], { cwd: dir }).catch(() => ({ + stdout: '', + })); + return stdout.trim() === ancestor; + } + private async fetchState(input: GitBranchInput): Promise { const dir = input.cloneDir; const { stdout: cached } = await this.git.run( @@ -275,7 +282,16 @@ export class GitMutations { { cwd: dir }, ); const remoteHeadSha = remote.trim(); - const localHeadSha = (await this.retainedHead(dir, input.branch)) ?? cached.trim(); + const retained = await this.retainedHead(dir, input.branch); + let localHeadSha = retained ?? cached.trim(); + // A pushed retained head is no longer local work: once a fetch has seen newer remote commits on top + // of it, continue from the fetched head (as without a retained commit) instead of blocking forever. + if ( + retained && + (await this.isAncestor(dir, retained, remoteHeadSha)) && + !(await this.isAncestor(dir, cached.trim(), retained)) + ) + localHeadSha = cached.trim(); const { stdout } = await this.git.run( ['rev-list', '--left-right', '--count', `${localHeadSha}...${remoteHeadSha}`], { cwd: dir }, diff --git a/src/ui/source/changes-view.tsx b/src/ui/source/changes-view.tsx index f93e510..ee4fbd0 100644 --- a/src/ui/source/changes-view.tsx +++ b/src/ui/source/changes-view.tsx @@ -287,6 +287,13 @@ export function ChangesView(props: { expectedRemoteSha: next.expectedRemoteSha, branch: next.branch, }); + } else if (next.operation !== 'push' && next.state && next.state.ahead > 0 && next.state.behind === 0) { + // An unpushed commit from an earlier visit: offer Push again (the server re-checks heads before pushing). + setRetained({ + commitSha: next.state.localHeadSha, + expectedRemoteSha: next.state.remoteHeadSha, + branch: next.state.branch, + }); } if (next.status === 'push_succeeded') setRetained(null); if (next.status === 'warnings_unacknowledged') setWarningAckScope(null); diff --git a/tests/e2e/changes-commit.spec.ts b/tests/e2e/changes-commit.spec.ts index 05e184b..cd289fe 100644 --- a/tests/e2e/changes-commit.spec.ts +++ b/tests/e2e/changes-commit.spec.ts @@ -261,6 +261,65 @@ test('mocked push success shows branch confirmation', async ({ page }) => { await page.unroute(`**/repositories/${repositoryId}/git/commit`); }); +test('an unpushed commit from an earlier visit can still be pushed', async ({ page }) => { + test.skip(isMobile(page), 'Retained push recovery runs once on desktop.'); + await setGitIdentity(page); + const marker = `retained-${Date.now()}`; + const { repositoryId } = sample(); + await openChangesWithDraft(page, marker); + + const local = 'f'.repeat(40); + const remote = '1'.repeat(40); + await page.route(`**/repositories/${repositoryId}/git/commit`, async (route) => { + if (route.request().method() !== 'POST') return route.continue(); + await route.fulfill({ + status: 409, + contentType: 'application/json', + body: JSON.stringify({ + status: 'draft_outdated', + repositoryId, + branch: 'main', + operation: 'inspect', + commitSha: null, + expectedRemoteSha: remote, + state: { branch: 'main', localHeadSha: local, remoteHeadSha: remote, ahead: 1, behind: 0 }, + problems: [], + outdatedPaths: [], + draftsPreserved: true, + }), + }); + }); + let pushed: unknown = null; + await page.route(`**/repositories/${repositoryId}/git/push`, async (route) => { + pushed = route.request().postDataJSON(); + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + status: 'push_succeeded', + repositoryId, + branch: 'main', + operation: 'push', + commitSha: local, + expectedRemoteSha: remote, + state: { branch: 'main', localHeadSha: local, remoteHeadSha: local, ahead: 0, behind: 0 }, + problems: [], + outdatedPaths: [], + draftsPreserved: true, + }), + }); + }); + + await page.getByLabel('Commit message').fill(`e2e: ${marker}`); + await page.getByRole('button', { name: 'Commit', exact: true }).click(); + await page.getByRole('button', { name: 'Push fffffff' }).click(); + await expect(page.getByRole('status').filter({ hasText: /Pushed fffffff to main/ })).toBeVisible(); + expect(pushed).toEqual({ commitSha: local, expectedRemoteSha: remote }); + await expectWorkPreserved(page); + await page.unroute(`**/repositories/${repositoryId}/git/commit`); + await page.unroute(`**/repositories/${repositoryId}/git/push`); +}); + test('mobile: changes commit layout and accessibility', async ({ page }) => { test.skip(!isMobile(page), 'Narrow layout only.'); await setGitIdentity(page); diff --git a/tests/integration/git-workflow.test.ts b/tests/integration/git-workflow.test.ts index 4b0a565..ca17add 100644 --- a/tests/integration/git-workflow.test.ts +++ b/tests/integration/git-workflow.test.ts @@ -1,3 +1,4 @@ +import { execFileSync } from 'node:child_process'; import fs from 'node:fs/promises'; import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -171,6 +172,27 @@ describe('draft commit and safe push workflow', () => { ).toMatchObject({ meta: { reason: 'remote_changed' } }); }); + it('continues committing after a pushed commit is built upon elsewhere', async () => { + await save(); + const pushed = await commit({ paths: ['compose.yaml'], message: 'First', push: true }); + expect(pushed.json.status).toBe('push_succeeded'); + // Someone else pushes on top of the published commit (for example from a laptop). + execFileSync('git', ['pull', '-q', '--ff-only', 'origin', 'main'], { + cwd: path.join(server.root, `.work-${REPO}`), + env: { ...process.env, GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null' }, + }); + const remote = server.commit(REPO, 'main', 'README.md', '# Remote\n'); + await save('notes.md', '# Notes\n'); + const changed = await commit({ paths: ['notes.md'], message: 'Notes' }); + expect(changed.json).toMatchObject({ status: 'remote_changed', state: { remoteHeadSha: remote } }); + + await h.container.repositories.sync(repositoryId); + const next = await commit({ paths: ['notes.md'], message: 'Notes' }); + expect(next.json.status).toBe('commit_succeeded'); + expect(await git(['rev-parse', `${next.json.commitSha}^`])).toBe(remote + '\n'); + expect((await push(next.json.commitSha, remote)).json.status).toBe('push_succeeded'); + }); + it('preserves a newer draft saved while the selected snapshot is being committed', async () => { await save(); const original = provider().commit.bind(provider());