| author | |
| committer | |
| log | 6e4ca6ce9d04009af290412b77a0acb9be9f8a18 |
| tree | 26733d5aab81f7d59abd91d718967546c56ea4a5 |
| parent | e08c31e1a1fd5e89e2d4130b9ad18b44d15f92cd |
| signature | Signed by PGP key B5690EEEBB952194 |
6 files changed, 42 insertions(+), 24 deletions(-)
apps/server/src/git/GitManager.test.ts+30-2| ... | @@ -3743,7 +3743,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { | ... | @@ -3743,7 +3743,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { |
| 3743 | }), | 3743 | }), |
| 3744 | ); | 3744 | ); |
| 3745 | 3745 | ||
| 3746 | it.effect("create_pr pushes a clean branch before creating the PR when needed", () => | 3746 | it.effect("create_pr pushes committed changes while preserving a dirty worktree", () => |
| 3747 | Effect.gen(function* () { | 3747 | Effect.gen(function* () { |
| 3748 | const repoDir = yield* makeTempDir("t3code-git-manager-"); | 3748 | const repoDir = yield* makeTempDir("t3code-git-manager-"); |
| 3749 | yield* initRepo(repoDir); | 3749 | yield* initRepo(repoDir); |
| ... | @@ -3753,6 +3753,10 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { | ... | @@ -3753,6 +3753,10 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { |
| 3753 | NodeFS.writeFileSync(NodePath.join(repoDir, "create-pr-only.txt"), "create pr\n"); | 3753 | NodeFS.writeFileSync(NodePath.join(repoDir, "create-pr-only.txt"), "create pr\n"); |
| 3754 | yield* runGit(repoDir, ["add", "create-pr-only.txt"]); | 3754 | yield* runGit(repoDir, ["add", "create-pr-only.txt"]); |
| 3755 | yield* runGit(repoDir, ["commit", "-m", "Create PR only branch"]); | 3755 | yield* runGit(repoDir, ["commit", "-m", "Create PR only branch"]); |
| 3756 | const headBefore = (yield* runGit(repoDir, ["rev-parse", "HEAD"])).stdout.trim(); | ||
| 3757 | NodeFS.writeFileSync(NodePath.join(repoDir, "README.md"), "uncommitted readme\n"); | ||
| 3758 | NodeFS.writeFileSync(NodePath.join(repoDir, "untracked.txt"), "untracked work\n"); | ||
| 3759 | const statusBefore = (yield* runGit(repoDir, ["status", "--porcelain"])).stdout; | ||
| 3756 | 3760 | ||
| 3757 | const { manager, ghCalls } = yield* makeManager({ | 3761 | const { manager, ghCalls } = yield* makeManager({ |
| 3758 | ghScenario: { | 3762 | ghScenario: { |
| ... | @@ -3781,6 +3785,10 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { | ... | @@ -3781,6 +3785,10 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { |
| 3781 | expect(result.push.setUpstream).toBe(true); | 3785 | expect(result.push.setUpstream).toBe(true); |
| 3782 | expect(result.pr.status).toBe("created"); | 3786 | expect(result.pr.status).toBe("created"); |
| 3783 | expect(result.pr.number).toBe(303); | 3787 | expect(result.pr.number).toBe(303); |
| 3788 | expect((yield* runGit(repoDir, ["status", "--porcelain"])).stdout).toBe(statusBefore); | ||
| 3789 | expect( | ||
| 3790 | (yield* runGit(remoteDir, ["rev-parse", "feature/create-pr-only"])).stdout.trim(), | ||
| 3791 | ).toBe(headBefore); | ||
| 3784 | expect( | 3792 | expect( |
| 3785 | ghCalls.some((call) => | 3793 | ghCalls.some((call) => |
| 3786 | call.includes("pr create --base main --head feature/create-pr-only"), | 3794 | call.includes("pr create --base main --head feature/create-pr-only"), |
| ... | @@ -6199,7 +6207,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { | ... | @@ -6199,7 +6207,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { |
| 6199 | }), | 6207 | }), |
| 6200 | ); | 6208 | ); |
| 6201 | 6209 | ||
| 6202 | it.effect("create_pr emits only the PR phase when the branch is already pushed", () => | 6210 | it.effect("create_pr preserves dirty work on an already pushed branch", () => |
| 6203 | Effect.gen(function* () { | 6211 | Effect.gen(function* () { |
| 6204 | const repoDir = yield* makeTempDir("t3code-git-manager-"); | 6212 | const repoDir = yield* makeTempDir("t3code-git-manager-"); |
| 6205 | yield* initRepo(repoDir); | 6213 | yield* initRepo(repoDir); |
| ... | @@ -6210,8 +6218,22 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { | ... | @@ -6210,8 +6218,22 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { |
| 6210 | yield* runGit(repoDir, ["add", "pr-only.txt"]); | 6218 | yield* runGit(repoDir, ["add", "pr-only.txt"]); |
| 6211 | yield* runGit(repoDir, ["commit", "-m", "PR only branch"]); | 6219 | yield* runGit(repoDir, ["commit", "-m", "PR only branch"]); |
| 6212 | yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-only-follow-up"]); | 6220 | yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-only-follow-up"]); |
| 6221 | const headBefore = (yield* runGit(repoDir, ["rev-parse", "HEAD"])).stdout; | ||
| 6222 | NodeFS.writeFileSync(NodePath.join(repoDir, "README.md"), "staged readme\n"); | ||
| 6223 | yield* runGit(repoDir, ["add", "README.md"]); | ||
| 6224 | NodeFS.writeFileSync(NodePath.join(repoDir, "pr-only.txt"), "unstaged feature work\n"); | ||
| 6225 | NodeFS.writeFileSync(NodePath.join(repoDir, "untracked.txt"), "untracked work\n"); | ||
| 6226 | const statusBefore = (yield* runGit(repoDir, ["status", "--porcelain"])).stdout; | ||
| 6227 | const stagedBefore = (yield* runGit(repoDir, ["diff", "--cached"])).stdout; | ||
| 6228 | let generatedContent: TextGeneration.PrContentGenerationInput | undefined; | ||
| 6213 | 6229 | ||
| 6214 | const { manager } = yield* makeManager({ | 6230 | const { manager } = yield* makeManager({ |
| 6231 | textGeneration: { | ||
| 6232 | generatePrContent: (input) => { | ||
| 6233 | generatedContent = input; | ||
| 6234 | return Effect.succeed({ title: "PR only branch", body: "Committed feature work" }); | ||
| 6235 | }, | ||
| 6236 | }, | ||
| 6215 | ghScenario: { | 6237 | ghScenario: { |
| 6216 | prListSequence: [ | 6238 | prListSequence: [ |
| 6217 | JSON.stringify([]), | 6239 | JSON.stringify([]), |
| ... | @@ -6251,6 +6273,12 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { | ... | @@ -6251,6 +6273,12 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { |
| 6251 | expect(result.commit.status).toBe("skipped_not_requested"); | 6273 | expect(result.commit.status).toBe("skipped_not_requested"); |
| 6252 | expect(result.push.status).toBe("skipped_not_requested"); | 6274 | expect(result.push.status).toBe("skipped_not_requested"); |
| 6253 | expect(result.pr.status).toBe("created"); | 6275 | expect(result.pr.status).toBe("created"); |
| 6276 | expect(generatedContent?.diffPatch).toContain("+pr only"); | ||
| 6277 | expect(generatedContent?.diffPatch).not.toContain("staged readme"); | ||
| 6278 | expect(generatedContent?.diffPatch).not.toContain("unstaged feature work"); | ||
| 6279 | expect((yield* runGit(repoDir, ["rev-parse", "HEAD"])).stdout).toBe(headBefore); | ||
| 6280 | expect((yield* runGit(repoDir, ["status", "--porcelain"])).stdout).toBe(statusBefore); | ||
| 6281 | expect((yield* runGit(repoDir, ["diff", "--cached"])).stdout).toBe(stagedBefore); | ||
| 6254 | expect( | 6282 | expect( |
| 6255 | events.filter( | 6283 | events.filter( |
| 6256 | (event): event is Extract<GitActionProgressEvent, { kind: "phase_started" }> => | 6284 | (event): event is Extract<GitActionProgressEvent, { kind: "phase_started" }> => |
apps/server/src/git/GitManager.ts-8| ... | @@ -2730,14 +2730,6 @@ export const make = Effect.gen(function* () { | ... | @@ -2730,14 +2730,6 @@ export const make = Effect.gen(function* () { |
| 2730 | detail: "Feature-branch checkout is only supported for commit actions.", | 2730 | detail: "Feature-branch checkout is only supported for commit actions.", |
| 2731 | }); | 2731 | }); |
| 2732 | } | 2732 | } |
| 2733 | if (input.action === "create_pr" && initialStatus.hasWorkingTreeChanges) { | ||
| 2734 | return yield* new GitManagerError({ | ||
| 2735 | operation: "runStackedAction", | ||
| 2736 | cwd: input.cwd, | ||
| 2737 | detail: "Commit local changes before creating a PR.", | ||
| 2738 | }); | ||
| 2739 | } | ||
| 2740 | |||
| 2741 | const phases: GitActionProgressPhase[] = [ | 2733 | const phases: GitActionProgressPhase[] = [ |
| 2742 | ...(input.featureBranch ? (["branch"] as const) : []), | 2734 | ...(input.featureBranch ? (["branch"] as const) : []), |
| 2743 | ...(wantsCommit ? (["commit"] as const) : []), | 2735 | ...(wantsCommit ? (["commit"] as const) : []), |
apps/web/src/components/GitActionsControl.logic.test.ts+9-3| ... | @@ -365,7 +365,7 @@ describe("when: source control provider uses merge requests", () => { | ... | @@ -365,7 +365,7 @@ describe("when: source control provider uses merge requests", () => { |
| 365 | }); | 365 | }); |
| 366 | 366 | ||
| 367 | describe("when: ref is clean, up to date, and has no open PR", () => { | 367 | describe("when: ref is clean, up to date, and has no open PR", () => { |
| 368 | it("enables create PR when synced with upstream but ahead of default", () => { | 368 | it("enables create PR when synced with upstream but ahead of default, even with local changes", () => { |
| 369 | const syncedFeature = status({ | 369 | const syncedFeature = status({ |
| 370 | aheadCount: 0, | 370 | aheadCount: 0, |
| 371 | behindCount: 0, | 371 | behindCount: 0, |
| ... | @@ -383,6 +383,12 @@ describe("when: ref is clean, up to date, and has no open PR", () => { | ... | @@ -383,6 +383,12 @@ describe("when: ref is clean, up to date, and has no open PR", () => { |
| 383 | 383 | ||
| 384 | const items = buildMenuItems(syncedFeature, false); | 384 | const items = buildMenuItems(syncedFeature, false); |
| 385 | assert.equal(items.find((item) => item.id === "pr")?.disabled, false); | 385 | assert.equal(items.find((item) => item.id === "pr")?.disabled, false); |
| 386 | |||
| 387 | const dirtyFeature = { ...syncedFeature, hasWorkingTreeChanges: true }; | ||
| 388 | assert.equal( | ||
| 389 | buildMenuItems(dirtyFeature, false).find((item) => item.id === "pr")?.disabled, | ||
| 390 | false, | ||
| 391 | ); | ||
| 386 | }); | 392 | }); |
| 387 | 393 | ||
| 388 | it("resolveQuickAction returns disabled no-action state", () => { | 394 | it("resolveQuickAction returns disabled no-action state", () => { |
| ... | @@ -550,7 +556,7 @@ describe("when: working tree has local changes", () => { | ... | @@ -550,7 +556,7 @@ describe("when: working tree has local changes", () => { |
| 550 | ]); | 556 | ]); |
| 551 | }); | 557 | }); |
| 552 | 558 | ||
| 553 | it("buildMenuItems enables push for ahead commits while local changes remain uncommitted", () => { | 559 | it("buildMenuItems enables push and create PR for ahead commits while local changes remain uncommitted", () => { |
| 554 | const items = buildMenuItems( | 560 | const items = buildMenuItems( |
| 555 | status({ | 561 | status({ |
| 556 | refName: "feature/test", | 562 | refName: "feature/test", |
| ... | @@ -584,7 +590,7 @@ describe("when: working tree has local changes", () => { | ... | @@ -584,7 +590,7 @@ describe("when: working tree has local changes", () => { |
| 584 | { | 590 | { |
| 585 | id: "pr", | 591 | id: "pr", |
| 586 | label: "Create PR", | 592 | label: "Create PR", |
| 587 | disabled: true, | 593 | disabled: false, |
| 588 | icon: "pr", | 594 | icon: "pr", |
| 589 | kind: "open_dialog", | 595 | kind: "open_dialog", |
| 590 | dialogAction: "create_pr", | 596 | dialogAction: "create_pr", |
apps/web/src/components/GitActionsControl.logic.ts-1| ... | @@ -184,7 +184,6 @@ export function buildMenuItems( | ... | @@ -184,7 +184,6 @@ export function buildMenuItems( |
| 184 | const canCreatePr = | 184 | const canCreatePr = |
| 185 | !isBusy && | 185 | !isBusy && |
| 186 | hasBranch && | 186 | hasBranch && |
| 187 | !hasChanges && | ||
| 188 | !hasOpenPr && | 187 | !hasOpenPr && |
| 189 | hasDefaultBranchDelta && | 188 | hasDefaultBranchDelta && |
| 190 | !isBehind && | 189 | !isBehind && |
apps/web/src/components/GitActionsControl.tsx+1-4| ... | @@ -338,13 +338,10 @@ function getMenuActionDisabledReason({ | ... | @@ -338,13 +338,10 @@ function getMenuActionDisabledReason({ |
| 338 | if (!hasBranch) { | 338 | if (!hasBranch) { |
| 339 | return `Detached HEAD: check out a branch before creating a ${terminology.singular}.`; | 339 | return `Detached HEAD: check out a branch before creating a ${terminology.singular}.`; |
| 340 | } | 340 | } |
| 341 | if (hasChanges) { | ||
| 342 | return `Commit local changes before creating a ${terminology.singular}.`; | ||
| 343 | } | ||
| 344 | if (!gitStatus.hasUpstream && !hasPrimaryRemote) { | 341 | if (!gitStatus.hasUpstream && !hasPrimaryRemote) { |
| 345 | return `Add an "origin" remote before creating a ${terminology.singular}.`; | 342 | return `Add an "origin" remote before creating a ${terminology.singular}.`; |
| 346 | } | 343 | } |
| 347 | if (!isAhead) { | 344 | if ((gitStatus.aheadOfDefaultCount ?? gitStatus.aheadCount) <= 0) { |
| 348 | return `No local commits to include in a ${terminology.singular}.`; | 345 | return `No local commits to include in a ${terminology.singular}.`; |
| 349 | } | 346 | } |
| 350 | if (isBehind) { | 347 | if (isBehind) { |
packages/client-runtime/src/state/gitActions.ts+2-6| ... | @@ -65,9 +65,8 @@ export function buildMenuItems( | ... | @@ -65,9 +65,8 @@ export function buildMenuItems( |
| 65 | const canCreatePr = | 65 | const canCreatePr = |
| 66 | !isBusy && | 66 | !isBusy && |
| 67 | hasBranch && | 67 | hasBranch && |
| 68 | !hasChanges && | ||
| 69 | !hasOpenPr && | 68 | !hasOpenPr && |
| 70 | gitStatus.aheadCount > 0 && | 69 | (gitStatus.aheadOfDefaultCount ?? gitStatus.aheadCount) > 0 && |
| 71 | !isBehind && | 70 | !isBehind && |
| 72 | (gitStatus.hasUpstream || canPushWithoutUpstream); | 71 | (gitStatus.hasUpstream || canPushWithoutUpstream); |
| 73 | const canOpenPr = !isBusy && hasOpenPr; | 72 | const canOpenPr = !isBusy && hasOpenPr; |
| ... | @@ -292,13 +291,10 @@ export function getGitActionDisabledReason(input: { | ... | @@ -292,13 +291,10 @@ export function getGitActionDisabledReason(input: { |
| 292 | if (!hasBranch) { | 291 | if (!hasBranch) { |
| 293 | return "Detached HEAD: checkout a branch before creating a PR."; | 292 | return "Detached HEAD: checkout a branch before creating a PR."; |
| 294 | } | 293 | } |
| 295 | if (hasChanges) { | ||
| 296 | return "Commit local changes before creating a PR."; | ||
| 297 | } | ||
| 298 | if (!gitStatus.hasUpstream && !hasOriginRemote) { | 294 | if (!gitStatus.hasUpstream && !hasOriginRemote) { |
| 299 | return 'Add an "origin" remote before creating a PR.'; | 295 | return 'Add an "origin" remote before creating a PR.'; |
| 300 | } | 296 | } |
| 301 | if (!isAhead) { | 297 | if ((gitStatus.aheadOfDefaultCount ?? gitStatus.aheadCount) <= 0) { |
| 302 | return "No local commits to include in a PR."; | 298 | return "No local commits to include in a PR."; |
| 303 | } | 299 | } |
| 304 | if (isBehind) { | 300 | if (isBehind) { |