From 6f16913007b9b869d00367ee4a0dcbf5241fe417 Mon Sep 17 00:00:00 2001 From: Paulo Carinhena <63272749+paulocarinhena@users.noreply.github.com> Date: Fri, 13 Nov 2026 22:35:38 -0300 Subject: [PATCH] =?UTF-8?q?Evita=20commit=20em=20HEAD=20destacado,=20diff?= =?UTF-8?q?=20da=20sele=C3=A7=C3=A3o=20errada,=20remote=20de=20fork=20ocul?= =?UTF-8?q?to,=20leitura=20fora=20do=20reposit=C3=B3rio=20e=20inbox=20para?= =?UTF-8?q?da=20por=20estrelas=20incompletas.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Cursor --- GitWeb/src/main/gitProcess.ts | 87 ++++++++++++++----- GitWeb/src/main/githubApi.ts | 67 +++++++++------ GitWeb/src/renderer/src/store/inboxStore.ts | 8 +- GitWeb/src/renderer/src/store/repoStore.ts | 41 ++++++--- GitWeb/src/shared/git.ts | 18 ++++ GitWeb/test/branchIntegration.test.ts | 28 +++++++ GitWeb/test/gitProcess.test.ts | 92 ++++++++++++++++++++- GitWeb/test/inbox.test.ts | 32 +++++++ GitWeb/test/repoStatus.test.ts | 14 +++- GitWeb/test/selectFileRace.test.ts | 78 +++++++++++++++++ 10 files changed, 405 insertions(+), 60 deletions(-) create mode 100644 GitWeb/test/selectFileRace.test.ts diff --git a/GitWeb/src/main/gitProcess.ts b/GitWeb/src/main/gitProcess.ts index 4b28f97..2cf5640 100644 --- a/GitWeb/src/main/gitProcess.ts +++ b/GitWeb/src/main/gitProcess.ts @@ -443,25 +443,42 @@ export async function getStatus(cwd: string): Promise { const header = headerEnd >= 0 ? raw.slice(0, headerEnd) : raw const rest = headerEnd >= 0 ? raw.slice(headerEnd + 1) : '' + const { branch, upstream, ahead, behind } = parseStatusBranchHeader(header) + + const mergeHead = await readGit(['rev-parse', '-q', '--verify', 'MERGE_HEAD'], cwd) + return { entries: parseStatusEntries(rest), branch, ahead, behind, upstream, mergeInProgress: mergeHead.exitCode === 0 } +} + +/** + * The `## ` record from `git status -b`. Branch names may contain dots + * (`fix/1.0.0`); the upstream separator is the literal `...`, not the first dot. + * Detached HEAD is `## HEAD (no branch)`. The first space must not become the + * branch name — `HEAD` is not a branch, and publishing it would commit onto + * nothing and then try to push a branch called HEAD. + */ +export function parseStatusBranchHeader(header: string): { + branch: string | null + upstream: string | null + ahead: number + behind: number +} { let branch: string | null = null let upstream: string | null = null - let ahead = 0 - let behind = 0 - - // Branch names may contain dots (e.g. fix/1.0.0). The upstream separator is - // the literal "..." sequence, not the first "." in the branch name. - const headerMatch = header.match(/^## (?:No commits yet on )?(.+?)(?:\.\.\.(\S+))?(?:\s|$)/) - if (headerMatch) { - branch = (headerMatch[1] ?? null)?.trim() || null - upstream = headerMatch[2] ?? null + if (!/^## HEAD \(no branch\)/.test(header)) { + const headerMatch = header.match(/^## (?:No commits yet on )?(.+?)(?:\.\.\.(\S+))?(?:\s|$)/) + if (headerMatch) { + branch = headerMatch[1]?.trim() || null + upstream = headerMatch[2] ?? null + } } const aheadMatch = header.match(/ahead (\d+)/) const behindMatch = header.match(/behind (\d+)/) - if (aheadMatch) ahead = Number(aheadMatch[1]) - if (behindMatch) behind = Number(behindMatch[1]) - - const mergeHead = await readGit(['rev-parse', '-q', '--verify', 'MERGE_HEAD'], cwd) - return { entries: parseStatusEntries(rest), branch, ahead, behind, upstream, mergeInProgress: mergeHead.exitCode === 0 } + return { + branch, + upstream, + ahead: aheadMatch ? Number(aheadMatch[1]) : 0, + behind: behindMatch ? Number(behindMatch[1]) : 0 + } } export async function stageFiles(paths: string[], cwd: string): Promise { @@ -655,9 +672,26 @@ export async function getRemoteBranches(cwd: string): Promise { .map(([name]) => name) } -/** Creates the local branch tracking `origin/` and switches to it. */ +/** + * Creates the local branch tracking `remoteBranch` (`origin/feature`) and + * switches to it. `git checkout --track` names the local branch after the + * part following the first slash, so `upstream/main` would collide with an + * existing `main` and be refused. In that case the local branch keeps the + * full remote name (`upstream/main`), which is a different ref. `origin/*` + * keeps the old refusal: a local `feature` already is that branch. + */ export async function checkoutRemoteBranch(remoteBranch: string, cwd: string): Promise { assertNotOption(remoteBranch, tMain().git.optionLabel.remoteBranch) + const slash = remoteBranch.indexOf('/') + const remoteName = slash === -1 ? '' : remoteBranch.slice(0, slash) + const short = slash === -1 ? remoteBranch : remoteBranch.slice(slash + 1) + const shortTaken = short !== remoteBranch && (await refExists(cwd, `refs/heads/${short}`)) + if (shortTaken && remoteName !== 'origin') { + if (await refExists(cwd, `refs/heads/${remoteBranch}`)) return runCheckout(['checkout', remoteBranch], cwd) + // The start point is the full remote-tracking ref, so a local name equal + // to `upstream/main` is not read as the remote ref itself. + return runCheckout(['checkout', '-b', remoteBranch, '--track', `refs/remotes/${remoteBranch}`], cwd) + } return runCheckout(['checkout', '--track', remoteBranch], cwd) } @@ -1184,14 +1218,24 @@ export async function getBranchIntegration( }) const localNames = new Set(refs.filter((r) => r.ref.startsWith('refs/heads/')).map((r) => r.name)) + const localTipByName = new Map(refs.filter((r) => r.ref.startsWith('refs/heads/')).map((r) => [r.name, r.tip])) const candidates = refs.filter((r) => { if (onlyLocal !== undefined) return r.ref === `refs/heads/${onlyLocal}` if (r.symref || r.ref === baseRef.ref) return false if (r.ref.startsWith('refs/heads/')) return r.name !== baseShort - // Remote: skip the remote's copy of the base and anything already covered - // by a local branch of the same name. - const branchName = r.name.slice(r.name.indexOf('/') + 1) - return branchName !== baseShort && !localNames.has(branchName) + // Remote: `origin/feature` is the local `feature` looked at from the other + // side, so it is not a second branch. `upstream/feature` is, unless it + // points at the same commit — a fork's main can be ahead of origin/main + // while sharing the short name. + const slash = r.name.indexOf('/') + const remoteName = slash === -1 ? '' : r.name.slice(0, slash) + const branchName = slash === -1 ? r.name : r.name.slice(slash + 1) + if (localNames.has(r.name)) return false + if (refs.some((local) => local.ref.startsWith('refs/heads/') && local.upstream === r.name)) return false + if (remoteName === 'origin' && (branchName === baseShort || localNames.has(branchName))) return false + const localTip = localTipByName.get(branchName) + if (localTip && localTip === r.tip) return false + return true }) // Several branches usually fork from the same base commit — scan it once. @@ -1665,7 +1709,10 @@ async function resolveRealWithinRepo(cwd: string, relativePath: string): Promise } export async function readRepoFile(cwd: string, relativePath: string): Promise { - return readFile(resolveWithinRepo(cwd, relativePath), 'utf-8') + // Same real-path check as writeRepoFile. A lexical join follows a symlink + // (`notes.txt` → `~/.ssh/id_rsa`) out of the repo, and this reader feeds the + // diff, conflict resolution and the fix proposal that goes to the model. + return readFile(await resolveRealWithinRepo(cwd, relativePath), 'utf-8') } // --- Files the AI agent is allowed to see ----------------------------------- diff --git a/GitWeb/src/main/githubApi.ts b/GitWeb/src/main/githubApi.ts index 3942fcd..d31f85f 100644 --- a/GitWeb/src/main/githubApi.ts +++ b/GitWeb/src/main/githubApi.ts @@ -108,13 +108,25 @@ async function send( return { data: (await response.json()) as T, next } } -/** GraphQL answers 200 even on failure, with the reasons in `errors`. */ -async function graphql(host: string, token: string, query: string, variables: Record): Promise { +/** + * GraphQL answers 200 even on failure, with the reasons in `errors`. + * `partial` keeps a payload that arrived next to those errors. Stars need it: + * one repository the token cannot see comes back as a null node plus an error, + * and throwing there used to drop every star and fail the whole inbox refresh. + * Callers that require a complete answer leave it off. + */ +async function graphql( + host: string, + token: string, + query: string, + variables: Record, + partial = false +): Promise { const { data } = await send<{ data?: T; errors?: { message: string }[] }>(graphqlUrlForHost(host), token, { method: 'POST', body: JSON.stringify({ query, variables }) }) - if (data.errors?.length || !data.data) { + if (!data.data || (data.errors?.length && !partial)) { throw new GitHubApiError(data.errors?.map((e) => e.message).join(' — ') || tMain().github.requestRejected(200), 200) } return data.data @@ -954,35 +966,44 @@ const RECENT_STARS_LIMIT = 20 * themselves: the latest few of each is enough to catch up between checks. * Costs a single GraphQL request however many repositories there are. */ +interface ApiStarRepository { + nameWithOwner?: string + stargazers?: { + edges?: ({ starredAt?: string; node?: { login?: string; avatarUrl?: string; url?: string } | null } | null)[] | null + } | null +} + export function getRecentStars(host: string): Promise { return whenSignedIn(host, async (token) => { const { viewer } = await graphql<{ - viewer: { - login: string - repositories: { - nodes: { - nameWithOwner: string - stargazers: { edges: { starredAt: string; node: { login: string; avatarUrl: string; url: string } }[] } - }[] - } - } + viewer: { login?: string; repositories?: { nodes?: (ApiStarRepository | null)[] | null } | null } | null }>( host, token, 'query { viewer { login repositories(first: 100, ownerAffiliations: [OWNER, ORGANIZATION_MEMBER], orderBy: { field: PUSHED_AT, direction: DESC }) { nodes { nameWithOwner stargazers(first: 5, orderBy: { field: STARRED_AT, direction: DESC }) { edges { starredAt node { login avatarUrl url } } } } } } }', - {} + {}, + true ) + if (!viewer?.login) throw new GitHubApiError(tMain().github.requestRejected(200), 200) const since = Date.now() - RECENT_STARS_DAYS * 24 * 60 * 60 * 1000 - return viewer.repositories.nodes - .flatMap((repository) => - repository.stargazers.edges.map((edge) => ({ - repoFullName: repository.nameWithOwner, - login: edge.node.login, - avatarUrl: edge.node.avatarUrl, - profileUrl: edge.node.url, - starredAt: edge.starredAt - })) - ) + return (viewer.repositories?.nodes ?? []) + .flatMap((repository) => { + const repoFullName = repository?.nameWithOwner + if (!repoFullName) return [] + return (repository.stargazers?.edges ?? []).flatMap((edge) => { + const login = edge?.node?.login + if (!edge?.starredAt || !login) return [] + return [ + { + repoFullName, + login, + avatarUrl: edge.node?.avatarUrl ?? '', + profileUrl: edge.node?.url ?? '', + starredAt: edge.starredAt + } + ] + }) + }) .filter((star) => star.login !== viewer.login && Date.parse(star.starredAt) >= since) .sort((a, b) => Date.parse(b.starredAt) - Date.parse(a.starredAt)) .slice(0, RECENT_STARS_LIMIT) diff --git a/GitWeb/src/renderer/src/store/inboxStore.ts b/GitWeb/src/renderer/src/store/inboxStore.ts index f4ec614..18ae2fe 100644 --- a/GitWeb/src/renderer/src/store/inboxStore.ts +++ b/GitWeb/src/renderer/src/store/inboxStore.ts @@ -146,14 +146,18 @@ export const useInboxStore = create((set, get) => { if (generation !== loadGeneration) return const signedOut = invitations.status === 'fulfilled' && invitations.value === null const failure = [invitations, notifications, stars].find((result) => result.status === 'rejected') + // A star query that GitHub only half answers must not freeze "last + // checked". The timestamp moves when anything came back; it stays put + // only when every source failed, so an offline startup retries from the + // same clock. The error for the source that failed is still shown. + const delivered = [invitations, notifications, stars].some((result) => result.status === 'fulfilled') set((state) => ({ invitations: invitations.status === 'fulfilled' ? (invitations.value ?? []) : state.invitations, notifications: signedOut ? [] : notifications.status === 'fulfilled' ? (notifications.value ?? []) : state.notifications, stars: signedOut ? [] : stars.status === 'fulfilled' ? (stars.value ?? []) : state.stars, signedIn: invitations.status === 'fulfilled' ? !signedOut : state.signedIn, - // Offline at startup, say: what was on screen stays, and the next check tries again. error: failure ? describeError(failure.reason) : null, - checkedAt: failure ? state.checkedAt : Date.now(), + checkedAt: delivered ? Date.now() : state.checkedAt, isLoading: false })) announce(get()) diff --git a/GitWeb/src/renderer/src/store/repoStore.ts b/GitWeb/src/renderer/src/store/repoStore.ts index 2adf762..02082e1 100644 --- a/GitWeb/src/renderer/src/store/repoStore.ts +++ b/GitWeb/src/renderer/src/store/repoStore.ts @@ -1,7 +1,7 @@ import { create } from 'zustand' import type { PublishRepositoryInput, PublishRepositoryResult } from '../../../shared/github' import type { ReorganizeResult, RepoMove } from '../../../shared/repoOrganize' -import { isConflicted, isHistoryRewrittenMessage, sameRepoStatus, stashBranchName } from '../../../shared/git' +import { isConflicted, isHistoryRewrittenMessage, remoteBranchesToOffer, sameRepoStatus, stashBranchName } from '../../../shared/git' import type { BranchInfo, CommitFile, @@ -249,6 +249,11 @@ let repoOpenGeneration = 0 // last one applied may land. let statusRequestSeq = 0 let statusAppliedSeq = 0 +// A diff or commit-file read started for an earlier click can resolve after +// the next one. Only the latest click may write the pane. +let selectFileSeq = 0 +let selectCommitSeq = 0 +let selectCommitFileSeq = 0 export const useRepoStore = create((set, get) => ({ currentRepoPath: null, @@ -608,7 +613,7 @@ export const useRepoStore = create((set, get) => ({ ]) if (generation !== repoOpenGeneration || get().currentRepoPath !== path) return const local = new Set(branches.map((b) => b.name)) - set({ branches, remoteBranches: remote.filter((r) => !local.has(r.slice(r.indexOf('/') + 1))) }) + set({ branches, remoteBranches: remoteBranchesToOffer(remote, branches.map((b) => b.name)) }) } catch (err) { if (generation !== repoOpenGeneration || get().currentRepoPath !== path) return set({ error: describeError(err) }) @@ -660,15 +665,27 @@ export const useRepoStore = create((set, get) => ({ if (!currentRepoPath) return const repoPath = currentRepoPath const generation = repoOpenGeneration + const request = ++selectFileSeq const entry = status?.entries.find((e) => e.path === path) const conflicted = entry ? isConflicted(entry) : false set({ selectedFile: { path, staged, conflicted }, diffText: '', conflictContent: null }) + const stillCurrent = (): boolean => { + const selected = get().selectedFile + return ( + request === selectFileSeq && + generation === repoOpenGeneration && + get().currentRepoPath === repoPath && + selected?.path === path && + selected.staged === staged + ) + } + try { if (conflicted) { const content = await window.api.fs.readRepoFile(repoPath, path) - if (generation !== repoOpenGeneration || get().currentRepoPath !== repoPath) return + if (!stillCurrent()) return set({ conflictContent: content }) return } @@ -677,6 +694,7 @@ export const useRepoStore = create((set, get) => ({ // "/" (it doesn't recurse into it) — that's not a file, so reading it // would throw EISDIR. if (path.endsWith('/')) { + if (!stillCurrent()) return set({ diffText: getT().stores.untrackedFolder }) return } @@ -685,16 +703,16 @@ export const useRepoStore = create((set, get) => ({ if (isNewUntracked) { // `git diff` shows nothing for untracked files; read the raw content instead. const content = await window.api.fs.readRepoFile(repoPath, path) - if (generation !== repoOpenGeneration || get().currentRepoPath !== repoPath) return + if (!stillCurrent()) return const preview = content.length > 200_000 ? `${content.slice(0, 200_000)}\n${getT().stores.fileTruncated}` : content set({ diffText: preview || getT().stores.emptyNewFile }) } else { const diffText = await window.api.git.diff(path, repoPath, staged) - if (generation !== repoOpenGeneration || get().currentRepoPath !== repoPath) return + if (!stillCurrent()) return set({ diffText }) } } catch (err) { - if (generation !== repoOpenGeneration || get().currentRepoPath !== repoPath) return + if (!stillCurrent()) return set({ error: describeError(err) }) } }, @@ -1346,9 +1364,10 @@ export const useRepoStore = create((set, get) => ({ if (!currentRepoPath) return const path = currentRepoPath const generation = repoOpenGeneration + const request = ++selectCommitSeq try { const files = await window.api.git.commitFiles(hash, path) - if (generation !== repoOpenGeneration || get().currentRepoPath !== path) return + if (request !== selectCommitSeq || generation !== repoOpenGeneration || get().currentRepoPath !== path) return set({ selectedCommit: { hash, files, selectedPath: null }, commitDiffText: '' }) // Open straight on the first file, as a PR does, instead of an empty // pane asking for a click. With a file filter active, that file. @@ -1356,7 +1375,7 @@ export const useRepoStore = create((set, get) => ({ const first = files.find((f) => logPath && f.path.split('\t').pop() === logPath) ?? files[0] if (first) void get().selectCommitFile(first.path) } catch (err) { - if (generation !== repoOpenGeneration || get().currentRepoPath !== path) return + if (request !== selectCommitSeq || generation !== repoOpenGeneration || get().currentRepoPath !== path) return set({ error: describeError(err) }) } }, @@ -1371,14 +1390,16 @@ export const useRepoStore = create((set, get) => ({ const repoPath = currentRepoPath const generation = repoOpenGeneration const commitHash = selectedCommit.hash + const request = ++selectCommitFileSeq + set({ selectedCommit: { ...selectedCommit, selectedPath: path }, commitDiffText: '' }) try { const commitDiffText = await window.api.git.commitDiff(commitHash, path, repoPath) - if (generation !== repoOpenGeneration || get().currentRepoPath !== repoPath) return + if (request !== selectCommitFileSeq || generation !== repoOpenGeneration || get().currentRepoPath !== repoPath) return const current = get().selectedCommit if (!current || current.hash !== commitHash) return set({ selectedCommit: { ...current, selectedPath: path }, commitDiffText }) } catch (err) { - if (generation !== repoOpenGeneration || get().currentRepoPath !== repoPath) return + if (request !== selectCommitFileSeq || generation !== repoOpenGeneration || get().currentRepoPath !== repoPath) return set({ error: describeError(err) }) } }, diff --git a/GitWeb/src/shared/git.ts b/GitWeb/src/shared/git.ts index 9299e95..1b391cc 100644 --- a/GitWeb/src/shared/git.ts +++ b/GitWeb/src/shared/git.ts @@ -446,6 +446,24 @@ export function isHistoryRewrittenMessage(message: string): boolean { * local name (main tracking origin/master) or even live on origin (a fork's * remote); with none yet, the app publishes the branch to origin as is. */ +/** + * Remote-tracking names worth offering for checkout. `origin/feature` is hidden + * once a local `feature` exists — that checkout would only recreate it. + * `upstream/feature` stays: a local `feature` does not contain the other + * remote's commits. A local branch already named `upstream/feature` covers it. + */ +export function remoteBranchesToOffer(remoteBranches: readonly string[], localNames: readonly string[]): string[] { + const local = new Set(localNames) + return remoteBranches.filter((remote) => { + if (local.has(remote)) return false + const slash = remote.indexOf('/') + if (slash <= 0) return true + const remoteName = remote.slice(0, slash) + const short = remote.slice(slash + 1) + return !(remoteName === 'origin' && local.has(short)) + }) +} + export function pushTargetBranch(status: Pick): string | null { if (!status.branch) return null if (!status.upstream) return status.branch diff --git a/GitWeb/test/branchIntegration.test.ts b/GitWeb/test/branchIntegration.test.ts index 9d2fe7c..e62fe77 100644 --- a/GitWeb/test/branchIntegration.test.ts +++ b/GitWeb/test/branchIntegration.test.ts @@ -135,3 +135,31 @@ describe('deleteCheckedBranch', () => { expect(git(repo, 'branch', '--list', 'pending')).toBe('') }) }) + +describe('a second remote', () => { + it('omits origin/pending and an upstream tip that matches, and reports one that diverged', async () => { + git(repo, 'remote', 'add', 'origin', repo) + git(repo, 'remote', 'add', 'upstream', repo) + git(repo, 'update-ref', 'refs/remotes/origin/pending', 'pending') + git(repo, 'branch', '-q', '--set-upstream-to', 'origin/pending', 'pending') + git(repo, 'update-ref', 'refs/remotes/upstream/pending', 'pending') + + const same = (await getBranchIntegration(repo, 'main')).branches.map((b) => b.name) + expect(same).not.toContain('origin/pending') + expect(same).not.toContain('upstream/pending') + expect(same).toContain('pending') + + git(repo, 'checkout', '-q', '--detach', 'pending') + commitFile(repo, 'remote-only.txt', 'remote\n', 'only on upstream') + const diverged = git(repo, 'rev-parse', 'HEAD') + git(repo, 'update-ref', 'refs/remotes/upstream/pending', diverged) + git(repo, 'checkout', '-q', 'main') + + const report = await getBranchIntegration(repo, 'main') + expect(report.branches.map((b) => b.name)).not.toContain('origin/pending') + expect(report.branches.find((b) => b.name === 'upstream/pending')).toMatchObject({ + remoteOnly: true, + state: 'unmerged' + }) + }) +}) diff --git a/GitWeb/test/gitProcess.test.ts b/GitWeb/test/gitProcess.test.ts index d762ae2..e2576e1 100644 --- a/GitWeb/test/gitProcess.test.ts +++ b/GitWeb/test/gitProcess.test.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'child_process' -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs' +import { existsSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'fs' import { tmpdir } from 'os' import { join } from 'path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -7,9 +7,20 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => ({ shell: { trashItem: vi.fn() } })) vi.mock('../src/main/credentials', () => ({ getSecret: vi.fn(async () => null) })) -const { cloneRepository, isCommitPublished, locateFileInHistory, parseStatusEntries, pushBranch, readGit, runGit, undoUnpublishedCommit } = await import( - '../src/main/gitProcess' -) +const { + checkoutRemoteBranch, + cloneRepository, + getStatus, + isCommitPublished, + locateFileInHistory, + parseStatusBranchHeader, + parseStatusEntries, + pushBranch, + readGit, + readRepoFile, + runGit, + undoUnpublishedCommit +} = await import('../src/main/gitProcess') function git(cwd: string, ...args: string[]): string { return execFileSync('git', args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim() @@ -317,3 +328,76 @@ describe('with real repositories', () => { expect(info.inIndex).toBe(true) }) }) + +describe('parseStatusBranchHeader', () => { + it('keeps dotted branch names and the upstream that follows ...', () => { + expect(parseStatusBranchHeader('## feature/1.2.3...origin/feature/1.2.3 [ahead 1, behind 2]')).toEqual({ + branch: 'feature/1.2.3', + upstream: 'origin/feature/1.2.3', + ahead: 1, + behind: 2 + }) + expect(parseStatusBranchHeader('## No commits yet on main')).toMatchObject({ branch: 'main', upstream: null }) + }) + + it('does not turn a detached HEAD into a branch named HEAD', () => { + expect(parseStatusBranchHeader('## HEAD (no branch)')).toEqual({ + branch: null, + upstream: null, + ahead: 0, + behind: 0 + }) + }) +}) + +describe('detached HEAD and symlink reads', () => { + let repo: string + + beforeEach(() => { + repo = mkdtempSync(join(tmpdir(), 'ogd-status-')) + git(repo, 'init', '-q', '-b', 'main') + git(repo, 'config', 'user.name', 'Test') + git(repo, 'config', 'user.email', 'test@example.com') + commitFile(repo, 'a.txt', 'a\n', 'initial') + }) + + afterEach(() => { + rmSync(repo, { recursive: true, force: true }) + }) + + it('reports no branch when HEAD is detached', async () => { + git(repo, 'checkout', '-q', '--detach') + expect((await getStatus(repo)).branch).toBeNull() + }) + + it('reads a file inside the repo and refuses a link that leaves it', async () => { + expect(await readRepoFile(repo, 'a.txt')).toBe('a\n') + const outside = mkdtempSync(join(tmpdir(), 'ogd-outside-')) + try { + writeFileSync(join(outside, 'secret.txt'), 'secret') + symlinkSync(outside, join(repo, 'leak'), process.platform === 'win32' ? 'junction' : 'dir') + await expect(readRepoFile(repo, 'leak/secret.txt')).rejects.toThrow(/escapes the repository root/) + } finally { + rmSync(outside, { recursive: true, force: true }) + } + }) + + it('checks out origin/feature as feature, and upstream/main beside an existing main', async () => { + git(repo, 'remote', 'add', 'origin', repo) + git(repo, 'remote', 'add', 'upstream', repo) + git(repo, 'update-ref', 'refs/remotes/origin/feature', 'HEAD') + git(repo, 'update-ref', 'refs/remotes/upstream/main', 'HEAD') + + expect((await checkoutRemoteBranch('origin/feature', repo)).switched).toBe(true) + expect(git(repo, 'branch', '--show-current')).toBe('feature') + + git(repo, 'checkout', '-q', 'main') + expect((await checkoutRemoteBranch('upstream/main', repo)).switched).toBe(true) + expect(git(repo, 'branch', '--show-current')).toBe('upstream/main') + // The short name is ambiguous with the local branch, so git prints + // remotes/upstream/main. The upstream ref itself is still the remote one. + expect(git(repo, 'rev-parse', '@{upstream}')).toBe(git(repo, 'rev-parse', 'refs/remotes/upstream/main')) + expect((await getStatus(repo)).branch).toBe('upstream/main') + expect(git(repo, 'rev-parse', 'main')).toBe(git(repo, 'rev-parse', 'upstream/main')) + }) +}) diff --git a/GitWeb/test/inbox.test.ts b/GitWeb/test/inbox.test.ts index 3244bff..4565e6c 100644 --- a/GitWeb/test/inbox.test.ts +++ b/GitWeb/test/inbox.test.ts @@ -87,6 +87,7 @@ beforeEach(() => { signedIn: null, isLoading: false, answeringId: null, + checkedAt: null, error: null, notice: null }) @@ -155,6 +156,25 @@ describe('getRecentStars', () => { vi.mocked(getSecret).mockResolvedValue(null) await expect(getRecentStars('github.com')).resolves.toBeNull() }) + + it('keeps the stars it could read when a repository node is missing', async () => { + respond(200, { + data: { + viewer: { + login: 'me', + repositories: { + nodes: [ + null, + { nameWithOwner: 'me/app', stargazers: { edges: [edge('ana', daysAgo(1))] } }, + { nameWithOwner: 'me/hidden', stargazers: null } + ] + } + } + }, + errors: [{ message: 'Could not resolve a repository' }] + }) + await expect(getRecentStars('github.com')).resolves.toMatchObject([{ login: 'ana', repoFullName: 'me/app' }]) + }) }) describe('getReceivedInvitations', () => { @@ -274,12 +294,24 @@ describe('inbox store', () => { const one = invitation() github.getReceivedInvitations.mockResolvedValue([one]) github.getRecentStars.mockRejectedValue(new Error('stars unavailable')) + useInboxStore.setState({ checkedAt: 1 }) await store().refresh() expect(store().invitations).toEqual([one]) expect(store().error).toBe('stars unavailable') + expect(store().checkedAt).toBeGreaterThan(1) expect(app.showNotification).toHaveBeenCalledTimes(1) }) + it('does not move the clock when every source fails', async () => { + useInboxStore.setState({ checkedAt: 1 }) + github.getReceivedInvitations.mockRejectedValue(new Error('offline')) + github.getNotifications.mockRejectedValue(new Error('offline')) + github.getRecentStars.mockRejectedValue(new Error('offline')) + await store().refresh() + expect(store().checkedAt).toBe(1) + expect(store().error).toBe('offline') + }) + it('announces new activity on a notification it already announced', async () => { github.getReceivedInvitations.mockResolvedValue([]) github.getNotifications.mockResolvedValue([notification('thread-1')]) diff --git a/GitWeb/test/repoStatus.test.ts b/GitWeb/test/repoStatus.test.ts index 84e52fa..08d61e7 100644 --- a/GitWeb/test/repoStatus.test.ts +++ b/GitWeb/test/repoStatus.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { sameRepoStatus, type RepoStatus } from '../src/shared/git' +import { remoteBranchesToOffer, sameRepoStatus, type RepoStatus } from '../src/shared/git' const base: RepoStatus = { entries: [ @@ -45,3 +45,15 @@ describe('sameRepoStatus', () => { expect(sameRepoStatus(base, reordered)).toBe(false) }) }) + +describe('remoteBranchesToOffer', () => { + const remote = ['origin/main', 'origin/feature', 'upstream/main', 'upstream/feature'] + + it('hides origin branches that already have a local branch, and keeps the other remote', () => { + expect(remoteBranchesToOffer(remote, ['main', 'feature'])).toEqual(['upstream/main', 'upstream/feature']) + }) + + it('hides a remote once a local branch is already named after it', () => { + expect(remoteBranchesToOffer(remote, ['main', 'upstream/main'])).toEqual(['origin/feature', 'upstream/feature']) + }) +}) diff --git a/GitWeb/test/selectFileRace.test.ts b/GitWeb/test/selectFileRace.test.ts new file mode 100644 index 0000000..6dad4e5 --- /dev/null +++ b/GitWeb/test/selectFileRace.test.ts @@ -0,0 +1,78 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { RepoStatus } from '../src/shared/git' + +const diff = vi.fn() +const commitDiff = vi.fn() +const readRepoFile = vi.fn() +vi.stubGlobal('window', { api: { git: { diff, commitDiff }, fs: { readRepoFile } } }) +vi.stubGlobal('navigator', { languages: ['pt-BR'] }) + +const { useRepoStore } = await import('../src/renderer/src/store/repoStore') + +function deferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve: (value: T) => void = () => undefined + const promise = new Promise((res) => { + resolve = res + }) + return { promise, resolve } +} + +const status: RepoStatus = { + entries: [ + { path: 'a.ts', index: ' ', worktree: 'M' }, + { path: 'b.ts', index: ' ', worktree: 'M' } + ], + branch: 'main', + ahead: 0, + behind: 0, + upstream: 'origin/main' +} + +beforeEach(() => { + diff.mockReset() + commitDiff.mockReset() + useRepoStore.setState({ + currentRepoPath: '/repo', + status, + selectedFile: null, + diffText: '', + conflictContent: null, + error: null, + selectedCommit: { hash: 'abc', files: [{ path: 'a.ts', status: 'M' }, { path: 'b.ts', status: 'M' }], selectedPath: null }, + commitDiffText: '' + }) +}) + +describe('selection races', () => { + it('keeps the diff of the file clicked last', async () => { + const first = deferred() + const second = deferred() + diff.mockImplementation((path: string) => (path === 'a.ts' ? first.promise : second.promise)) + + const selectA = useRepoStore.getState().selectFile('a.ts', false) + const selectB = useRepoStore.getState().selectFile('b.ts', false) + second.resolve('diff-b') + await selectB + first.resolve('diff-a') + await selectA + + expect(useRepoStore.getState().selectedFile).toMatchObject({ path: 'b.ts', staged: false }) + expect(useRepoStore.getState().diffText).toBe('diff-b') + }) + + it('keeps the commit diff of the file clicked last', async () => { + const first = deferred() + const second = deferred() + commitDiff.mockImplementation((_hash: string, path: string) => (path === 'a.ts' ? first.promise : second.promise)) + + const selectA = useRepoStore.getState().selectCommitFile('a.ts') + const selectB = useRepoStore.getState().selectCommitFile('b.ts') + second.resolve('diff-b') + await selectB + first.resolve('diff-a') + await selectA + + expect(useRepoStore.getState().selectedCommit?.selectedPath).toBe('b.ts') + expect(useRepoStore.getState().commitDiffText).toBe('diff-b') + }) +})