Skip to content

Commit 9384aee

Browse files
ryancbahanclaude
andcommitted
Address PR review feedback on git.ts
- Preserve exitCode in gitCommand when wrapping errors in AbortError - Use null byte delimiter in getLatestGitCommit to handle multiline bodies - Narrow error handling in checkIfIgnoredInGitRepository (exitCode === 1) - Narrow error handling in insideGitDirectory (exitCode === 128) - Narrow error handling in getLatestTag (exitCode === 128) - Remove unused progressUpdater from GitCloneOptions interface - Add error message matcher to latestTag test assertion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7468292 commit 9384aee

2 files changed

Lines changed: 18 additions & 17 deletions

File tree

packages/cli-kit/src/public/node/git.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ describe('downloadRepository()', async () => {
109109
const latestTag = true
110110

111111
await git.downloadGitRepository({repoUrl, destination, latestTag})
112-
}).rejects.toThrowError()
112+
}).rejects.toThrowError(/fatal: No names found/)
113113
})
114114

115115
test('clones and checks out the latest tag', async () => {
@@ -220,7 +220,7 @@ describe('createGitIgnore()', () => {
220220

221221
describe('getLatestCommit()', () => {
222222
test('gets the latest commit through git log', async () => {
223-
mockGitCommand('abc123\n2024-01-01\ncommit message\nHEAD -> main\n\nJohn\njohn@test.com')
223+
mockGitCommand('abc123\x002024-01-01\x00commit message\x00HEAD -> main\x00\x00John\x00john@test.com')
224224

225225
const result = await git.getLatestGitCommit()
226226
expect(result.hash).toBe('abc123')
@@ -236,7 +236,7 @@ describe('getLatestCommit()', () => {
236236

237237
test('passes the directory option', async () => {
238238
const directory = '/test/directory'
239-
mockGitCommand('abc123\n2024-01-01\nmsg\nrefs\n\nJohn\njohn@test.com')
239+
mockGitCommand('abc123\x002024-01-01\x00msg\x00refs\x00\x00John\x00john@test.com')
240240

241241
await git.getLatestGitCommit(directory)
242242

packages/cli-kit/src/public/node/git.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ async function gitCommand(args: string[], directory?: string): Promise<string> {
3535
if (err instanceof Error) {
3636
const abortError = new AbortError(err.message)
3737
abortError.stack = err.stack
38+
if ('exitCode' in err) {
39+
Object.assign(abortError, {exitCode: err.exitCode})
40+
}
3841
throw abortError
3942
}
4043
throw err
@@ -70,7 +73,7 @@ export async function checkIfIgnoredInGitRepository(directory: string, files: st
7073
return stdout.split('\n').filter(Boolean)
7174
} catch (error) {
7275
// git check-ignore exits with code 1 when no files are ignored
73-
if (error instanceof AbortError) return []
76+
if (error instanceof AbortError && 'exitCode' in error && error.exitCode === 1) return []
7477
throw error
7578
}
7679
}
@@ -138,14 +141,12 @@ export function addToGitIgnore(root: string, entry: string): void {
138141
*
139142
* @param repoUrl - The URL of the repository to clone.
140143
* @param destination - The directory where the repository will be cloned.
141-
* @param progressUpdater - A function that will be called with the progress of the clone.
142144
* @param shallow - Whether to clone the repository shallowly.
143145
* @param latestTag - Whether to clone the latest tag instead of the default branch.
144146
*/
145147
export interface GitCloneOptions {
146148
repoUrl: string
147149
destination: string
148-
progressUpdater?: (statusString: string) => void
149150
shallow?: boolean
150151
latestTag?: boolean
151152
}
@@ -249,7 +250,7 @@ async function getLatestTagFromDirectory(directory: string, repoUrl: string): Pr
249250
* @returns The latest commit of the repository.
250251
*/
251252
export async function getLatestGitCommit(directory?: string): Promise<GitLogEntry> {
252-
const format = '%H%n%ai%n%s%n%D%n%b%n%an%n%ae'
253+
const format = '%H%x00%ai%x00%s%x00%D%x00%b%x00%an%x00%ae'
253254
const stdout = await gitCommand(['log', '-1', `--format=${format}`], directory)
254255
if (!stdout.trim()) {
255256
throw new AbortError(
@@ -259,15 +260,15 @@ export async function getLatestGitCommit(directory?: string): Promise<GitLogEntr
259260
)} to create your first commit.`,
260261
)
261262
}
262-
const lines = stdout.split('\n')
263+
const parts = stdout.split('\x00')
263264
return {
264-
hash: lines[0]!,
265-
date: lines[1]!,
266-
message: lines[2]!,
267-
refs: lines[3]!,
268-
body: lines[4]!,
269-
author_name: lines[5]!,
270-
author_email: lines[6]!,
265+
hash: parts[0]!,
266+
date: parts[1]!,
267+
message: parts[2]!,
268+
refs: parts[3]!,
269+
body: parts[4]!,
270+
author_name: parts[5]!,
271+
author_email: parts[6]!,
271272
}
272273
}
273274

@@ -365,7 +366,7 @@ export async function insideGitDirectory(directory?: string): Promise<boolean> {
365366
await execa('git', ['rev-parse', '--git-dir'], {cwd: directory})
366367
return true
367368
} catch (error) {
368-
if (error instanceof Error && 'exitCode' in error) return false
369+
if (error instanceof Error && 'exitCode' in error && error.exitCode === 128) return false
369370
throw error
370371
}
371372
}
@@ -405,7 +406,7 @@ export async function getLatestTag(directory?: string): Promise<string | undefin
405406
const stdout = await gitCommand(['describe', '--tags', '--abbrev=0'], directory)
406407
return stdout.trim() || undefined
407408
} catch (error) {
408-
if (error instanceof AbortError) return undefined
409+
if (error instanceof AbortError && 'exitCode' in error && error.exitCode === 128) return undefined
409410
throw error
410411
}
411412
}

0 commit comments

Comments
 (0)