diff --git a/apps/mobile/src/features/review/reviewModel.ts b/apps/mobile/src/features/review/reviewModel.ts index 1b2131c73e0c1..21b57322da1cd 100644 --- a/apps/mobile/src/features/review/reviewModel.ts +++ b/apps/mobile/src/features/review/reviewModel.ts @@ -534,17 +534,22 @@ export function buildReviewSectionItems(input: { }, ); - const gitItems = input.gitSections.map((section) => ({ - id: `git:${section.kind}`, - kind: section.kind, - title: section.title, - subtitle: gitSubtitle(section), - diff: section.diff, - source: section, - ...(section.files ? { files: section.files } : {}), - truncated: section.truncated, - isLoading: false, - })); + const gitItems = input.gitSections + .filter( + (section): section is typeof section & { kind: ReviewSectionKind } => + section.kind !== "commit", + ) + .map((section) => ({ + id: `git:${section.kind}`, + kind: section.kind, + title: section.title, + subtitle: gitSubtitle(section), + diff: section.diff, + source: section, + ...(section.files ? { files: section.files } : {}), + truncated: section.truncated, + isLoading: false, + })); const hasChangesItem = gitItems.some((item) => item.id === CHANGES_SECTION_ID); const visibleGitItems = input.loadingGitSections && !hasChangesItem diff --git a/apps/server/src/auth/RpcAuthorization.ts b/apps/server/src/auth/RpcAuthorization.ts index 3806262cced53..bf2b8d49daf8d 100644 --- a/apps/server/src/auth/RpcAuthorization.ts +++ b/apps/server/src/auth/RpcAuthorization.ts @@ -170,6 +170,7 @@ export const RPC_REQUIRED_SCOPES = { [WS_METHODS.vcsRefreshStatus]: AuthOrchestrationReadScope, [WS_METHODS.gitResolvePullRequest]: AuthOrchestrationReadScope, [WS_METHODS.vcsListRefs]: AuthOrchestrationReadScope, + [WS_METHODS.reviewGetBranchCommits]: AuthFilesystemReadScope, [WS_METHODS.reviewGetDiffPreview]: AuthFilesystemReadScope, [WS_METHODS.reviewGetDiffFileContents]: AuthFilesystemReadScope, [WS_METHODS.terminalOpen]: AuthTerminalOperateScope, diff --git a/apps/server/src/observability/RpcInstrumentation.ts b/apps/server/src/observability/RpcInstrumentation.ts index 028ea5a561d44..cb8b0b6a86cf3 100644 --- a/apps/server/src/observability/RpcInstrumentation.ts +++ b/apps/server/src/observability/RpcInstrumentation.ts @@ -181,6 +181,7 @@ const RPC_AGGREGATES = { [WS_METHODS.vcsCreateRef]: "vcs", [WS_METHODS.vcsSwitchRef]: "vcs", [WS_METHODS.vcsInit]: "vcs", + [WS_METHODS.reviewGetBranchCommits]: "review", [WS_METHODS.reviewGetDiffPreview]: "review", [WS_METHODS.reviewGetDiffFileContents]: "review", [WS_METHODS.terminalOpen]: "terminal", diff --git a/apps/server/src/review/ReviewService.ts b/apps/server/src/review/ReviewService.ts index 8f88b387bd454..39ce1554cea61 100644 --- a/apps/server/src/review/ReviewService.ts +++ b/apps/server/src/review/ReviewService.ts @@ -8,6 +8,8 @@ import * as Path from "effect/Path"; import { VcsRepositoryDetectionError, VcsUnsupportedOperationError, + type ReviewBranchCommitsInput, + type ReviewBranchCommitsResult, type ReviewDiffFileContentsInput, type ReviewDiffFileContentsResult, type ReviewDiffPreviewError, @@ -20,9 +22,18 @@ import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; import * as KnownWorkspaces from "../workspace/KnownWorkspaces.ts"; +const WORKSPACE_BOUND_SUBJECTS = { + "ReviewService.getBranchCommits": "commit list", + "ReviewService.getDiffPreview": "diff preview", + "ReviewService.getDiffFileContents": "diff file contents", +} as const; + export class ReviewService extends Context.Service< ReviewService, { + readonly getBranchCommits: ( + input: ReviewBranchCommitsInput, + ) => Effect.Effect; readonly getDiffPreview: ( input: ReviewDiffPreviewInput, ) => Effect.Effect; @@ -61,7 +72,7 @@ export const make = Effect.gen(function* () { }; const assertWorkspaceBoundCwd = Effect.fn("ReviewService.assertWorkspaceBoundCwd")(function* ( - operation: "ReviewService.getDiffPreview" | "ReviewService.getDiffFileContents", + operation: keyof typeof WORKSPACE_BOUND_SUBJECTS, cwd: string, ) { const [candidate, workspaceRoot] = yield* Effect.all([ @@ -78,13 +89,27 @@ export const make = Effect.gen(function* () { return yield* new VcsRepositoryDetectionError({ operation, cwd, - detail: - operation === "ReviewService.getDiffPreview" - ? "Review diff preview cwd must stay within the configured workspace root." - : "Review diff file contents cwd must stay within the configured workspace root.", + detail: `Review ${WORKSPACE_BOUND_SUBJECTS[operation]} cwd must stay within the configured workspace root.`, }); }); + const getBranchCommits: ReviewService["Service"]["getBranchCommits"] = Effect.fn( + "ReviewService.getBranchCommits", + )(function* (input) { + yield* assertWorkspaceBoundCwd("ReviewService.getBranchCommits", input.cwd); + + const handle = yield* vcsRegistry.detect({ cwd: input.cwd, requestedKind: "auto" }); + if (!handle) return { commits: [], truncated: false }; + if (handle.kind !== "git") { + return yield* new VcsUnsupportedOperationError({ + operation: "ReviewService.getBranchCommits", + kind: handle.kind, + detail: "Review commit lists require a Git repository.", + }); + } + return yield* git.getReviewBranchCommits(input); + }); + const getDiffPreview: ReviewService["Service"]["getDiffPreview"] = Effect.fn( "ReviewService.getDiffPreview", )(function* (input) { @@ -120,6 +145,7 @@ export const make = Effect.gen(function* () { }); return ReviewService.of({ + getBranchCommits, getDiffPreview, getDiffFileContents, }); diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 95833200a1b5f..74e409a8bc184 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -20,6 +20,8 @@ import { type VcsCreateRefResult, type VcsCreateWorktreeInput, type VcsCreateWorktreeResult, + type ReviewBranchCommitsInput, + type ReviewBranchCommitsResult, type ReviewDiffPreviewInput, type ReviewDiffPreviewResult, type ReviewDiffFileContentsInput, @@ -328,6 +330,9 @@ export class GitVcsDriver extends Context.Service< cwd: string, baseRef: string, ) => Effect.Effect; + readonly getReviewBranchCommits: ( + input: ReviewBranchCommitsInput, + ) => Effect.Effect; readonly getReviewDiffPreview: ( input: ReviewDiffPreviewInput, ) => Effect.Effect; diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index d05a41a6f3824..cd4b6949fad86 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -1,5 +1,7 @@ // @effect-diagnostics nodeBuiltinImport:off - realpathSync.native resolves Windows 8.3 short names, which the Effect realPath does not. import * as NodeFS from "node:fs"; +import * as NodeOS from "node:os"; +import * as NodeURL from "node:url"; import * as NodeServices from "@effect/platform-node/NodeServices"; import * as HostProcess from "@t3tools/shared/HostProcess"; import { assert, it, describe } from "@effect/vitest"; @@ -1466,6 +1468,140 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }); describe("review diff previews", () => { + it.effect("diffs a root commit against the empty tree but rejects a shallow boundary", () => + Effect.gen(function* () { + const upstream = yield* makeTmpDir(); + yield* initRepoWithCommit(upstream); + const driver = yield* GitVcsDriver.GitVcsDriver; + const emptyTree = yield* git(upstream, ["hash-object", "-t", "tree", NodeOS.devNull]); + const root = yield* git(upstream, ["rev-parse", "HEAD"]); + const rootSource = (yield* driver.getReviewDiffPreview({ cwd: upstream, commitSha: root })) + .sources[0]!; + assert.equal(rootSource.baseRef, emptyTree); + assert.include(rootSource.diff, "+# test"); + + yield* writeTextFile(upstream, "README.md", "# boundary\n"); + yield* git(upstream, ["commit", "-am", "boundary"]); + const sha = yield* git(upstream, ["rev-parse", "HEAD"]); + const cwd = yield* makeTmpDir(); + yield* git(upstream, ["clone", "--depth=1", NodeURL.pathToFileURL(upstream).href, cwd], { + GIT_CONFIG_GLOBAL: NodeOS.devNull, + GIT_CONFIG_NOSYSTEM: "1", + }); + assert.equal(yield* git(cwd, ["rev-parse", "--is-shallow-repository"]), "true"); + for (const request of [ + driver.getReviewDiffPreview({ cwd, commitSha: sha }), + driver.getReviewDiffFileContents( + makeReviewDiffFileContentsInput(cwd, { + sourceKind: "commit", + changeType: "new", + headRef: sha, + baseRef: emptyTree, + }), + ), + ]) { + const error = yield* Effect.asVoid(request).pipe( + Effect.flip, + ); + assert.include(error.detail, "parent"); + } + }), + ); + + it.effect("lists the branch's commits newest first, at most 100, and flags the rest", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* git(cwd, ["checkout", "-b", "feature/commits"]); + for (let index = 0; index < 102; index++) { + yield* git(cwd, ["commit", "--allow-empty", "-m", `feature ${index}`]); + } + + const result = yield* driver.getReviewBranchCommits({ cwd, baseRef: initialBranch }); + assert.strictEqual(result.truncated, true); + assert.strictEqual(result.commits.length, 100); + assert.strictEqual(result.commits[0]?.sha, yield* git(cwd, ["rev-parse", "HEAD"])); + assert.deepStrictEqual( + [result.commits[0]?.subject, result.commits[99]?.subject], + ["feature 101", "feature 2"], + ); + }), + ); + + it.effect("previews one selected commit, a merge against its first parent", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* git(cwd, ["checkout", "-b", "feature/merge-preview"]); + yield* git(cwd, ["checkout", "-b", "feature/side"]); + yield* writeTextFile(cwd, "README.md", "# side commit\n"); + yield* git(cwd, ["add", "README.md"]); + yield* git(cwd, ["commit", "-m", "side change"]); + yield* git(cwd, ["checkout", "feature/merge-preview"]); + yield* writeTextFile(cwd, "feature.txt", "feature commit\n"); + yield* git(cwd, ["add", "feature.txt"]); + yield* git(cwd, ["commit", "-m", "feature change"]); + const firstParent = yield* git(cwd, ["rev-parse", "HEAD"]); + yield* git(cwd, ["merge", "--no-ff", "--no-edit", "feature/side"]); + const mergeSha = yield* git(cwd, ["rev-parse", "HEAD"]); + yield* git(cwd, ["checkout", "-b", "feature/elsewhere", initialBranch]); + yield* writeTextFile(cwd, "elsewhere.txt", "elsewhere\n"); + yield* git(cwd, ["add", "elsewhere.txt"]); + yield* git(cwd, ["commit", "-m", "elsewhere change"]); + const elsewhereSha = yield* git(cwd, ["rev-parse", "HEAD"]); + yield* git(cwd, ["checkout", "feature/merge-preview"]); + yield* writeTextFile(cwd, "README.md", "# dirty tree\n"); + + const elsewhere = yield* driver.getReviewDiffPreview({ cwd, commitSha: elsewhereSha }); + assert.deepStrictEqual( + elsewhere.sources + .find((candidate) => candidate.kind === "commit") + ?.files?.map((file) => file.path), + ["elsewhere.txt"], + ); + + const preview = yield* driver.getReviewDiffPreview({ + cwd, + baseRef: initialBranch, + commitSha: mergeSha, + }); + const source = preview.sources.find((candidate) => candidate.kind === "commit")!; + assert.strictEqual(source.baseRef, firstParent); + assert.strictEqual(source.headRef, mergeSha); + assert.deepStrictEqual( + source.files?.map((file) => file.path), + ["README.md"], + ); + assert.include(source.diff, "+# side commit"); + assert.notInclude(source.diff, "dirty tree"); + assert.notInclude(source.diff, "feature.txt"); + + const patch = yield* driver.getReviewDiffPreview({ + cwd, + baseRef: initialBranch, + commitSha: mergeSha, + file: { path: "README.md", previousPath: null, sourceKind: "commit" }, + }); + assert.include( + patch.sources.find((candidate) => candidate.kind === "commit")!.diff, + "+# side commit", + ); + const contents = yield* driver.getReviewDiffFileContents( + makeReviewDiffFileContentsInput(cwd, { + sourceKind: "commit", + baseRef: source.baseRef, + headRef: source.headRef, + }), + ); + assert.deepStrictEqual(contents, { + oldContents: "# test\n", + newContents: "# side commit\n", + }); + }), + ); + it.effect("loads repository-relative files from a nested project directory", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 3a7c99fbc3d31..c5bdf3d068ef9 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -26,6 +26,7 @@ import { GitCommandError, type GitCommandFailureReason, T3_PROJECT_FILE_NAME, + type ReviewBranchCommitsInput, type ReviewDiffFileContentsInput, type ReviewDiffPreviewInput, type ReviewDiffFileStat, @@ -74,6 +75,9 @@ const REVIEW_METADATA_MAX_OUTPUT_BYTES = 16 * 1024 * 1024; // Temporary-index git add scales poorly (50k files took ~30 s); report incomplete totals above this cap. const REVIEW_UNTRACKED_MAX_FILES = 5_000; const REVIEW_DIFF_FILE_MAX_OUTPUT_BYTES = 1024 * 1024; +const REVIEW_BRANCH_COMMIT_LIMIT = 100; +const REVIEW_BRANCH_SUBJECT_LIMIT = 300; +const REVIEW_BRANCH_COMMIT_MAX_OUTPUT_BYTES = 128 * 1024; // Client patch parsers require a/ and b/ prefixes; override user diff prefix settings. export const PATCH_RENDER_PREFIX_ARGS = ["--src-prefix=a/", "--dst-prefix=b/"] as const; const REVIEW_DIFF_ARGS = [ @@ -3115,6 +3119,141 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* return { baseRef, insertions, deletions }; }, Effect.scoped); + const reviewCommitError = (cwd: string, detail: string) => + new GitCommandError({ + operation: "GitVcsDriver.reviewCommit", + command: "git cat-file", + cwd, + detail, + }); + + const resolveReviewCommit = Effect.fn("resolveReviewCommit")(function* ( + cwd: string, + sha: string, + ) { + if (!/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/i.test(sha)) { + return yield* reviewCommitError(cwd, "Selected commit is invalid or no longer available."); + } + const present = yield* executeGit( + "GitVcsDriver.reviewCommit.present", + cwd, + ["cat-file", "-e", `${sha}^{commit}`], + { allowNonZeroExit: true, env: { GIT_NO_LAZY_FETCH: "1" } }, + ); + if (present.exitCode !== 0) { + return yield* reviewCommitError(cwd, "Selected commit is invalid or no longer available."); + } + // The raw object keeps the parents a shallow clone grafts away, so a boundary is not a root. + const object = yield* runGitStdout("GitVcsDriver.reviewCommit.parents", cwd, [ + "cat-file", + "commit", + sha, + ]); + const secondLine = object.split("\n", 2)[1]; + const firstParent = secondLine?.startsWith("parent ") ? secondLine.slice(7) : undefined; + if (firstParent !== undefined) { + const parent = yield* executeGit( + "GitVcsDriver.reviewCommit.parent", + cwd, + ["cat-file", "-e", `${firstParent}^{commit}`], + { allowNonZeroExit: true, env: { GIT_NO_LAZY_FETCH: "1" } }, + ); + if (parent.exitCode !== 0) { + return yield* reviewCommitError(cwd, "Selected commit's parent is not available locally."); + } + } + return { sha: sha.toLowerCase(), parent: firstParent ?? (yield* readEmptyTreeHash(cwd)) }; + }); + + const readReviewCommitMessage = Effect.fn("GitVcsDriver.readReviewCommitMessage")(function* ( + cwd: string, + sha: string, + ) { + const result = yield* executeGit( + "GitVcsDriver.readReviewCommitMessage", + cwd, + ["show", "-s", "--no-notes", "--no-show-signature", "--format=%B", sha], + { maxOutputBytes: 8_192, appendTruncationMarker: true }, + ); + const endOfSubject = result.stdout.indexOf("\n"); + return { + subject: result.stdout.slice(0, endOfSubject < 0 ? undefined : endOfSubject).slice(0, 300), + }; + }); + + const getReviewBranchCommits = Effect.fn("getReviewBranchCommits")(function* ( + input: ReviewBranchCommitsInput, + ) { + const repository = yield* resolveRepositoryPathsUncached(input.cwd); + if (!repository?.worktreeRoot) return { commits: [], truncated: false }; + const cwd = repository.worktreeRoot; + const head = yield* executeGit( + "GitVcsDriver.getReviewBranchCommits.head", + cwd, + ["rev-parse", "--verify", "--quiet", "HEAD"], + { allowNonZeroExit: true }, + ); + if (head.exitCode !== 0) { + if (head.exitCode === 1 && head.stderr.trim() === "") { + return { commits: [], truncated: false }; + } + return yield* reviewCommitError(cwd, "Could not resolve the current branch commit."); + } + const { mergeBase } = yield* resolveReviewMergeBase( + cwd, + repository.currentBranch, + input.baseRef, + ); + const result = yield* executeGit( + "GitVcsDriver.getReviewBranchCommits", + cwd, + [ + "log", + "-z", + "--no-notes", + "--no-show-signature", + "--format=%H%x00%s%x00%cI", + `--max-count=${REVIEW_BRANCH_COMMIT_LIMIT + 1}`, + `${mergeBase}..HEAD`, + ], + { + maxOutputBytes: REVIEW_BRANCH_COMMIT_MAX_OUTPUT_BYTES, + appendTruncationMarker: true, + }, + ); + if (!result.stdoutTruncated && result.stdout.length > 0 && !result.stdout.endsWith("\0")) { + return yield* reviewCommitError(cwd, "Could not read branch commit metadata."); + } + const fields = result.stdout.split("\0"); + const completeFieldCount = fields.length - 1; + const commits: Array<{ sha: string; subject: string; committedAt: string }> = []; + for ( + let index = 0; + index + 2 < completeFieldCount && commits.length <= REVIEW_BRANCH_COMMIT_LIMIT; + index += 3 + ) { + const sha = fields[index]; + const subject = fields[index + 1]; + const committedAt = fields[index + 2]; + if ( + !sha || + subject === undefined || + !committedAt || + !/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/i.test(sha) + ) { + return yield* reviewCommitError(cwd, "Could not read branch commit metadata."); + } + commits.push({ sha, subject: subject.slice(0, REVIEW_BRANCH_SUBJECT_LIMIT), committedAt }); + } + if (result.stdoutTruncated && commits.length === 0) { + return yield* reviewCommitError(cwd, "Branch commit metadata exceeds the response limit."); + } + return { + commits: commits.slice(0, REVIEW_BRANCH_COMMIT_LIMIT), + truncated: result.stdoutTruncated || commits.length > REVIEW_BRANCH_COMMIT_LIMIT, + }; + }); + const getReviewDiffPreview = Effect.fn("getReviewDiffPreview")(function* ( input: ReviewDiffPreviewInput, ) { @@ -3141,6 +3280,64 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* } const cwd = repository.worktreeRoot; + const hashDiff = (diff: string, files: ReadonlyArray) => + crypto.digest("SHA-256", new TextEncoder().encode(JSON.stringify([diff, files]))).pipe( + Effect.map(Hex.encode), + Effect.mapError( + (cause) => + new GitCommandError({ + operation: "GitVcsDriver.getReviewDiffPreview.hash", + command: "crypto.digest SHA-256", + cwd, + detail: "Failed to hash review diff.", + cause, + }), + ), + ); + if (input.commitSha !== undefined || input.file?.sourceKind === "commit") { + if (!input.commitSha || (input.file && input.file.sourceKind !== "commit")) { + return yield* reviewCommitError(cwd, "Selected commit request is invalid."); + } + const commit = yield* resolveReviewCommit(cwd, input.commitSha); + const { subject } = yield* readReviewCommitMessage(cwd, commit.sha); + const diffArgs = [ + ...REVIEW_DIFF_ARGS, + ...(input.ignoreWhitespace ? ["--ignore-all-space"] : []), + ]; + const stat = yield* executeGit( + "GitVcsDriver.getReviewDiffPreview.commitStat", + cwd, + [...diffArgs, "--numstat", "-z", commit.parent, commit.sha, "--", ...pathArgs], + { maxOutputBytes: REVIEW_METADATA_MAX_OUTPUT_BYTES }, + ); + const files = parseReviewNumstat(stat.stdout); + const patch = yield* executeGit( + "GitVcsDriver.getReviewDiffPreview.commitPatch", + cwd, + [...diffArgs, "--patch", commit.parent, commit.sha, "--", ...pathArgs], + { maxOutputBytes: patchLimit, appendTruncationMarker: true }, + ); + const diffHash = yield* hashDiff(patch.stdout, files); + return { + cwd: input.cwd, + generatedAt: yield* DateTime.now, + sources: [ + { + id: commit.sha, + kind: "commit" as const, + title: `Commit ${commit.sha.slice(0, 7)}`, + ...(subject.trim() ? { commitSubject: subject.trim() } : {}), + baseRef: commit.parent, + headRef: commit.sha, + diff: patch.stdout, + files, + diffHash, + truncated: patch.stdoutTruncated, + }, + ], + }; + } + const dirtyRef = input.file?.sourceKind === "branch-range" ? null : "HEAD"; const review = input.file?.sourceKind === "working-tree" @@ -3220,20 +3417,6 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const baseFiles = baseResult.files; const dirtyDiff = dirtyTrackedResult.stdout; const baseDiff = baseResult.stdout; - const hashDiff = (diff: string, files: ReadonlyArray) => - crypto.digest("SHA-256", new TextEncoder().encode(JSON.stringify([diff, files]))).pipe( - Effect.map(Hex.encode), - Effect.mapError( - (cause) => - new GitCommandError({ - operation: "GitVcsDriver.getReviewDiffPreview.hash", - command: "crypto.digest SHA-256", - cwd, - detail: "Failed to hash review diff.", - cause, - }), - ), - ); const [dirtyDiffHash, baseDiffHash] = yield* Effect.all([ hashDiff(dirtyDiff, dirtyFiles ?? []), hashDiff(baseDiff, baseFiles ?? []), @@ -3385,6 +3568,30 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* if (repositoryRoot.length === 0) { return yield* reviewDiffFileError(input, "Could not resolve the Git repository root."); } + if (input.sourceKind === "commit") { + if (!input.headRef) { + return yield* reviewDiffFileError(input, "Selected commit is no longer available."); + } + const commit = yield* resolveReviewCommit(input.cwd, input.headRef); + if (input.baseRef !== commit.parent) { + return yield* reviewDiffFileError( + input, + "Selected commit changed. Refresh the comparison.", + ); + } + const [oldContents, newContents] = yield* Effect.all( + [ + input.changeType === "new" + ? Effect.succeed("") + : readReviewFileAtRevision(input, commit.parent, input.oldPath), + input.changeType === "deleted" + ? Effect.succeed("") + : readReviewFileAtRevision(input, commit.sha, input.newPath), + ], + { concurrency: 2 }, + ); + return { oldContents, newContents }; + } const oldRevision = input.sourceKind === "working-tree" ? (input.baseRef ?? "HEAD") @@ -4649,6 +4856,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* return yield* withListRefsInvalidation(cwd, pullCurrentBranch(cwd)); }), readRangeContext, + getReviewBranchCommits, getReviewDiffPreview, getReviewDiffFileContents, readConfigValue, diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 32737aa771df6..befad768da3e5 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -3001,6 +3001,7 @@ export const layerWsRpc = ( gitWorkflow.switchRef(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))), [WS_METHODS.vcsInit]: (input) => vcsProvisioning.initRepository(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))), + [WS_METHODS.reviewGetBranchCommits]: (input) => review.getBranchCommits(input), [WS_METHODS.reviewGetDiffPreview]: (input) => review.getDiffPreview(input), [WS_METHODS.reviewGetDiffFileContents]: (input) => review.getDiffFileContents(input), [WS_METHODS.terminalOpen]: (input) => terminalManager.open(input), diff --git a/apps/web/src/browser/browserLinkTarget.ts b/apps/web/src/browser/browserLinkTarget.ts index 50ff1a4e351fc..f1c4e65a99913 100644 --- a/apps/web/src/browser/browserLinkTarget.ts +++ b/apps/web/src/browser/browserLinkTarget.ts @@ -1,6 +1,11 @@ -import type { BrowserLinkTarget, ScopedThreadRef } from "@t3tools/contracts"; +import { + AuthPreviewOperateScope, + type BrowserLinkTarget, + type ScopedThreadRef, +} from "@t3tools/contracts"; import { ensureClientSettingsHydrated, getClientSettings } from "~/hooks/useSettings"; +import { readEnvironmentScope } from "~/state/session"; import { isPreviewAvailableFor } from "./previewRuntime"; export interface ResolveLinkTargetInput { @@ -38,5 +43,9 @@ export async function resolveBrowserLinkTargetPreference(): Promise { const explicitThreadRef = explicitDiffOpenRef.current; explicitDiffOpenRef.current = null; - if (diffOpen && activeThreadRef && explicitThreadRef !== activeThreadRef) { + if ( + diffOpen && + activeThreadRef && + explicitThreadRef !== activeThreadRef && + !useDiffPanelStore.getState().consumeKeepSelectionOnOpen(activeThreadRef) + ) { useDiffPanelStore.getState().selectGitScope(activeThreadRef, "branch"); } }, [activeThreadRef, diffOpen]); diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index ddce50db3cfe3..ef95c24d578a2 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -6,7 +6,7 @@ import { squashAtomCommandFailure, } from "@t3tools/client-runtime/state/runtime"; import { safeErrorLogAttributes } from "@t3tools/client-runtime/errors"; -import type { ScopedThreadRef, RunId } from "@t3tools/contracts"; +import type { ReviewBranchCommitsResult, ScopedThreadRef, RunId } from "@t3tools/contracts"; import { ArrowRightIcon, CheckIcon, @@ -20,7 +20,7 @@ import { } from "lucide-react"; import { ChevronDown, ChevronRight, ChevronsDownUp, ChevronsUpDown } from "lucide"; import * as Schema from "effect/Schema"; -import { useEffect, useRef, useState } from "react"; +import { type MouseEvent, useEffect, useRef, useState } from "react"; import { useCodeViewFileReveal } from "./diffs/useCodeViewFileReveal"; import { useFilesystemReadAccess } from "~/state/filesystem"; import { useOpenInPreferredEditor } from "../editorPreferences"; @@ -33,6 +33,7 @@ import { revisionedQuery } from "~/state/revisionedQuery"; import { cn } from "~/lib/utils"; import { resolveToolCallDiffTarget, + selectThreadBranchBaseRef, selectThreadDiffPanelSelection, useDiffPanelSessionStore, useDiffPanelStore, @@ -60,7 +61,7 @@ import { useQuietThreadVisibleTurnItems, } from "../state/entities"; import { useClientSettings, updateClientSettings } from "../hooks/useSettings"; -import { formatShortTimestamp } from "../timestampFormat"; +import { formatRelativeTimeLabel, formatShortTimestamp } from "../timestampFormat"; import { DiffFilePathCopyButton } from "./DiffFilePathCopyButton"; import { DiffPanelLoadingState, DiffPanelShell, type DiffPanelMode } from "./DiffPanelShell"; import { DiffStatLabel } from "./chat/DiffStatLabel"; @@ -83,6 +84,7 @@ import { import { DropdownMenu, DropdownMenuContent, + DropdownMenuItem, DropdownMenuRadioGroup, DropdownMenuRadioItem, DropdownMenuSub, @@ -103,6 +105,8 @@ import { useReviewFilePatches } from "./diffs/useReviewFilePatches"; import { DiffFileLoadingBoundary } from "./diffs/DiffFileLoadingBoundary"; import { DiffFileStatus } from "./diffs/DiffFileStatus"; import { useThreadRouteRef } from "../hooks/useThreadRouteTarget"; +import { projectCommitUrl } from "@t3tools/client-runtime/change-request-links"; +import { useOpenLink } from "../browser/useOpenLink"; type DiffThemeType = "light" | "dark"; const AUTOMATIC_BASE_REF = "__automatic_base_ref__"; @@ -223,6 +227,8 @@ export default function DiffPanel({ ); const [hideTestFiles, setHideTestFiles] = useHideTestFiles(); const [baseRefQuery, setBaseRefQuery] = useState(""); + // The open commit menu keeps the list it opened with, so rows do not move under the pointer. + const [openCommitMenu, setOpenCommitMenu] = useState(null); const [codeViewRevision, setCodeViewRevision] = useState(0); const routeThreadRef = useThreadRouteRef(); @@ -263,8 +269,14 @@ export default function DiffPanel({ const diffSelection = useDiffPanelStore((state) => selectThreadDiffPanelSelection(state.byThreadKey, routeThreadRef), ); + const rememberedBaseRef = useDiffPanelStore((state) => + selectThreadBranchBaseRef(state, routeThreadRef), + ); const toolCallSelection = diffSelection.kind === "tool-call" ? diffSelection : null; - const isGitSelection = diffSelection.kind === "branch" || diffSelection.kind === "unstaged"; + const isGitSelection = + diffSelection.kind === "branch" || + diffSelection.kind === "unstaged" || + diffSelection.kind === "commit"; const toolCallItems = useQuietThreadVisibleTurnItems(toolCallSelection ? routeThreadRef : null); const toolCallDetail = useTurnItemDetail( toolCallSelection && routeThreadRef @@ -294,8 +306,15 @@ export default function DiffPanel({ }, [diffSelection, orderedTurnDiffSummaries, routeThreadRef]); const selectedRunId = diffSelection.kind === "turn" ? diffSelection.turnId : null; - const selectedGitScope = diffSelection.kind === "unstaged" ? "unstaged" : "branch"; - const selectedBaseRef = diffSelection.kind === "branch" ? diffSelection.baseRef : null; + const selectedGitScope = + diffSelection.kind === "unstaged" || diffSelection.kind === "commit" + ? diffSelection.kind + : "branch"; + const selectedCommitSha = diffSelection.kind === "commit" ? diffSelection.sha : null; + const selectedBaseRef = + diffSelection.kind === "branch" || diffSelection.kind === "commit" + ? diffSelection.baseRef + : null; const selectedFilePath = diffSelection.kind === "turn" ? diffSelection.filePath : null; const selectedFileRevealRequestId = diffSelection.kind === "turn" ? diffSelection.revealRequestId : 0; @@ -306,31 +325,17 @@ export default function DiffPanel({ orderedTurnDiffSummaries[0]); const selectedCheckpointTurnCount = selectedTurn?.checkpointTurnCount; const latestTurn = orderedTurnDiffSummaries[0]; - const selectedScopeLabel = toolCallSelection - ? "Tool call" - : selectedRunId === null - ? selectedGitScope === "unstaged" - ? "Uncommitted" - : "Changes" - : selectedTurn?.runId === latestTurn?.runId - ? "Latest turn" - : `Turn ${selectedCheckpointTurnCount ?? "?"}`; const reviewSectionId = toolCallSelection ? `tool-call:${toolCallSelection.threadId}:${toolCallSelection.itemId}` : selectedTurn ? `turn:${selectedTurn.runId}` - : selectedGitScope; + : selectedCommitSha !== null + ? `commit:${selectedCommitSha}` + : selectedGitScope; const collapseScopeKey = routeThreadRef ? `${routeThreadRef.environmentId}:${routeThreadRef.threadId}:${reviewSectionId}` : null; const codeViewMountKey = `${collapseScopeKey ?? reviewSectionId}:${codeViewRevision}`; - const reviewSectionTitle = toolCallSelection - ? "Tool call" - : selectedTurn - ? `Turn ${selectedCheckpointTurnCount ?? "?"}` - : selectedGitScope === "unstaged" - ? "Uncommitted" - : "Changes"; const selectedCheckpointRange = typeof selectedCheckpointTurnCount === "number" ? { @@ -354,6 +359,7 @@ export default function DiffPanel({ ? { cwd: activeCwd, ...(selectedBaseRef ? { baseRef: selectedBaseRef } : {}), + ...(selectedCommitSha !== null ? { commitSha: selectedCommitSha } : {}), ignoreWhitespace: diffIgnoreWhitespace, } : undefined; @@ -372,13 +378,69 @@ export default function DiffPanel({ ) : null, ); + const branchCommitsQuery = useEnvironmentQuery( + visible && isGitRepo && canReadFiles && activeThread && activeCwd + ? reviewEnvironment.branchCommits({ + environmentId: activeThread.environmentId, + input: { + cwd: activeCwd, + ...(rememberedBaseRef ? { baseRef: rememberedBaseRef } : {}), + }, + }) + : null, + ); + const branchCommits = openCommitMenu ?? branchCommitsQuery.data; const activeThreadRefreshKey = routeThreadRef ? `${routeThreadRef.environmentId}:${routeThreadRef.threadId}` : null; const selectedGitSource = branchDiffPreview.data?.sources.find( - (source) => source.kind === (selectedGitScope === "unstaged" ? "working-tree" : "branch-range"), + (source) => + source.kind === + (selectedGitScope === "unstaged" + ? "working-tree" + : selectedGitScope === "commit" + ? "commit" + : "branch-range"), ); + const selectedCommitSubject = + selectedCommitSha !== null && selectedGitSource?.headRef === selectedCommitSha + ? selectedGitSource.commitSubject + : undefined; + const selectedGitScopeLabel = + selectedCommitSha !== null + ? (selectedCommitSubject ?? `Commit ${selectedCommitSha.slice(0, 7)}`) + : selectedGitScope === "unstaged" + ? "Uncommitted" + : "Changes"; + const selectedScopeLabel = toolCallSelection + ? "Tool call" + : selectedRunId === null + ? selectedCommitSha !== null + ? (selectedCommitSubject ?? "Commit") + : selectedGitScopeLabel + : selectedTurn?.runId === latestTurn?.runId + ? "Latest turn" + : `Turn ${selectedCheckpointTurnCount ?? "?"}`; + const reviewSectionTitle = toolCallSelection + ? "Tool call" + : selectedTurn + ? `Turn ${selectedCheckpointTurnCount ?? "?"}` + : selectedGitScopeLabel; + const headerCommitSha = selectedRunId === null ? selectedCommitSha : null; + const selectedCommitUrl = + headerCommitSha !== null + ? projectCommitUrl(activeProject?.repositoryIdentity, headerCommitSha) + : null; + const openLink = useOpenLink(routeThreadRef); + // A SHA link goes to the code host; it must not route back into this pane's commit selection. + const openCommitUrl = (event: MouseEvent, url: string) => { + event.stopPropagation(); + if (event.metaKey || event.ctrlKey || event.shiftKey || event.altKey || event.button === 1) + return; + event.preventDefault(); + void openLink(url, { event }); + }; const refreshPreviewQuery = branchDiffPreview.refresh; const refreshDiffFromUserAction = refreshPreviewQuery; @@ -512,7 +574,8 @@ export default function DiffPanel({ environmentId: activeThread?.environmentId, cwd: branchDiffPreview.data?.cwd, source: lazySource, - baseRef: lazySource?.baseRef ?? selectedBaseRef, + baseRef: + selectedCommitSha !== null ? selectedBaseRef : (lazySource?.baseRef ?? selectedBaseRef), ignoreWhitespace: diffIgnoreWhitespace, theme: resolvedTheme, previewInput: branchDiffPreviewInput, @@ -693,16 +756,23 @@ export default function DiffPanel({ useDiffPanelStore.getState().selectBranchBaseRef(routeThreadRef, baseRef); }; const selectedTurnValue = selectedTurn ? `turn:${selectedTurn.runId}` : ""; + const selectedCommitValue = selectedCommitSha !== null ? `commit:${selectedCommitSha}` : ""; const selectedScopeValue = toolCallSelection ? "" : selectedRunId === null - ? selectedGitScope + ? selectedCommitValue || selectedGitScope : selectedTurn?.runId === latestTurn?.runId ? "latest" : selectedTurnValue; const selectScopeValue = (value: string) => { if (value === "unstaged" || value === "branch") { selectGitScope(value); + } else if (value === selectedCommitValue) { + selectGitScope("branch"); + } else if (value.startsWith("commit:")) { + if (routeThreadRef) { + useDiffPanelStore.getState().selectCommit(routeThreadRef, value.slice("commit:".length)); + } } else if (value === "latest") { if (latestTurn) selectTurn(latestTurn.runId); } else { @@ -717,13 +787,37 @@ export default function DiffPanel({ } - className="max-w-full" + className={cn("max-w-full", headerCommitSha !== null && "min-w-0 shrink")} aria-label={`Diff scope: ${selectedScopeLabel}`} data-workspace-focus-default > {selectedScopeLabel} + {headerCommitSha !== null ? ( + selectedCommitUrl ? ( + + openCommitUrl(event, selectedCommitUrl)} + /> + } + > + {headerCommitSha.slice(0, 7)} + + {selectedCommitUrl} + + ) : ( + + {headerCommitSha.slice(0, 7)} + + ) + ) : null} @@ -760,6 +854,99 @@ export default function DiffPanel({ + { + setOpenCommitMenu(open ? (branchCommitsQuery.data ?? null) : null); + if (!open) return; + // The menu keeps the list it opened with; the refreshed one shows next time. + branchCommitsQuery.refresh(); + }} + > + Commits + + + {branchCommits?.commits.map((commit) => { + const commitUrl = projectCommitUrl( + activeProject?.repositoryIdentity, + commit.sha, + ); + return ( +
+ + + } + > + + {commit.subject} + + {formatRelativeTimeLabel(commit.committedAt)} + + + + + {commit.subject} + + + {commitUrl ? ( + + } + onClick={(event) => openCommitUrl(event, commitUrl)} + > + + {commit.sha.slice(0, 7)} + + + ) : ( + + {commit.sha.slice(0, 7)} + + )} +
+ ); + })} + + {branchCommitsQuery.error ? ( + {branchCommitsQuery.error} + ) : !canReadFiles ? ( + + {fileAccess.isPending + ? "Checking file access..." + : (fileAccess.error ?? "This connection cannot read local diffs.")} + + ) : branchCommits === null ? ( + Loading commits... + ) : branchCommits.commits.length === 0 ? ( + No commits on this branch. + ) : branchCommits.truncated ? ( + + {`Showing latest ${branchCommits.commits.length} commits`} + + ) : null} + + {isGitSelection && selectedGitScope === "branch" && selectedGitSource?.baseRef && ( @@ -894,7 +1081,12 @@ export default function DiffPanel({ )} -
+
{codeViewFiles.length > 0 || (isGitSelection && selectedGitSource?.files?.length) ? ( ) : ( diff --git a/apps/web/src/components/desktop/DesktopWebLinkCoordinator.browser.test.tsx b/apps/web/src/components/desktop/DesktopWebLinkCoordinator.browser.test.tsx index a2a0f874d6e90..31ad2028c5f51 100644 --- a/apps/web/src/components/desktop/DesktopWebLinkCoordinator.browser.test.tsx +++ b/apps/web/src/components/desktop/DesktopWebLinkCoordinator.browser.test.tsx @@ -1,11 +1,16 @@ -import { EnvironmentId, ProjectId } from "@t3tools/contracts"; -import type { Atom } from "effect/reactivity"; +import { EnvironmentId, ProjectId, ThreadId } from "@t3tools/contracts"; +import { scopeThreadRef } from "@t3tools/client-runtime/environment"; +import type { EnvironmentProject } from "@t3tools/client-runtime/state/shell"; +import { AsyncResult, type Atom } from "effect/reactivity"; import { expect, it, vi } from "vite-plus/test"; import { act, render, stubGlobal } from "../../../test/render"; const state = vi.hoisted(() => ({ - projects: [] as never[], + projects: [] as EnvironmentProject[], + routeParams: {} as Record, + preview: vi.fn(), + openLink: vi.fn(async () => undefined), navigate: vi.fn(async (): Promise => undefined), openScratchProject: vi.fn(async () => null), confirm: vi.fn(async (_message: string) => true), @@ -18,7 +23,10 @@ const state = vi.hoisted(() => ({ vi.mock(import("@tanstack/react-router"), async (original) => ({ ...(await original()), useNavigate: (() => state.navigate) as never, - useRouter: (() => ({ navigate: state.navigate, state: { matches: [] } })) as never, + useRouter: (() => ({ + navigate: state.navigate, + state: { matches: [{ params: state.routeParams }] }, + })) as never, })); vi.mock(import("../../state/environments"), async (original) => ({ ...(await original()), @@ -47,11 +55,17 @@ vi.mock(import("../../state/server"), async (original) => { }); vi.mock(import("../../state/shell"), async (original) => { const { Atom } = await import("effect/reactivity"); - return { ...(await original()), allReachableShellsLiveAtom: Atom.make(false) }; + return { + ...(await original()), + allReachableShellsLiveAtom: Atom.make(false).pipe(Atom.keepAlive), + }; }); vi.mock(import("../../state/entities"), async (original) => ({ ...(await original()), readProjects: () => state.projects, + readThreadShell: (ref: { threadId: string }) => + ref.threadId === "thread" ? ({ projectId: "effect", worktreePath: null } as never) : null, + readProject: () => state.projects[0] ?? null, })); vi.mock(import("../../hooks/useScratchProject"), async (original) => ({ ...(await original()), @@ -61,6 +75,10 @@ vi.mock(import("../../hooks/useScratchProject"), async (original) => ({ })) as never, })); +vi.mock("../../state/use-atom-query-runner", () => ({ + useAtomQueryRunner: () => state.preview, +})); +vi.mock("../../browser/useOpenLink", () => ({ useOpenLink: () => state.openLink })); vi.mock(import("../../confirmDialog"), async (original) => ({ ...(await original()), requestConfirmDialog: state.confirm, @@ -82,6 +100,12 @@ vi.mock(import("../../state/use-atom-command"), async (original) => { import { AppAtomRegistryProvider, appAtomRegistry } from "../../rpc/atomRegistry"; import { allReachableShellsLiveAtom } from "../../state/shell"; +import { selectThreadDiffPanelSelection, useDiffPanelStore } from "../../diffPanelStore"; +import { + selectActiveRightPanel, + selectActiveRightPanelSurface, + useRightPanelStore, +} from "../../rightPanelStore"; import { DesktopWebLinkCoordinator } from "./DesktopWebLinkCoordinator"; const discoverySettled = allReachableShellsLiveAtom as Atom.Writable; @@ -89,6 +113,7 @@ const environmentId = EnvironmentId.make("local"); const projectId = ProjectId.make("effect"); const project = { id: projectId, + workspaceRoot: "/workspace/effect", environmentId, repositoryIdentity: { provider: "github", @@ -100,7 +125,7 @@ const project = { remoteUrl: "https://github.com/Effect-TS/effect.git", }, }, -} as never; +} as EnvironmentProject; const pullRequestUrl = "https://github.com/Effect-TS/effect/pull/9339"; const pullRequestView = { to: "/pull-requests", @@ -123,6 +148,12 @@ function setup() { state.confirm.mockResolvedValue(true); state.localEnvironmentDisabled = false; state.projects = []; + state.routeParams = {}; + state.preview + .mockReset() + .mockResolvedValue(AsyncResult.success({ sources: [{ kind: "commit", headRef: sha }] })); + useDiffPanelStore.setState({ byThreadKey: {}, branchBaseRefByThreadKey: {} }); + useRightPanelStore.setState({ byThreadKey: {}, userActionRevisionByThreadKey: {} }); appAtomRegistry.set(discoverySettled, false); } @@ -172,6 +203,80 @@ it("holds a launch PR link until project discovery settles, then opens the PR vi expect(state.openScratchProject).not.toHaveBeenCalled(); }); +const sha = "a".repeat(40); +const commitUrl = `https://github.com/Effect-TS/effect/pull/9339/commits/${sha}`; +const threadRef = scopeThreadRef(environmentId, ThreadId.make("thread")); + +it.each([ + ["github.com", "pull/9339/commits"], + ["code.example.com:8443", "commit"], +])("opens a %s/%s commit in the active thread's Diff pane after discovery", async (host, path) => { + setup(); + state.routeParams = { environmentId: "local", threadId: "thread" }; + await using _coordinator = await mount(`https://${host}/Effect-TS/effect/${path}/${sha}`); + expect(state.preview).not.toHaveBeenCalled(); + + state.projects = [ + { + ...project, + repositoryIdentity: { + ...project.repositoryIdentity, + canonicalKey: `${host}/effect-ts/effect`, + locator: { + ...project.repositoryIdentity?.locator, + remoteUrl: `https://${host}/Effect-TS/effect.git`, + }, + }, + } as EnvironmentProject, + ]; + await act(async () => appAtomRegistry.set(discoverySettled, true)); + await vi.waitFor(() => { + expect(selectActiveRightPanel(useRightPanelStore.getState().byThreadKey, threadRef)).toBe( + "diff", + ); + }); + expect( + selectThreadDiffPanelSelection(useDiffPanelStore.getState().byThreadKey, threadRef), + ).toMatchObject({ kind: "commit", sha }); + expect(state.preview).toHaveBeenCalledExactlyOnceWith({ + environmentId, + input: expect.objectContaining({ cwd: "/workspace/effect", commitSha: sha }), + }); + expect(state.navigate).not.toHaveBeenCalled(); + expect(state.openLink).not.toHaveBeenCalled(); +}); + +it("opens a PR commit the workspace lacks in the thread's PR panel", async () => { + setup(); + state.routeParams = { environmentId: "local", threadId: "thread" }; + state.projects = [project]; + state.preview.mockResolvedValueOnce(AsyncResult.success({ sources: [] })); + appAtomRegistry.set(discoverySettled, true); + await using _coordinator = await mount(commitUrl); + await vi.waitFor(() => + expect( + selectActiveRightPanelSurface(useRightPanelStore.getState().byThreadKey, threadRef), + ).toMatchObject({ kind: "pull-request", repository: "effect-ts/effect", number: 9339 }), + ); + expect(useDiffPanelStore.getState().byThreadKey).toEqual({}); + expect(state.openLink).not.toHaveBeenCalled(); +}); + +it("opens a plain commit the workspace lacks like any other delivered link", async () => { + setup(); + state.routeParams = { environmentId: "local", threadId: "thread" }; + state.projects = [project]; + state.preview.mockResolvedValueOnce(AsyncResult.success({ sources: [] })); + appAtomRegistry.set(discoverySettled, true); + const plainCommitUrl = `https://github.com/Effect-TS/effect/commit/${sha}`; + await using _coordinator = await mount(plainCommitUrl); + await vi.waitFor(() => expect(state.openScratchProject).toHaveBeenCalledOnce()); + // Handing it to the OS browser would loop when T3 Code is the default browser. + expect(state.openLink).not.toHaveBeenCalled(); + expect(useDiffPanelStore.getState().byThreadKey).toEqual({}); + expect(useRightPanelStore.getState().byThreadKey).toEqual({}); +}); + it("adds the backend from a pairing link after the user confirms its host", async () => { setup(); await using _coordinator = await mount(pairLink); diff --git a/apps/web/src/components/desktop/DesktopWebLinkCoordinator.tsx b/apps/web/src/components/desktop/DesktopWebLinkCoordinator.tsx index 4ae0b6cdb25b4..ddd892d02bd12 100644 --- a/apps/web/src/components/desktop/DesktopWebLinkCoordinator.tsx +++ b/apps/web/src/components/desktop/DesktopWebLinkCoordinator.tsx @@ -27,6 +27,7 @@ import { selectActiveRightPanelSurface, useRightPanelStore } from "../../rightPa import { appAtomRegistry } from "../../rpc/atomRegistry"; import { resolveThreadRouteTarget } from "../../threadRoutes"; import { assetEnvironment } from "../../state/assets"; +import { readProject, readThreadShell } from "../../state/entities"; import { useEnvironmentHttpBaseUrl, usePrimaryEnvironment } from "../../state/environments"; import { previewEnvironment } from "../../state/preview"; import { allReachableShellsLiveAtom } from "../../state/shell"; @@ -163,25 +164,14 @@ export function DesktopWebLinkCoordinator() { return threadRef; }; - const openLink = useEffectEvent(async (url: string) => { - const pairingUrl = readPairingAppLink(url); - if (pairingUrl !== null) { - await pairFromLink(pairingUrl); - return; - } - const appPath = promptLinkAppPath(url); - if (appPath !== null) { - await router.navigate({ href: appPath }); - return; - } - if (openChangeRequest(externalClick, url)) return; + const openInBrowser = useEffectEvent(async (url: string) => { + const routeParams = router.state.matches.at(-1)?.params ?? {}; const isFile = url.startsWith("file:"); // Only this machine's environment serves files, so fail before starting a thread. if (isFile && httpBaseUrl === null) { reportFailure(url); return; } - const routeParams = router.state.matches.at(-1)?.params ?? {}; const threadRef = threadShowingBrowser( routeParams, @@ -210,15 +200,49 @@ export function DesktopWebLinkCoordinator() { } }); + const openLink = useEffectEvent(async (url: string) => { + const pairingUrl = readPairingAppLink(url); + if (pairingUrl !== null) { + await pairFromLink(pairingUrl); + return; + } + const appPath = promptLinkAppPath(url); + if (appPath !== null) { + await router.navigate({ href: appPath }); + return; + } + const routeParams = router.state.matches.at(-1)?.params ?? {}; + const routeTarget = resolveThreadRouteTarget(routeParams); + const activeThreadRef = routeTarget?.kind === "server" ? routeTarget.threadRef : undefined; + const activeThread = activeThreadRef ? readThreadShell(activeThreadRef) : null; + const activeProject = + activeThread && activeThreadRef + ? readProject(scopeProjectRef(activeThreadRef.environmentId, activeThread.projectId)) + : null; + const commitThreadRef = + activeThread && + sourceControlClients.parseCommitUrl(url, activeProject?.repositoryIdentity?.provider) !== null + ? activeThreadRef + : undefined; + // A commit the thread's workspace lacks continues here, after its lookup. + const openUnavailableCommit = () => { + queueRef.current = queueRef.current.then(() => openInBrowser(url)).catch(() => undefined); + }; + if (openChangeRequest(externalClick, url, commitThreadRef, undefined, openUnavailableCommit)) + return; + await openInBrowser(url); + }); + const receiveLink = useEffectEvent((url: string) => { - // A connected server publishes its config before its project snapshot, so a PR link - // delivered at launch would miss its project and fall through to the browser. Hold PR - // links, and only those, until discovery settles; an ordinary link never waits for it. + // A connected server publishes its config before its project snapshot, so a source control + // link delivered at launch would miss its project and fall through to the browser. Hold + // those links until discovery settles; an ordinary link never waits for it. // Discovery never settles without the local environment, so those links do not wait. const hold = openPullRequestsInApp && !localEnvironmentDisabled && - sourceControlClients.parseChangeRequestUrl(url) !== null + (sourceControlClients.parseChangeRequestUrl(url) !== null || + sourceControlClients.parseCommitUrl(url) !== null) ? projectsSettled() : Promise.resolve(); void hold.then(() => { diff --git a/apps/web/src/components/diffs/useReviewFilePatches.ts b/apps/web/src/components/diffs/useReviewFilePatches.ts index b683e13a8f6ee..69dafc2dd7823 100644 --- a/apps/web/src/components/diffs/useReviewFilePatches.ts +++ b/apps/web/src/components/diffs/useReviewFilePatches.ts @@ -193,6 +193,7 @@ function reviewFileQueries({ request: { cwd, ...(baseRef ? { baseRef } : {}), + ...(source.kind === "commit" ? { commitSha: source.id } : {}), ignoreWhitespace, file: { path: file.path, diff --git a/apps/web/src/components/settings/settingsSearch.ts b/apps/web/src/components/settings/settingsSearch.ts index dc494caf5344d..aabfd0300eda5 100644 --- a/apps/web/src/components/settings/settingsSearch.ts +++ b/apps/web/src/components/settings/settingsSearch.ts @@ -923,7 +923,7 @@ export const SETTINGS_SEARCH_ITEMS = [ title: "Open pull requests in T3 Code", to: "/settings/diffs", searchTerms: [ - `pr links ${SOURCE_CONTROL_HOST_LABELS} browser external click default pull request view panel`, + `pr commit links ${SOURCE_CONTROL_HOST_LABELS} browser external click default pull request view panel`, ], }, { diff --git a/apps/web/src/diffPanelStore.ts b/apps/web/src/diffPanelStore.ts index 11d96a3909ce7..6bd7eb0e01bdd 100644 --- a/apps/web/src/diffPanelStore.ts +++ b/apps/web/src/diffPanelStore.ts @@ -17,6 +17,7 @@ export type DiffPanelSelection = | ({ kind: "tool-call" } & ToolCallDiffTarget) | { kind: "branch"; baseRef: string | null } | { kind: "unstaged" } + | { kind: "commit"; sha: string; baseRef: string | null } | { kind: "turn"; turnId: RunId; filePath: string | null; revealRequestId: number }; const DEFAULT_SELECTION: DiffPanelSelection = { kind: "branch", baseRef: null }; @@ -24,10 +25,14 @@ const DEFAULT_SELECTION: DiffPanelSelection = { kind: "branch", baseRef: null }; interface DiffPanelStoreState { byThreadKey: Record; branchBaseRefByThreadKey: Record; + /** A link that selected a commit before opening the closed pane; opening keeps that selection. */ + keepSelectionOnOpenThreadKey: string | null; selectGitScope: (ref: ScopedThreadRef, scope: "branch" | "unstaged") => void; selectBranchBaseRef: (ref: ScopedThreadRef, baseRef: string | null) => void; selectToolCall: (ref: ScopedThreadRef, target: ToolCallDiffTarget) => void; selectTurn: (ref: ScopedThreadRef, turnId: RunId, filePath?: string) => void; + selectCommit: (ref: ScopedThreadRef, sha: string, keepOnOpen?: boolean) => void; + consumeKeepSelectionOnOpen: (ref: ScopedThreadRef) => boolean; reconcileTurnSelection: (ref: ScopedThreadRef, availableTurnIds: ReadonlyArray) => void; removeThread: (ref: ScopedThreadRef) => void; } @@ -37,19 +42,27 @@ function normalizeBaseRef(baseRef: string | null): string | null { return normalized ? normalized : null; } +function branchBaseRef( + state: Pick, + threadKey: string, +): string | null { + const previous = state.byThreadKey[threadKey]; + return previous?.kind === "branch" || previous?.kind === "commit" + ? previous.baseRef + : (state.branchBaseRefByThreadKey[threadKey] ?? null); +} + export const useDiffPanelStore = create()( persist( - (set) => ({ + (set, get) => ({ byThreadKey: {}, branchBaseRefByThreadKey: {}, + keepSelectionOnOpenThreadKey: null, selectGitScope: (ref, scope) => set((state) => { const threadKey = scopedThreadKey(ref); const previous = state.byThreadKey[threadKey]; - const previousBaseRef = - previous?.kind === "branch" - ? previous.baseRef - : (state.branchBaseRefByThreadKey[threadKey] ?? null); + const previousBaseRef = branchBaseRef(state, threadKey); return { byThreadKey: { ...state.byThreadKey, @@ -59,7 +72,7 @@ export const useDiffPanelStore = create()( : { kind: "unstaged" }, }, branchBaseRefByThreadKey: - previous?.kind === "branch" + previous?.kind === "branch" || previous?.kind === "commit" ? { ...state.branchBaseRefByThreadKey, [threadKey]: previous.baseRef } : state.branchBaseRefByThreadKey, }; @@ -102,6 +115,26 @@ export const useDiffPanelStore = create()( }, }; }), + selectCommit: (ref, sha, keepOnOpen = false) => + set((state) => { + const threadKey = scopedThreadKey(ref); + const baseRef = branchBaseRef(state, threadKey); + return { + byThreadKey: { + ...state.byThreadKey, + [threadKey]: { kind: "commit", sha, baseRef }, + }, + branchBaseRefByThreadKey: { ...state.branchBaseRefByThreadKey, [threadKey]: baseRef }, + keepSelectionOnOpenThreadKey: keepOnOpen + ? threadKey + : state.keepSelectionOnOpenThreadKey, + }; + }), + consumeKeepSelectionOnOpen: (ref) => { + if (get().keepSelectionOnOpenThreadKey !== scopedThreadKey(ref)) return false; + set({ keepSelectionOnOpenThreadKey: null }); + return true; + }, reconcileTurnSelection: (ref, availableTurnIds) => set((state) => { const threadKey = scopedThreadKey(ref); @@ -178,6 +211,13 @@ export function selectThreadDiffPanelSelection( return byThreadKey[scopedThreadKey(ref)] ?? DEFAULT_SELECTION; } +export function selectThreadBranchBaseRef( + state: Pick, + ref: ScopedThreadRef | null | undefined, +): string | null { + return ref ? branchBaseRef(state, scopedThreadKey(ref)) : null; +} + export function resolveToolCallDiffTarget( target: ToolCallDiffTarget, items: ReadonlyArray, diff --git a/apps/web/src/lib/openPullRequestLink.browser.test.tsx b/apps/web/src/lib/openPullRequestLink.browser.test.tsx new file mode 100644 index 0000000000000..a93f32a8902f7 --- /dev/null +++ b/apps/web/src/lib/openPullRequestLink.browser.test.tsx @@ -0,0 +1,160 @@ +import { EnvironmentId, ThreadId } from "@t3tools/contracts"; +import { scopeThreadRef, scopedThreadKey } from "@t3tools/client-runtime/environment"; +import { AsyncResult } from "effect/reactivity"; +import { expect, it, vi } from "vite-plus/test"; +import { page } from "vite-plus/test/browser"; + +import { act, render, stubGlobal } from "../../test/render"; + +const state = vi.hoisted(() => ({ preview: vi.fn(), openLink: vi.fn() })); +vi.mock("../state/use-atom-query-runner", () => ({ useAtomQueryRunner: () => state.preview })); +vi.mock("../browser/useOpenLink", () => ({ useOpenLink: () => state.openLink })); +vi.mock(import("../state/environments"), async (original) => ({ + ...(await original()), + usePrimaryEnvironmentId: () => EnvironmentId.make("local"), +})); +vi.mock("../state/entities", async (original) => ({ + ...(await original()), + readProjects: () => [project], + useProjects: () => [project], + readProject: () => project, + readThreadShell: () => ({ projectId: "project", worktreePath: null }), +})); +vi.mock("../state/server", async (original) => { + const { Atom } = await import("effect/reactivity"); + return { + ...(await original()), + environmentServerConfigsAtom: Atom.make(new Map([["local", {}]])), + }; +}); + +import { DEFAULT_CLIENT_SETTINGS } from "@t3tools/contracts/settings"; +import { __setClientSettingsForTests } from "../hooks/useSettings"; +import { ToastProvider } from "../components/ui/toast"; +import { AppAtomRegistryProvider } from "../rpc/atomRegistry"; +import { useRightPanelStore } from "../rightPanelStore"; +import { useDiffPanelStore } from "../diffPanelStore"; +import { useOpenChangeRequestLink } from "./openPullRequestLink"; + +const environmentId = EnvironmentId.make("local"); +const threadRef = scopeThreadRef(environmentId, ThreadId.make("thread")); +const project = { + id: "project", + environmentId, + workspaceRoot: "/repo", + repositoryIdentity: { + provider: "github", + canonicalKey: "github.com/effect-ts/effect", + displayName: "Effect-TS/effect", + locator: { + source: "git-remote", + remoteName: "origin", + remoteUrl: "https://github.com/Effect-TS/effect.git", + }, + }, +}; +const commitUrl = `https://github.com/Effect-TS/effect/commit/${"a".repeat(40)}?plain=1#diff-original`; +const pullRequestCommitUrl = `https://github.com/Effect-TS/effect/pull/1/commits/${"a".repeat(40)}`; + +function CommitLink({ url = commitUrl }: { url?: string }) { + const open = useOpenChangeRequestLink(threadRef); + return ( + open(event, url)}> + Commit + + ); +} + +function setup() { + state.preview.mockReset().mockResolvedValue(AsyncResult.success({ sources: [] })); + state.openLink.mockReset().mockResolvedValue(undefined); + __setClientSettingsForTests({ + ...DEFAULT_CLIENT_SETTINGS, + pullRequestLinksOpenInApp: true, + browserLinkTarget: "system", + }); + useDiffPanelStore.setState({ + byThreadKey: {}, + branchBaseRefByThreadKey: {}, + keepSelectionOnOpenThreadKey: null, + }); + useRightPanelStore.setState({ byThreadKey: {}, userActionRevisionByThreadKey: {} }); +} + +it.each([ + ["shiftKey", commitUrl], + ["altKey", commitUrl], + ["shiftKey", pullRequestCommitUrl], +] as const)("keeps %s clicks on %s native", async (modifier, url) => { + setup(); + await using screen = await render( + + + , + ); + const event = new MouseEvent("click", { bubbles: true, cancelable: true, [modifier]: true }); + const defaults: boolean[] = []; + screen.container.addEventListener( + "click", + (nativeEvent) => { + defaults.push(nativeEvent.defaultPrevented); + nativeEvent.preventDefault(); + }, + { once: true }, + ); + await act(() => screen.getByRole("link", { name: "Commit" }).element().dispatchEvent(event)); + expect(defaults).toEqual([false]); + expect(state.preview).not.toHaveBeenCalled(); + expect(useRightPanelStore.getState().byThreadKey).toEqual({}); +}); + +it.each(["system", "app"] as const)( + "offers the original unavailable commit URL when web popups are blocked (%s)", + async (browserLinkTarget) => { + setup(); + // Without preview access, the "app" preference opens a popup too. + __setClientSettingsForTests({ + ...DEFAULT_CLIENT_SETTINGS, + pullRequestLinksOpenInApp: true, + browserLinkTarget, + }); + using _desktop = stubGlobal("desktopBridge", undefined); + await using screen = await render( + + + + + , + ); + using open = vi.spyOn(window, "open").mockReturnValue(null); + await screen.getByRole("link", { name: "Commit" }).click(); + await expect + .element(page.getByRole("button", { name: "Open link", exact: true })) + .toBeVisible(); + expect(open).not.toHaveBeenCalled(); + await page.getByRole("button", { name: "Open link", exact: true }).click(); + expect(open).toHaveBeenCalledExactlyOnceWith(commitUrl, "_blank", "noopener,noreferrer"); + expect(state.openLink).not.toHaveBeenCalled(); + }, +); + +it("opens an available commit in the closed Diff pane without the pane resetting it", async () => { + setup(); + const sha = "a".repeat(40); + state.preview.mockResolvedValue( + AsyncResult.success({ sources: [{ kind: "commit", headRef: sha }] }), + ); + await using screen = await render( + + + , + ); + await screen.getByRole("link", { name: "Commit" }).click(); + await vi.waitFor(() => + expect(useDiffPanelStore.getState().byThreadKey).toEqual({ + [scopedThreadKey(threadRef)]: { kind: "commit", sha, baseRef: null }, + }), + ); + // ChatView resets a pane opened without an explicit request to Changes unless this is set. + expect(useDiffPanelStore.getState().consumeKeepSelectionOnOpen(threadRef)).toBe(true); +}); diff --git a/apps/web/src/lib/openPullRequestLink.ts b/apps/web/src/lib/openPullRequestLink.ts index 014728f058a43..31c04b6263e5f 100644 --- a/apps/web/src/lib/openPullRequestLink.ts +++ b/apps/web/src/lib/openPullRequestLink.ts @@ -8,15 +8,25 @@ import { resolvePullRequestPreviewTarget, } from "@t3tools/client-runtime/change-request-links"; +import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients"; + +import { canOpenLinksInApp } from "../browser/browserLinkTarget"; import { useOpenLink } from "../browser/useOpenLink"; import { useClientSettings } from "../hooks/useSettings"; import { stackedThreadToast, toastManager } from "../components/ui/toast"; -import { useRightPanelStore } from "../rightPanelStore"; +import { selectActiveRightPanel, useRightPanelStore } from "../rightPanelStore"; +import { + selectThreadBranchBaseRef, + selectThreadDiffPanelSelection, + useDiffPanelStore, +} from "../diffPanelStore"; import { appAtomRegistry } from "../rpc/atomRegistry"; -import { readProjects, useProjects } from "../state/entities"; +import { readProject, readProjects, readThreadShell, useProjects } from "../state/entities"; import { environmentServerConfigsAtom, serverEnvironment } from "../state/server"; import { usePrimaryEnvironmentId } from "../state/environments"; +import { reviewEnvironment } from "../state/review"; +import { useAtomQueryRunner } from "../state/use-atom-query-runner"; export function usePullRequestPreviewTarget(environmentId: EnvironmentId | null, url: string) { const projects = useProjects(); @@ -43,71 +53,178 @@ export function useOpenChangeRequestLink( event: Pick< MouseEvent, "preventDefault" | "stopPropagation" | "metaKey" | "ctrlKey" - >, + > & + Partial, "button" | "shiftKey" | "altKey">>, targetUrl: string, targetThreadRef?: ScopedThreadRef, targetEnvironmentId?: EnvironmentId, + onCommitUnavailable?: () => void, ) => boolean { const navigate = useNavigate(); const primaryEnvironmentId = usePrimaryEnvironmentId(); const openInApp = useClientSettings((settings) => settings.pullRequestLinksOpenInApp); - return (event, targetUrl, targetThreadRef, targetEnvironmentId) => { + const ignoreWhitespace = useClientSettings((settings) => settings.diffIgnoreWhitespace); + const browserLinkTarget = useClientSettings((settings) => settings.browserLinkTarget); + const readDiffPreview = useAtomQueryRunner(reviewEnvironment.diffPreview, { + reportFailure: false, + refresh: true, + }); + const openLink = useOpenLink(threadRef); + return (event, targetUrl, targetThreadRef, targetEnvironmentId, onCommitUnavailable) => { if (!openInApp || shouldOpenPullRequestExternally(event)) return false; - // Read at click time: every provider status push replaces the config map, and - // subscribing would hand every rendered link a new handler. Projects likewise, so a - // link can resolve against a snapshot that arrived after this handler was built. - const serverConfigs = appAtomRegistry.get(environmentServerConfigsAtom); const resolvedThreadRef = targetThreadRef ?? threadRef; const resolvedPanelRef = panelRef ?? resolvedThreadRef; - const target = resolveChangeRequestLinkTarget({ - url: targetUrl, - projects: readProjects(), - configLoaded: (environmentId) => serverConfigs.has(environmentId), - environmentId: resolvedThreadRef?.environmentId ?? targetEnvironmentId, - primaryEnvironmentId, - lendHostProject: resolvedPanelRef !== undefined, - }); - if (target === null) return false; - const { project, repository, number, host } = target; - event.preventDefault(); - event.stopPropagation(); - const pullRequestsSearch = { - repository, - number, - ...(host === undefined ? {} : { selectedHost: host }), - selectedProjectId: project.id, - selectedEnvironmentId: project.environmentId, - }; - if (resolvedPanelRef) { - useRightPanelStore.getState().openPullRequest(resolvedPanelRef, { - // The standalone PR panel has a synthetic ref; each tab keeps its real environment. - ...(resolvedPanelRef.environmentId === project.environmentId - ? {} - : { environmentId: project.environmentId }), - projectId: project.id, - ...(host === undefined ? {} : { host }), - repository, + const openPullRequest = () => { + // Read at click time: every provider status push replaces the config map, and + // subscribing would hand every rendered link a new handler. Projects likewise, so a + // link can resolve against a snapshot that arrived after this handler was built. + const serverConfigs = appAtomRegistry.get(environmentServerConfigsAtom); + const target = resolveChangeRequestLinkTarget({ url: targetUrl, - number, + projects: readProjects(), + configLoaded: (environmentId) => serverConfigs.has(environmentId), + environmentId: resolvedThreadRef?.environmentId ?? targetEnvironmentId, + primaryEnvironmentId, + lendHostProject: resolvedPanelRef !== undefined, }); - if (!resolvedThreadRef) { - void navigate({ - to: "/pull-requests", - search: (previous) => ({ - ...previous, - involvement: previous.involvement ?? "all", - state: previous.state ?? "all", - ...pullRequestsSearch, - }), - replace: true, + if (target === null) return false; + const { project, repository, number, host } = target; + const pullRequestsSearch = { + repository, + number, + ...(host === undefined ? {} : { selectedHost: host }), + selectedProjectId: project.id, + selectedEnvironmentId: project.environmentId, + }; + if (resolvedPanelRef) { + useRightPanelStore.getState().openPullRequest(resolvedPanelRef, { + // The standalone PR panel has a synthetic ref; each tab keeps its real environment. + ...(resolvedPanelRef.environmentId === project.environmentId + ? {} + : { environmentId: project.environmentId }), + projectId: project.id, + ...(host === undefined ? {} : { host }), + repository, + url: targetUrl, + number, }); + if (!resolvedThreadRef) { + void navigate({ + to: "/pull-requests", + search: (previous) => ({ + ...previous, + involvement: previous.involvement ?? "all", + state: previous.state ?? "all", + ...pullRequestsSearch, + }), + replace: true, + }); + } + return true; } + void navigate({ + to: "/pull-requests", + search: { involvement: "all", state: "all", ...pullRequestsSearch }, + }); + return true; + }; + const commitThread = resolvedThreadRef ? readThreadShell(resolvedThreadRef) : null; + const commitProject = + commitThread && resolvedThreadRef + ? readProject({ + environmentId: resolvedThreadRef.environmentId, + projectId: commitThread.projectId, + }) + : null; + const identity = commitProject?.repositoryIdentity; + const parsedCommit = sourceControlClients.parseCommitUrl(targetUrl, identity?.provider); + // Shift and Alt keep their browser meaning (new window, download) on commit links. + if (parsedCommit !== null && (event.shiftKey || event.altKey)) return false; + if ( + parsedCommit !== null && + commitThread && + resolvedThreadRef && + commitProject?.environmentId === resolvedThreadRef.environmentId && + identity && + identity.provider === parsedCommit.definition.kind && + parsedCommit.definition.isChangeRequestInRepository(identity, parsedCommit.link) + ) { + const commit = parsedCommit.link; + event.preventDefault(); + event.stopPropagation(); + const userActionRevision = useRightPanelStore.getState().markUserAction(resolvedThreadRef); + const baseRef = selectThreadBranchBaseRef(useDiffPanelStore.getState(), resolvedThreadRef); + const selection = selectThreadDiffPanelSelection( + useDiffPanelStore.getState().byThreadKey, + resolvedThreadRef, + ); + void readDiffPreview({ + environmentId: resolvedThreadRef.environmentId, + input: { + cwd: commitThread.worktreePath ?? commitProject.workspaceRoot, + ...(baseRef ? { baseRef } : {}), + commitSha: commit.sha, + ignoreWhitespace, + }, + }) + .then((result) => { + if ( + useRightPanelStore.getState().getUserActionRevision(resolvedThreadRef) !== + userActionRevision || + selectThreadDiffPanelSelection( + useDiffPanelStore.getState().byThreadKey, + resolvedThreadRef, + ) !== selection + ) + return; + if ( + result._tag === "Success" && + result.value.sources.some( + (source) => source.kind === "commit" && source.headRef === commit.sha, + ) + ) { + const diffOpen = + selectActiveRightPanel( + useRightPanelStore.getState().byThreadKey, + resolvedThreadRef, + ) === "diff"; + useDiffPanelStore.getState().selectCommit(resolvedThreadRef, commit.sha, !diffOpen); + useRightPanelStore.getState().open(resolvedThreadRef, "diff"); + return; + } + if (openPullRequest()) return; + if (onCommitUnavailable) return onCommitUnavailable(); + // A popup opened after the lookup has lost the click's activation, so browsers block it. + if ( + window.desktopBridge === undefined && + (browserLinkTarget !== "app" || !canOpenLinksInApp(resolvedThreadRef)) + ) { + const toastId = toastManager.add( + stackedThreadToast({ + type: "info", + title: "This commit is not in the thread's workspace", + actionProps: { + children: "Open link", + onClick: () => { + toastManager.close(toastId); + window.open(targetUrl, "_blank", "noopener,noreferrer"); + }, + }, + }), + ); + return; + } + return openLink(targetUrl, { + event: { metaKey: event.metaKey, ctrlKey: event.ctrlKey, button: event.button }, + threadRef: resolvedThreadRef, + }); + }) + .catch((error: unknown) => console.error(error)); return true; } - void navigate({ - to: "/pull-requests", - search: { involvement: "all", state: "all", ...pullRequestsSearch }, - }); + if (!openPullRequest()) return false; + event.preventDefault(); + event.stopPropagation(); return true; }; } diff --git a/apps/web/src/rightPanelStore.ts b/apps/web/src/rightPanelStore.ts index 33c537a3cf942..ea043c0298bdb 100644 --- a/apps/web/src/rightPanelStore.ts +++ b/apps/web/src/rightPanelStore.ts @@ -109,6 +109,7 @@ interface RightPanelStoreState { userActionRevisionByThreadKey: Record; closeRevisionByThreadKey: Record; getUserActionRevision: (ref: ScopedThreadRef) => number; + markUserAction: (ref: ScopedThreadRef) => number; /** Refused if the user chose a panel after expectedUserActionRevision was read. */ openProactive: ( ref: ScopedThreadRef, @@ -598,6 +599,16 @@ export const useRightPanelStore = create()( closeRevisionByThreadKey: {}, getUserActionRevision: (ref) => get().userActionRevisionByThreadKey[scopedThreadKey(ref)] ?? 0, + markUserAction: (ref) => { + const revision = get().getUserActionRevision(ref) + 1; + set((state) => ({ + userActionRevisionByThreadKey: { + ...state.userActionRevisionByThreadKey, + [scopedThreadKey(ref)]: revision, + }, + })); + return revision; + }, openProactive: (ref, request, expectedUserActionRevision, activateExisting = false) => { let opened = false; set((state) => { diff --git a/docs/user/source-control.md b/docs/user/source-control.md index 815e5dd0668fa..9eb2751f9315f 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -10,6 +10,16 @@ rebases, the files Git brought in are left out. A file stays in the list when th committed it, or fixed a conflict in it. Use the branch comparison to review everything that changed against your base branch. Restore still returns the complete saved workspace. +On web and desktop, open the Diff pane source picker and choose **Commits** to review one local +commit at a time. The list shows the newest 100 commits in the branch comparison. A merge commit +shows its changes against its first parent. + +A commit link from a supported code host opens in the Diff pane when the thread's repository has +that commit locally, on any branch. Otherwise a pull request commit link opens the pull request. Other +commit links follow your **Open links in** setting; on web, when that opens a new tab, T3 Code first +shows an **Open link** button. Command/Ctrl+Click always opens the link in your browser. Commit links follow the **Open pull requests in T3 Code** +setting in **Settings → Diffs**. + ## Connect an account Install Git and configure authentication on the machine running your T3 Code server. For a remote diff --git a/packages/client-runtime/src/changeRequestLinks.ts b/packages/client-runtime/src/changeRequestLinks.ts index 2983647f3e26f..76f60a5953f2b 100644 --- a/packages/client-runtime/src/changeRequestLinks.ts +++ b/packages/client-runtime/src/changeRequestLinks.ts @@ -150,3 +150,21 @@ export function projectChangeRequestUrl( ...(ownRepository && identity.webUrl !== undefined ? { webUrl: identity.webUrl } : {}), }); } + +export function projectCommitUrl( + identity: RepositoryIdentity | null | undefined, + sha: string, +): string | null { + if (identity?.provider === undefined) return null; + const [host, ...path] = identity.canonicalKey.split("/"); + if (!host) return null; + return ( + sourceControlClients.get(identity.provider).commitUrl?.({ + host, + repository: path.join("/"), + sha, + remoteUrl: identity.locator.remoteUrl.trim(), + ...(identity.webUrl !== undefined ? { webUrl: identity.webUrl } : {}), + }) ?? null + ); +} diff --git a/packages/client-runtime/src/state/review.ts b/packages/client-runtime/src/state/review.ts index 691212aedd501..6700be9af657a 100644 --- a/packages/client-runtime/src/state/review.ts +++ b/packages/client-runtime/src/state/review.ts @@ -51,6 +51,11 @@ export function createReviewEnvironmentAtoms( }); }); return { + branchCommits: createEnvironmentRpcQueryAtomFamily(runtime, { + label: "environment-data:review:branch-commits", + tag: WS_METHODS.reviewGetBranchCommits, + staleTimeMs: 5_000, + }), diffPreview, diffFilePatch: createEnvironmentQueryAtomFamily(runtime, { label: "environment-data:review:diff-file-patch", diff --git a/packages/contracts/src/review.ts b/packages/contracts/src/review.ts index 5a95442917c9a..a40bddfc1e33e 100644 --- a/packages/contracts/src/review.ts +++ b/packages/contracts/src/review.ts @@ -6,18 +6,41 @@ import { VcsError } from "./vcs.ts"; export const ReviewDiffPreviewInput = Schema.Struct({ cwd: TrimmedNonEmptyString, baseRef: Schema.optional(TrimmedNonEmptyString), + commitSha: Schema.optional(TrimmedNonEmptyString), ignoreWhitespace: Schema.optionalKey(Schema.Boolean), file: Schema.optionalKey( Schema.Struct({ path: Schema.NonEmptyString, previousPath: Schema.NullOr(Schema.NonEmptyString), - sourceKind: Schema.Literals(["working-tree", "branch-range"]), + sourceKind: Schema.Literals(["working-tree", "branch-range", "commit"]), }), ), }); export type ReviewDiffPreviewInput = typeof ReviewDiffPreviewInput.Type; -export const ReviewDiffPreviewSourceKind = Schema.Literals(["working-tree", "branch-range"]); +export const ReviewBranchCommitsInput = Schema.Struct({ + cwd: TrimmedNonEmptyString, + baseRef: Schema.optional(TrimmedNonEmptyString), +}); +export type ReviewBranchCommitsInput = typeof ReviewBranchCommitsInput.Type; + +export const ReviewBranchCommitsResult = Schema.Struct({ + commits: Schema.Array( + Schema.Struct({ + sha: Schema.String, + subject: Schema.String, + committedAt: Schema.String, + }), + ), + truncated: Schema.Boolean, +}); +export type ReviewBranchCommitsResult = typeof ReviewBranchCommitsResult.Type; + +export const ReviewDiffPreviewSourceKind = Schema.Literals([ + "working-tree", + "branch-range", + "commit", +]); export type ReviewDiffPreviewSourceKind = typeof ReviewDiffPreviewSourceKind.Type; export const ReviewDiffFileStat = Schema.Struct({ @@ -32,6 +55,7 @@ export const ReviewDiffPreviewSource = Schema.Struct({ id: TrimmedNonEmptyString, kind: ReviewDiffPreviewSourceKind, title: TrimmedNonEmptyString, + commitSubject: Schema.optionalKey(TrimmedNonEmptyString), baseRef: Schema.NullOr(TrimmedNonEmptyString), headRef: Schema.NullOr(TrimmedNonEmptyString), diff: Schema.String, diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index 445048eb2685d..edc7a9ab852a4 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -149,6 +149,8 @@ import { VcsStatusStreamEvent, } from "./git.ts"; import { + ReviewBranchCommitsInput, + ReviewBranchCommitsResult, ReviewDiffFileContentsInput, ReviewDiffFileContentsResult, ReviewDiffPreviewError, @@ -455,6 +457,7 @@ export const WS_METHODS = { gitResolvePullRequest: "git.resolvePullRequest", gitPreparePullRequestThread: "git.preparePullRequestThread", + reviewGetBranchCommits: "review.getBranchCommits", reviewGetDiffPreview: "review.getDiffPreview", reviewGetDiffFileContents: "review.getDiffFileContents", @@ -1500,6 +1503,12 @@ const WsVcsInitRpc = Rpc.make(WS_METHODS.vcsInit, { error: Schema.Union([VcsError, EnvironmentAuthorizationError]), }); +const WsReviewGetBranchCommitsRpc = Rpc.make(WS_METHODS.reviewGetBranchCommits, { + payload: ReviewBranchCommitsInput, + success: ReviewBranchCommitsResult, + error: Schema.Union([ReviewDiffPreviewError, EnvironmentAuthorizationError]), +}); + const WsReviewGetDiffPreviewRpc = Rpc.make(WS_METHODS.reviewGetDiffPreview, { payload: ReviewDiffPreviewInput, success: ReviewDiffPreviewResult, @@ -2092,6 +2101,7 @@ export const WsRpcGroup = RpcGroup.make( WsVcsCreateRefRpc, WsVcsSwitchRefRpc, WsVcsInitRpc, + WsReviewGetBranchCommitsRpc, WsReviewGetDiffPreviewRpc, WsReviewGetDiffFileContentsRpc, WsTerminalOpenRpc, diff --git a/packages/source-control-azure-devops/src/shared/definition.ts b/packages/source-control-azure-devops/src/shared/definition.ts index b43adccedc846..2dd036674747b 100644 --- a/packages/source-control-azure-devops/src/shared/definition.ts +++ b/packages/source-control-azure-devops/src/shared/definition.ts @@ -1,15 +1,18 @@ import { SourceControlProviderKind } from "@t3tools/contracts"; import { + buildCommitUrl, changeRequestUrlBelow, defineSourceControl, isChangeRequestAuthorityOfProject, parseChangeRequestPath, + parseCommitPath, remoteWebOrigin, } from "@t3tools/source-control-core/shared/definition"; import { icon } from "./icon.ts"; // Azure DevOps Server runs on any hostname; its `_git` route identifies it. const CHANGE_REQUEST_PATH = /^\/((?:[^/]+\/)*_git\/[^/]+)\/pullrequest\/(\d+)(?:\/|$)/u; +const COMMIT_PATH = /^\/((?:[^/]+\/)*_git\/[^/]+)\/commit\/([0-9a-f]+)\/?$/iu; const CHECKOUT_COMMAND = /^az\s+repos\s+pr\s+checkout\s+(.+)$/i; const CHANGE_REQUEST_REFERENCE = /^https:\/\/(?:dev\.azure\.com\/[^/\s]+\/[^/\s]+|[^/\s]+\.visualstudio\.com\/[^/\s]+)\/_git\/[^/\s]+\/pullrequest\/(\d+)(?:[/?#].*)?$/i; @@ -88,6 +91,12 @@ export const definition = defineSourceControl({ const [canonicalHost = host, ...path] = (canonicalRepositoryKey(key) ?? key).split("/"); return `${remoteWebOrigin(canonicalHost, remoteUrl)}/${path.join("/")}/pullrequest/${number}`; }, + commitUrl: (input) => { + const key = `${input.host}/${input.repository}`.toLowerCase(); + const [host = input.host, ...path] = (canonicalRepositoryKey(key) ?? key).split("/"); + return buildCommitUrl({ ...input, host, repository: path.join("/") }, "commit"); + }, + parseCommitUrl: (url) => parseCommitPath(url, COMMIT_PATH), // Servers on different ports are different servers, so a port stays in the link. parseChangeRequestUrl: (url) => { const link = parseChangeRequestPath(url, CHANGE_REQUEST_PATH); diff --git a/packages/source-control-bitbucket/src/shared/definition.ts b/packages/source-control-bitbucket/src/shared/definition.ts index 946098b166d34..a8e786cb0ba6e 100644 --- a/packages/source-control-bitbucket/src/shared/definition.ts +++ b/packages/source-control-bitbucket/src/shared/definition.ts @@ -6,11 +6,13 @@ import { import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; import { + buildCommitUrl, changeRequestUrlBelow, defineSourceControl, isChangeRequestInProjectRepository, isChangeRequestOnProjectHost, parseChangeRequestPath, + parseCommitPath, projectChangeRequestHost, } from "@t3tools/source-control-core/shared/definition"; import { icon } from "./icon.ts"; @@ -19,6 +21,7 @@ const safeShellArgument = /^[A-Za-z0-9._/@+=,-]+$/; const repositoryName = /^[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+$/; const CHANGE_REQUEST_PATH = /^\/([^/]+\/[^/]+)\/pull-requests\/(\d+)(?:\/|$)/u; +const COMMIT_PATH = /^\/([^/]+\/[^/]+)\/commits\/([0-9a-f]+)\/?$/iu; const KIND = SourceControlProviderKind.make("bitbucket"); @@ -87,6 +90,9 @@ export const definition = defineSourceControl({ : `https://${host}/${repository}/pull-requests/${number}`, parseChangeRequestUrl: (url) => isBitbucketHost(url.hostname) ? parseChangeRequestPath(url, CHANGE_REQUEST_PATH) : null, + commitUrl: (input) => buildCommitUrl(input, "commits"), + parseCommitUrl: (url) => + isBitbucketHost(url.hostname) ? parseCommitPath(url, COMMIT_PATH) : null, changeRequestUrlFromReferenceAutolink: () => null, // No endpoint reopens a declined pull request, and nothing documented moves one in or out of // draft, so neither is offered rather than failing when pressed. diff --git a/packages/source-control-core/src/shared/definition.ts b/packages/source-control-core/src/shared/definition.ts index 6f99525260129..e9e68325b7418 100644 --- a/packages/source-control-core/src/shared/definition.ts +++ b/packages/source-control-core/src/shared/definition.ts @@ -30,6 +30,17 @@ export interface ParsedChangeRequestUrl extends ChangeRequestLink { readonly repositoryUrl: string; } +export interface CommitLink { + readonly host: string; + readonly repository: string; + readonly sha: string; + readonly authority?: string; +} + +export interface CommitUrlInput extends Omit { + readonly sha: string; +} + export interface ChangeRequestTerminology { readonly shortLabel: string; /** Spelled out because the article follows how the label is said: "an MR". */ @@ -238,6 +249,9 @@ export interface SourceControlDefinition { readonly newRepositoryOwner: (account: string | null) => { readonly owner: string | null } | null; readonly defaultCloneTransport: "https" | "ssh"; readonly changeRequestUrl: (input: ChangeRequestUrlInput) => string | null; + readonly commitUrl?: (input: CommitUrlInput) => string | null; + /** `known` is set when the caller already knows this host serves the URL's repository. */ + readonly parseCommitUrl?: (url: URL, known: boolean) => CommitLink | null; /** Null when the URL is not one of this host's change requests. */ readonly parseChangeRequestUrl: (url: URL) => ParsedChangeRequestUrl | null; /** @@ -290,7 +304,7 @@ export interface SourceControlDefinition { readonly isChangeRequestReference: (url: string) => boolean; readonly isChangeRequestInRepository: ( identity: ChangeRequestProjectIdentity, - link: ChangeRequestLink, + link: ChangeRequestLink | CommitLink, ) => boolean; readonly canReadChangeRequestOnHost: ( identity: ChangeRequestProjectIdentity, @@ -335,6 +349,42 @@ export type SourceControlHostSettingsSchema = { readonly fields: Readonly>; } & Schema.Decoder; +const FULL_COMMIT_SHA = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/iu; + +export function parseCommitPath(url: URL, route: RegExp): CommitLink | null { + if (url.protocol !== "https:" && url.protocol !== "http:") return null; + const match = route.exec(url.pathname); + const repository = match?.[1]; + const sha = match?.[2]; + return repository && sha && FULL_COMMIT_SHA.test(sha) + ? { + host: url.hostname.toLowerCase(), + repository: repository.toLowerCase(), + sha: sha.toLowerCase(), + ...(url.port ? { authority: url.host.toLowerCase() } : {}), + } + : null; +} + +export function buildCommitUrl( + { host, repository, sha, remoteUrl, webUrl }: CommitUrlInput, + route: string, +): string | null { + if (!repository || !FULL_COMMIT_SHA.test(sha)) return null; + try { + const url = new URL(webUrl || `${remoteWebOrigin(host, remoteUrl)}/${repository}`); + if (url.protocol !== "http:" && url.protocol !== "https:") return null; + url.pathname = changeRequestUrlBelow(url.pathname, `${route}/${sha.toLowerCase()}`); + url.username = ""; + url.password = ""; + url.search = ""; + url.hash = ""; + return url.toString(); + } catch { + return null; + } +} + /** * Reads a change request from `url` with `route`, whose first group is the repository path and * second the number. @@ -383,7 +433,7 @@ export function defineSourceControl( export function isChangeRequestOnProjectHost( projectHost: string, identity: ChangeRequestProjectIdentity, - link: ChangeRequestLink, + link: ChangeRequestLink | CommitLink, ): boolean { if (!isChangeRequestAuthorityOfProject(identity, link)) return false; return projectHost === link.host.toLowerCase() || projectHost === link.authority; @@ -426,7 +476,7 @@ function projectRepositoryPath(identity: ChangeRequestProjectIdentity): string | export function isChangeRequestInProjectRepository( projectHost: string, identity: ChangeRequestProjectIdentity, - link: ChangeRequestLink, + link: ChangeRequestLink | CommitLink, ): boolean { const repository = projectRepositoryPath(identity); return ( @@ -533,6 +583,11 @@ export interface ParsedChangeRequest { readonly link: ParsedChangeRequestUrl; } +export interface ParsedCommit { + readonly definition: SourceControlDefinition; + readonly link: CommitLink; +} + export interface SourceControlMedia extends SourceControlMediaUrl { readonly definition: SourceControlDefinition; } @@ -549,6 +604,7 @@ export interface SourceControlDefinitionRegistry { readonly findByPublicDomain: (hostname: string) => SourceControlDefinition | undefined; /** The first host, in registry order, that reads `url` as one of its change requests. */ readonly parseChangeRequestUrl: (url: string) => ParsedChangeRequest | null; + readonly parseCommitUrl: (url: string, kind?: SourceControlProviderKind) => ParsedCommit | null; readonly findByChangeRequestUrl: (url: string) => SourceControlDefinition | undefined; readonly hostLabelForChangeRequestUrl: (url: string) => string; readonly isSameChangeRequest: (left: string, right: string) => boolean; @@ -648,6 +704,16 @@ export function makeSourceControlDefinitionRegistry( ); }, parseChangeRequestUrl, + parseCommitUrl: (value, kind) => { + const url = httpUrl(value); + if (url === null) return null; + const preferred = kind === undefined ? undefined : byKind.get(kind); + for (const definition of kind === undefined ? claimOrder : preferred ? [preferred] : []) { + const link = definition.parseCommitUrl?.(url, preferred !== undefined); + if (link != null) return { definition, link }; + } + return null; + }, findByChangeRequestUrl: (url) => parseChangeRequestUrl(url)?.definition, hostLabelForChangeRequestUrl: (url) => parseChangeRequestUrl(url)?.definition.label ?? "the host", diff --git a/packages/source-control-definitions/src/registry.ts b/packages/source-control-definitions/src/registry.ts index 0e42360855350..e44a6bb281717 100644 --- a/packages/source-control-definitions/src/registry.ts +++ b/packages/source-control-definitions/src/registry.ts @@ -23,7 +23,7 @@ export const builtInSourceControlDefinitions = makeSourceControlDefinitionRegist AzureDevOps.definition, ], { - claimChangeRequestUrlsFirst: [GitCafe.definition.kind], + claimChangeRequestUrlsFirst: [GitCafe.definition.kind, AzureDevOps.definition.kind], recognizeRemotesIn: [ GitCafe.definition.kind, Forgejo.definition.kind, diff --git a/packages/source-control-forgejo/src/shared/definition.ts b/packages/source-control-forgejo/src/shared/definition.ts index 32258854c79da..2cf8ac5bb5838 100644 --- a/packages/source-control-forgejo/src/shared/definition.ts +++ b/packages/source-control-forgejo/src/shared/definition.ts @@ -1,5 +1,6 @@ import { SourceControlProviderKind } from "@t3tools/contracts"; import { + buildCommitUrl, changeRequestUrlBelow, type ChangeRequestHostIdentity, type ChangeRequestProjectIdentity, @@ -8,6 +9,7 @@ import { isChangeRequestInProjectRepository, isChangeRequestOnProjectHost, parseChangeRequestPath, + parseCommitPath, projectChangeRequestHost, remoteWebOrigin, } from "@t3tools/source-control-core/shared/definition"; @@ -19,6 +21,7 @@ const CHANGE_REQUEST_REFERENCE = // Forgejo and Gitea run on any hostname, possibly mounted below a path. const CHANGE_REQUEST_PATH = /^\/([^/]+(?:\/[^/]+)+)\/pulls\/(\d+)(?:\/|$)/u; +const COMMIT_PATH = /^\/([^/]+(?:\/[^/]+)+)\/commit\/([0-9a-f]+)\/?$/iu; const KIND = SourceControlProviderKind.make("forgejo"); @@ -82,6 +85,8 @@ export const definition = defineSourceControl({ return link === null ? null : { ...link, authority: url.host.toLowerCase() }; }, changeRequestUrlFromReferenceAutolink: () => null, + commitUrl: (input) => buildCommitUrl(input, "commit"), + parseCommitUrl: (url) => parseCommitPath(url, COMMIT_PATH), changeRequestActions: new Set(["merge", "close", "reopen", "update-branch"] as const), // Neither `fj` nor `tea` checks out by number. checkoutCommand: ({ number, repositoryUrl }) => diff --git a/packages/source-control-gitcafe/src/shared/definition.ts b/packages/source-control-gitcafe/src/shared/definition.ts index dc352be748dc2..a7b22261b2b4b 100644 --- a/packages/source-control-gitcafe/src/shared/definition.ts +++ b/packages/source-control-gitcafe/src/shared/definition.ts @@ -7,11 +7,13 @@ import { import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; import { + buildCommitUrl, changeRequestUrlBelow, defineSourceControl, isChangeRequestInProjectRepository, isChangeRequestOnProjectHost, parseChangeRequestPath, + parseCommitPath, projectChangeRequestHost, } from "@t3tools/source-control-core/shared/definition"; import { icon } from "./icon.ts"; @@ -21,6 +23,7 @@ const GITCAFE_HOSTS = new Set(["git.cafe", "staging.git.cafe"]); /** GitCafe serves pull requests at `/pull/` and redirects the older `/pulls/` there. */ const CHANGE_REQUEST_PATH = /^\/([^/]+\/[^/]+)\/pulls?\/(\d+)(?:\/|$)/u; +const COMMIT_PATH = /^\/([^/]+\/[^/]+)\/commit\/([0-9a-f]+)\/?$/iu; function parseChangeRequestUrl(url: URL) { return GITCAFE_HOSTS.has(url.hostname.toLowerCase()) @@ -68,6 +71,9 @@ export const definition = defineSourceControl({ // `/pull/` and `/pulls/` are GitHub's and Forgejo's shapes too, so only GitCafe's own hosts // claim them. parseChangeRequestUrl, + commitUrl: (input) => buildCommitUrl(input, "commit"), + parseCommitUrl: (url) => + GITCAFE_HOSTS.has(url.hostname.toLowerCase()) ? parseCommitPath(url, COMMIT_PATH) : null, changeRequestUrlFromReferenceAutolink: () => null, changeRequestActions: new Set([ "ready", diff --git a/packages/source-control-github/src/shared/definition.ts b/packages/source-control-github/src/shared/definition.ts index 965ed42814c19..6ac8aa305f3cd 100644 --- a/packages/source-control-github/src/shared/definition.ts +++ b/packages/source-control-github/src/shared/definition.ts @@ -1,11 +1,13 @@ import { makeProviderSettingsSchema, SourceControlProviderKind } from "@t3tools/contracts"; import { + buildCommitUrl, changeRequestUrlBelow, defineSourceControl, hostCredentialSettingsFields, isChangeRequestInProjectRepository, isChangeRequestOnProjectHost, parseChangeRequestPath, + parseCommitPath, projectChangeRequestHost, remoteWebOrigin, type SourceControlAppSignIn, @@ -22,15 +24,14 @@ const CHANGE_REQUEST_REFERENCE = // GitHub Enterprise Server runs on any hostname, so the `//pull/` route alone // identifies a GitHub pull request. const CHANGE_REQUEST_PATH = /^\/([^/]+\/[^/]+)\/pull\/(\d+)(?:\/|$)/u; +const COMMIT_PATH = /^\/([^/]+\/[^/]+)\/(?:commit|pull\/\d+\/commits)\/([0-9a-f]+)\/?$/iu; const ISSUE_PATH = /^\/([^/]+\/[^/]+)\/issues\/(\d+)(?:\/|$)/u; const KIND = SourceControlProviderKind.make("github"); function isGitHubHost(hostname: string): boolean { const host = hostname.toLowerCase(); - return ( - host === "github.com" || host.endsWith(".github.com") || host.split(".").includes("github") - ); + return host === "github.com" || host.endsWith(".ghe.com") || host.split(".").includes("github"); } function isRepositoryPath(repository: string): boolean { @@ -91,6 +92,11 @@ export const definition = defineSourceControl({ return null; } }, + commitUrl: (input) => + isRepositoryPath(input.repository) ? buildCommitUrl(input, "commit") : null, + // A known project's host can be any GitHub Enterprise Server name. + parseCommitUrl: (url, known) => + known || isGitHubHost(url.hostname) ? parseCommitPath(url, COMMIT_PATH) : null, parseChangeRequestUrl: (url) => parseChangeRequestPath(url, CHANGE_REQUEST_PATH), // GitHub writes every bare reference through `/issues/`, including pull requests. changeRequestUrlFromReferenceAutolink: (url) => { @@ -133,9 +139,7 @@ export const definition = defineSourceControl({ changeRequestHeadRef: (number) => `refs/pull/${number}/head`, // `*.ghe.com` is GitHub Enterprise Cloud with data residency, one subdomain per enterprise. recognizeRemote: ({ host, hostname }) => - hostname === "github.com" || - hostname.endsWith(".ghe.com") || - hostname.split(".").includes("github") + isGitHubHost(hostname) ? { kind: KIND, name: hostname === "github.com" ? "GitHub" : "GitHub Self-Hosted", diff --git a/packages/source-control-gitlab/src/shared/definition.ts b/packages/source-control-gitlab/src/shared/definition.ts index 437320c0d0419..2d280f01c62f2 100644 --- a/packages/source-control-gitlab/src/shared/definition.ts +++ b/packages/source-control-gitlab/src/shared/definition.ts @@ -1,10 +1,12 @@ import { SourceControlProviderKind } from "@t3tools/contracts"; import { + buildCommitUrl, changeRequestUrlBelow, defineSourceControl, isChangeRequestInProjectRepository, isChangeRequestOnProjectHost, parseChangeRequestPath, + parseCommitPath, projectChangeRequestHost, } from "@t3tools/source-control-core/shared/definition"; import { icon } from "./icon.ts"; @@ -15,6 +17,7 @@ const CHANGE_REQUEST_REFERENCE = // Self-managed GitLab runs on any hostname and nests projects in groups. const CHANGE_REQUEST_PATH = /^\/([^/]+(?:\/[^/]+)+)\/-\/merge_requests\/(\d+)(?:\/|$)/u; +const COMMIT_PATH = /^\/([^/]+(?:\/[^/]+)+)\/-\/commit\/([0-9a-f]+)\/?$/iu; const KIND = SourceControlProviderKind.make("gitlab"); @@ -40,6 +43,8 @@ export const definition = defineSourceControl({ ? changeRequestUrlBelow(webUrl, `-/merge_requests/${number}`) : `https://${host}/${repository}/-/merge_requests/${number}`, parseChangeRequestUrl: (url) => parseChangeRequestPath(url, CHANGE_REQUEST_PATH), + commitUrl: (input) => buildCommitUrl(input, "-/commit"), + parseCommitUrl: (url) => parseCommitPath(url, COMMIT_PATH), changeRequestUrlFromReferenceAutolink: () => null, changeRequestActions: new Set([ "merge",