| author | |
| committer | |
| log | 0c637e3d5ec54b79b3df72b074b998ddc439b14a |
| tree | 66706b78fc46302c2a467bbc240f2455f01fc4d1 |
| parent | 0e2d50a62082aefb14249b85691cc500d4dd35dc |
- Compute ahead count from base branch when upstream is missing
- Skip push/PR actions when no local commits are ahead
- Update server and UI tests for no-upstream push/PR behavior6 files changed, 220 insertions(+), 28 deletions(-)
apps/server/src/git/Layers/GitCore.test.ts+36| ... | ... | @@ -1141,6 +1141,42 @@ it.layer(TestLayer)("git integration", (it) => { |
| 1141 | 1141 | }), |
| 1142 | 1142 | ); |
| 1143 | 1143 | |
| 1144 | it.effect("computes ahead count against base branch when no upstream is configured", () => | |
| 1145 | Effect.gen(function* () { | |
| 1146 | const tmp = yield* makeTmpDir(); | |
| 1147 | yield* initRepoWithCommit(tmp); | |
| 1148 | const core = yield* GitCore; | |
| 1149 | ||
| 1150 | yield* core.createBranch({ cwd: tmp, branch: "feature/no-upstream-ahead" }); | |
| 1151 | yield* core.checkoutBranch({ cwd: tmp, branch: "feature/no-upstream-ahead" }); | |
| 1152 | yield* writeTextFile(path.join(tmp, "feature.txt"), "ahead of base\n"); | |
| 1153 | yield* git(tmp, ["add", "feature.txt"]); | |
| 1154 | yield* git(tmp, ["commit", "-m", "feature commit"]); | |
| 1155 | ||
| 1156 | const details = yield* core.statusDetails(tmp); | |
| 1157 | expect(details.branch).toBe("feature/no-upstream-ahead"); | |
| 1158 | expect(details.hasUpstream).toBe(false); | |
| 1159 | expect(details.aheadCount).toBe(1); | |
| 1160 | expect(details.behindCount).toBe(0); | |
| 1161 | }), | |
| 1162 | ); | |
| 1163 | ||
| 1164 | it.effect("skips push when no upstream is configured and branch is not ahead of base", () => | |
| 1165 | Effect.gen(function* () { | |
| 1166 | const tmp = yield* makeTmpDir(); | |
| 1167 | yield* initRepoWithCommit(tmp); | |
| 1168 | const core = yield* GitCore; | |
| 1169 | ||
| 1170 | yield* core.createBranch({ cwd: tmp, branch: "feature/no-upstream-no-ahead" }); | |
| 1171 | yield* core.checkoutBranch({ cwd: tmp, branch: "feature/no-upstream-no-ahead" }); | |
| 1172 | ||
| 1173 | const pushed = yield* core.pushCurrentBranch(tmp, null); | |
| 1174 | expect(pushed.status).toBe("skipped_up_to_date"); | |
| 1175 | expect(pushed.branch).toBe("feature/no-upstream-no-ahead"); | |
| 1176 | expect(pushed.setUpstream).toBeUndefined(); | |
| 1177 | }), | |
| 1178 | ); | |
| 1179 | ||
| 1144 | 1180 | it.effect("includes command context when worktree removal fails", () => |
| 1145 | 1181 | Effect.gen(function* () { |
| 1146 | 1182 | const tmp = yield* makeTmpDir(); |
apps/server/src/git/Layers/GitCore.ts+93-1| ... | ... | @@ -7,6 +7,7 @@ import { GitCore, type GitCoreShape } from "../Services/GitCore.ts"; |
| 7 | 7 | const STATUS_UPSTREAM_REFRESH_INTERVAL = Duration.seconds(15); |
| 8 | 8 | const STATUS_UPSTREAM_REFRESH_TIMEOUT = Duration.seconds(5); |
| 9 | 9 | const STATUS_UPSTREAM_REFRESH_CACHE_CAPACITY = 2_048; |
| 10 | const DEFAULT_BASE_BRANCH_CANDIDATES = ["main", "master"] as const; | |
| 10 | 11 | |
| 11 | 12 | class StatusUpstreamRefreshCacheKey extends Data.Class<{ |
| 12 | 13 | cwd: string; |
| ... | ... | @@ -161,6 +162,16 @@ function commandLabel(args: readonly string[]): string { |
| 161 | 162 | return `git ${args.join(" ")}`; |
| 162 | 163 | } |
| 163 | 164 | |
| 165 | function parseDefaultBranchFromRemoteHeadRef(value: string): string | null { | |
| 166 | const trimmed = value.trim(); | |
| 167 | const prefix = "refs/remotes/origin/"; | |
| 168 | if (!trimmed.startsWith(prefix)) { | |
| 169 | return null; | |
| 170 | } | |
| 171 | const branch = trimmed.slice(prefix.length).trim(); | |
| 172 | return branch.length > 0 ? branch : null; | |
| 173 | } | |
| 174 | ||
| 164 | 175 | function createGitCommandError( |
| 165 | 176 | operation: string, |
| 166 | 177 | cwd: string, |
| ... | ... | @@ -340,6 +351,79 @@ const makeGitCore = Effect.gen(function* () { |
| 340 | 351 | yield* fetchUpstreamRef(cwd, upstream); |
| 341 | 352 | }); |
| 342 | 353 | |
| 354 | const resolveDefaultBranchName = (cwd: string): Effect.Effect<string | null, GitCommandError> => | |
| 355 | executeGit("GitCore.resolveDefaultBranchName", cwd, ["symbolic-ref", "refs/remotes/origin/HEAD"], { | |
| 356 | allowNonZeroExit: true, | |
| 357 | }).pipe( | |
| 358 | Effect.map((result) => { | |
| 359 | if (result.code !== 0) { | |
| 360 | return null; | |
| 361 | } | |
| 362 | return parseDefaultBranchFromRemoteHeadRef(result.stdout); | |
| 363 | }), | |
| 364 | ); | |
| 365 | ||
| 366 | const branchExists = (cwd: string, branch: string): Effect.Effect<boolean, GitCommandError> => | |
| 367 | executeGit("GitCore.branchExists", cwd, ["show-ref", "--verify", "--quiet", `refs/heads/${branch}`], { | |
| 368 | allowNonZeroExit: true, | |
| 369 | }).pipe(Effect.map((result) => result.code === 0)); | |
| 370 | ||
| 371 | const resolveBaseBranchForNoUpstream = ( | |
| 372 | cwd: string, | |
| 373 | branch: string, | |
| 374 | ): Effect.Effect<string | null, GitCommandError> => | |
| 375 | Effect.gen(function* () { | |
| 376 | const configuredBaseBranch = yield* runGitStdout( | |
| 377 | "GitCore.resolveBaseBranchForNoUpstream.config", | |
| 378 | cwd, | |
| 379 | ["config", "--get", `branch.${branch}.gh-merge-base`], | |
| 380 | true, | |
| 381 | ).pipe(Effect.map((stdout) => stdout.trim())); | |
| 382 | ||
| 383 | const defaultBranch = yield* resolveDefaultBranchName(cwd); | |
| 384 | const candidates = [ | |
| 385 | configuredBaseBranch.length > 0 ? configuredBaseBranch : null, | |
| 386 | defaultBranch, | |
| 387 | ...DEFAULT_BASE_BRANCH_CANDIDATES, | |
| 388 | ]; | |
| 389 | ||
| 390 | for (const candidate of candidates) { | |
| 391 | if (!candidate || candidate === branch) { | |
| 392 | continue; | |
| 393 | } | |
| 394 | ||
| 395 | if (yield* branchExists(cwd, candidate)) { | |
| 396 | return candidate; | |
| 397 | } | |
| 398 | } | |
| 399 | ||
| 400 | return null; | |
| 401 | }); | |
| 402 | ||
| 403 | const computeAheadCountAgainstBase = ( | |
| 404 | cwd: string, | |
| 405 | branch: string, | |
| 406 | ): Effect.Effect<number, GitCommandError> => | |
| 407 | Effect.gen(function* () { | |
| 408 | const baseBranch = yield* resolveBaseBranchForNoUpstream(cwd, branch); | |
| 409 | if (!baseBranch) { | |
| 410 | return 0; | |
| 411 | } | |
| 412 | ||
| 413 | const result = yield* executeGit( | |
| 414 | "GitCore.computeAheadCountAgainstBase", | |
| 415 | cwd, | |
| 416 | ["rev-list", "--count", `${baseBranch}..HEAD`], | |
| 417 | { allowNonZeroExit: true }, | |
| 418 | ); | |
| 419 | if (result.code !== 0) { | |
| 420 | return 0; | |
| 421 | } | |
| 422 | ||
| 423 | const parsed = Number.parseInt(result.stdout.trim(), 10); | |
| 424 | return Number.isFinite(parsed) ? Math.max(0, parsed) : 0; | |
| 425 | }); | |
| 426 | ||
| 343 | 427 | const readBranchRecency = (cwd: string): Effect.Effect<Map<string, number>, GitCommandError> => |
| 344 | 428 | Effect.gen(function* () { |
| 345 | 429 | const branchRecency = yield* executeGit( |
| ... | ... | @@ -429,6 +513,14 @@ const makeGitCore = Effect.gen(function* () { |
| 429 | 513 | if (pathValue) changedFilesWithoutNumstat.add(pathValue); |
| 430 | 514 | } |
| 431 | 515 | } |
| 516 | ||
| 517 | if (!upstreamRef && branch) { | |
| 518 | aheadCount = yield* computeAheadCountAgainstBase(cwd, branch).pipe( | |
| 519 | Effect.catch(() => Effect.succeed(0)), | |
| 520 | ); | |
| 521 | behindCount = 0; | |
| 522 | } | |
| 523 | ||
| 432 | 524 | const stagedEntries = parseNumstatEntries(stagedNumstatStdout); |
| 433 | 525 | const unstagedEntries = parseNumstatEntries(unstagedNumstatStdout); |
| 434 | 526 | const fileStatMap = new Map<string, { insertions: number; deletions: number }>(); |
| ... | ... | @@ -538,7 +630,7 @@ const makeGitCore = Effect.gen(function* () { |
| 538 | 630 | ); |
| 539 | 631 | } |
| 540 | 632 | |
| 541 | if (details.hasUpstream && details.aheadCount === 0 && details.behindCount === 0) { | |
| 633 | if (details.aheadCount === 0 && details.behindCount === 0) { | |
| 542 | 634 | return { |
| 543 | 635 | status: "skipped_up_to_date" as const, |
| 544 | 636 | branch, |
apps/server/src/git/Layers/GitManager.test.ts+50| ... | ... | @@ -551,6 +551,56 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { |
| 551 | 551 | }), |
| 552 | 552 | ); |
| 553 | 553 | |
| 554 | it.effect( | |
| 555 | "pushes and creates PR from a no-upstream branch when local commits are ahead of base", | |
| 556 | () => | |
| 557 | Effect.gen(function* () { | |
| 558 | const repoDir = yield* makeTempDir("t3code-git-manager-"); | |
| 559 | yield* initRepo(repoDir); | |
| 560 | yield* runGit(repoDir, ["checkout", "-b", "feature/no-upstream-pr"]); | |
| 561 | const remoteDir = yield* createBareRemote(); | |
| 562 | yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); | |
| 563 | fs.writeFileSync(path.join(repoDir, "feature.txt"), "feature\n"); | |
| 564 | ||
| 565 | const { manager, ghCalls } = yield* makeManager({ | |
| 566 | ghScenario: { | |
| 567 | prListSequence: [ | |
| 568 | "[]", | |
| 569 | JSON.stringify([ | |
| 570 | { | |
| 571 | number: 77, | |
| 572 | title: "Add no-upstream PR flow", | |
| 573 | url: "https://github.com/pingdotgg/codething-mvp/pull/77", | |
| 574 | baseRefName: "main", | |
| 575 | headRefName: "feature/no-upstream-pr", | |
| 576 | }, | |
| 577 | ]), | |
| 578 | ], | |
| 579 | }, | |
| 580 | }); | |
| 581 | ||
| 582 | const result = yield* runStackedAction(manager, { | |
| 583 | cwd: repoDir, | |
| 584 | action: "commit_push_pr", | |
| 585 | }); | |
| 586 | ||
| 587 | expect(result.commit.status).toBe("created"); | |
| 588 | expect(result.push.status).toBe("pushed"); | |
| 589 | expect(result.push.setUpstream).toBe(true); | |
| 590 | expect(result.pr.status).toBe("created"); | |
| 591 | expect( | |
| 592 | yield* runGit(repoDir, ["rev-parse", "--abbrev-ref", "@{upstream}"]).pipe( | |
| 593 | Effect.map((result) => result.stdout.trim()), | |
| 594 | ), | |
| 595 | ).toBe("origin/feature/no-upstream-pr"); | |
| 596 | expect( | |
| 597 | ghCalls.some((call) => | |
| 598 | call.includes("pr create --base main --head feature/no-upstream-pr"), | |
| 599 | ), | |
| 600 | ).toBe(true); | |
| 601 | }), | |
| 602 | ); | |
| 603 | ||
| 554 | 604 | it.effect("skips push when branch is already up to date", () => |
| 555 | 605 | Effect.gen(function* () { |
| 556 | 606 | const repoDir = yield* makeTempDir("t3code-git-manager-"); |
apps/web/src/components/GitActionsControl.logic.test.ts+29-11| ... | ... | @@ -515,20 +515,20 @@ describe("when: HEAD is detached and there are no local changes", () => { |
| 515 | 515 | }); |
| 516 | 516 | |
| 517 | 517 | describe("when: branch has no upstream configured", () => { |
| 518 | it("resolveQuickAction runs push when clean, no upstream, and no local commits are ahead", () => { | |
| 518 | it("resolveQuickAction is disabled when clean, no upstream, and no local commits are ahead", () => { | |
| 519 | 519 | const quick = resolveQuickAction( |
| 520 | 520 | status({ hasUpstream: false, pr: null, aheadCount: 0 }), |
| 521 | 521 | false, |
| 522 | 522 | ); |
| 523 | 523 | assert.deepInclude(quick, { |
| 524 | kind: "run_action", | |
| 525 | action: "commit_push", | |
| 524 | kind: "show_hint", | |
| 526 | 525 | label: "Push", |
| 527 | disabled: false, | |
| 526 | hint: "No local commits to push.", | |
| 527 | disabled: true, | |
| 528 | 528 | }); |
| 529 | 529 | }); |
| 530 | 530 | |
| 531 | it("resolveQuickAction runs push when clean and no upstream even if PR metadata exists", () => { | |
| 531 | it("resolveQuickAction opens PR when clean, no upstream, no local commits are ahead, and PR exists", () => { | |
| 532 | 532 | const quick = resolveQuickAction( |
| 533 | 533 | status({ |
| 534 | 534 | hasUpstream: false, |
| ... | ... | @@ -545,9 +545,8 @@ describe("when: branch has no upstream configured", () => { |
| 545 | 545 | false, |
| 546 | 546 | ); |
| 547 | 547 | assert.deepInclude(quick, { |
| 548 | kind: "run_action", | |
| 549 | action: "commit_push", | |
| 550 | label: "Push", | |
| 548 | kind: "open_pr", | |
| 549 | label: "Open PR", | |
| 551 | 550 | disabled: false, |
| 552 | 551 | }); |
| 553 | 552 | }); |
| ... | ... | @@ -576,7 +575,7 @@ describe("when: branch has no upstream configured", () => { |
| 576 | 575 | }); |
| 577 | 576 | }); |
| 578 | 577 | |
| 579 | it("buildMenuItems enables push but keeps create PR disabled when no commits are ahead", () => { | |
| 578 | it("buildMenuItems disables push and create PR when no commits are ahead", () => { | |
| 580 | 579 | const items = buildMenuItems( |
| 581 | 580 | status({ hasUpstream: false, pr: null, aheadCount: 0 }), |
| 582 | 581 | false, |
| ... | ... | @@ -593,7 +592,7 @@ describe("when: branch has no upstream configured", () => { |
| 593 | 592 | { |
| 594 | 593 | id: "push", |
| 595 | 594 | label: "Push", |
| 596 | disabled: false, | |
| 595 | disabled: true, | |
| 597 | 596 | icon: "push", |
| 598 | 597 | kind: "open_dialog", |
| 599 | 598 | dialogAction: "push", |
| ... | ... | @@ -659,7 +658,7 @@ describe("when: branch has no upstream configured", () => { |
| 659 | 658 | ]); |
| 660 | 659 | }); |
| 661 | 660 | |
| 662 | it("resolveQuickAction uses push-only on default branch when no upstream exists", () => { | |
| 661 | it("resolveQuickAction is disabled on default branch when no upstream exists and no commits are ahead", () => { | |
| 663 | 662 | const quick = resolveQuickAction( |
| 664 | 663 | status({ |
| 665 | 664 | branch: "main", |
| ... | ... | @@ -670,6 +669,25 @@ describe("when: branch has no upstream configured", () => { |
| 670 | 669 | false, |
| 671 | 670 | true, |
| 672 | 671 | ); |
| 672 | assert.deepInclude(quick, { | |
| 673 | kind: "show_hint", | |
| 674 | label: "Push", | |
| 675 | hint: "No local commits to push.", | |
| 676 | disabled: true, | |
| 677 | }); | |
| 678 | }); | |
| 679 | ||
| 680 | it("resolveQuickAction uses push-only on default branch when no upstream exists and commits are ahead", () => { | |
| 681 | const quick = resolveQuickAction( | |
| 682 | status({ | |
| 683 | branch: "main", | |
| 684 | hasUpstream: false, | |
| 685 | aheadCount: 1, | |
| 686 | pr: null, | |
| 687 | }), | |
| 688 | false, | |
| 689 | true, | |
| 690 | ); | |
| 673 | 691 | assert.deepInclude(quick, { |
| 674 | 692 | kind: "run_action", |
| 675 | 693 | action: "commit_push", |
apps/web/src/components/GitActionsControl.logic.ts+12-10| ... | ... | @@ -111,14 +111,8 @@ export function buildMenuItems( |
| 111 | 111 | const hasChanges = gitStatus.hasWorkingTreeChanges; |
| 112 | 112 | const hasOpenPr = gitStatus.pr?.state === "open"; |
| 113 | 113 | const isBehind = gitStatus.behindCount > 0; |
| 114 | const canPublishUntrackedBranch = !gitStatus.hasUpstream; | |
| 115 | 114 | const canCommit = !isBusy && hasChanges; |
| 116 | const canPush = | |
| 117 | !isBusy && | |
| 118 | hasBranch && | |
| 119 | !hasChanges && | |
| 120 | !isBehind && | |
| 121 | (gitStatus.aheadCount > 0 || canPublishUntrackedBranch); | |
| 115 | const canPush = !isBusy && hasBranch && !hasChanges && !isBehind && gitStatus.aheadCount > 0; | |
| 122 | 116 | const canCreatePr = |
| 123 | 117 | !isBusy && |
| 124 | 118 | hasBranch && |
| ... | ... | @@ -211,10 +205,18 @@ export function resolveQuickAction( |
| 211 | 205 | } |
| 212 | 206 | |
| 213 | 207 | if (!gitStatus.hasUpstream) { |
| 214 | if (hasOpenPr || isDefaultBranch) { | |
| 215 | return { label: "Push", disabled: false, kind: "run_action", action: "commit_push" }; | |
| 216 | } | |
| 217 | 208 | if (!isAhead) { |
| 209 | if (hasOpenPr) { | |
| 210 | return { label: "Open PR", disabled: false, kind: "open_pr" }; | |
| 211 | } | |
| 212 | return { | |
| 213 | label: "Push", | |
| 214 | disabled: true, | |
| 215 | kind: "show_hint", | |
| 216 | hint: "No local commits to push.", | |
| 217 | }; | |
| 218 | } | |
| 219 | if (hasOpenPr || isDefaultBranch) { | |
| 218 | 220 | return { label: "Push", disabled: false, kind: "run_action", action: "commit_push" }; |
| 219 | 221 | } |
| 220 | 222 | return { |
apps/web/src/components/GitActionsControl.tsx-6| ... | ... | @@ -87,9 +87,6 @@ function getMenuActionDisabledReason( |
| 87 | 87 | if (isBehind) { |
| 88 | 88 | return "Branch is behind upstream. Pull/rebase before pushing."; |
| 89 | 89 | } |
| 90 | if (!gitStatus.hasUpstream) { | |
| 91 | return "Branch has no upstream. Push to publish it."; | |
| 92 | } | |
| 93 | 90 | if (!isAhead) { |
| 94 | 91 | return "No local commits to push."; |
| 95 | 92 | } |
| ... | ... | @@ -105,9 +102,6 @@ function getMenuActionDisabledReason( |
| 105 | 102 | if (hasChanges) { |
| 106 | 103 | return "Commit local changes before creating a PR."; |
| 107 | 104 | } |
| 108 | if (!gitStatus.hasUpstream) { | |
| 109 | return "Set an upstream branch before creating a PR."; | |
| 110 | } | |
| 111 | 105 | if (!isAhead) { |
| 112 | 106 | return "No local commits to include in a PR."; |
| 113 | 107 | } |