Skip to content

Commit 84b7d28

Browse files
amoscickiclaude
amoscicki
and
claude
committed
refactor: extract withTempGitRemote helper for code reuse
Centralizes the temp remote add/remove pattern into a shared helper function in common.ts, used by both check.ts and pull.ts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 60be9d5 commit 84b7d28

File tree

3 files changed

+111
-118
lines changed

3 files changed

+111
-118
lines changed

‎apps/server/src/routes/updates/common.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { exec } from 'child_process';
77
import { promisify } from 'util';
88
import path from 'path';
99
import fs from 'fs';
10+
import crypto from 'crypto';
1011
import { fileURLToPath } from 'url';
1112
import { getErrorMessage as getErrorMessageShared, createLogError } from '../common.js';
1213

@@ -160,3 +161,31 @@ export function isValidGitUrl(url: string): boolean {
160161

161162
return startsWithValidProtocol && !hasShellChars;
162163
}
164+
165+
/**
166+
* Execute a callback with a temporary git remote, ensuring cleanup.
167+
* Centralizes the pattern of adding a temp remote, doing work, and removing it.
168+
*/
169+
export async function withTempGitRemote<T>(
170+
installPath: string,
171+
sourceUrl: string,
172+
callback: (tempRemoteName: string) => Promise<T>
173+
): Promise<T> {
174+
const tempRemoteName = `automaker-temp-remote-${crypto.randomBytes(8).toString('hex')}`;
175+
try {
176+
await execAsync(`git remote add ${tempRemoteName} "${sourceUrl}"`, {
177+
cwd: installPath,
178+
env: execEnv,
179+
});
180+
return await callback(tempRemoteName);
181+
} finally {
182+
try {
183+
await execAsync(`git remote remove ${tempRemoteName}`, {
184+
cwd: installPath,
185+
env: execEnv,
186+
});
187+
} catch {
188+
// Ignore cleanup errors
189+
}
190+
}
191+
}

‎apps/server/src/routes/updates/routes/check.ts‎

Lines changed: 47 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import type { Request, Response } from 'express';
88
import type { SettingsService } from '../../../services/settings-service.js';
99
import type { UpdateCheckResult } from '@automaker/types';
10-
import crypto from 'crypto';
1110
import {
1211
execAsync,
1312
execEnv,
@@ -17,6 +16,7 @@ import {
1716
isGitRepo,
1817
isGitAvailable,
1918
isValidGitUrl,
19+
withTempGitRemote,
2020
getErrorMessage,
2121
logError,
2222
} from '../common.js';
@@ -62,65 +62,56 @@ export function createCheckHandler(settingsService: SettingsService) {
6262
const localVersion = await getCurrentCommit(installPath);
6363
const localVersionShort = await getShortCommit(installPath);
6464

65-
// Use a random remote name to avoid conflicts with concurrent checks
66-
const tempRemoteName = `automaker-update-check-${crypto.randomBytes(8).toString('hex')}`;
67-
6865
try {
69-
// Add temporary remote
70-
await execAsync(`git remote add ${tempRemoteName} "${sourceUrl}"`, {
71-
cwd: installPath,
72-
env: execEnv,
73-
});
74-
75-
// Fetch from the temporary remote
76-
await execAsync(`git fetch ${tempRemoteName} main`, {
77-
cwd: installPath,
78-
env: execEnv,
79-
});
80-
81-
// Get remote version
82-
const { stdout: remoteVersionOutput } = await execAsync(
83-
`git rev-parse ${tempRemoteName}/main`,
84-
{ cwd: installPath, env: execEnv }
85-
);
86-
const remoteVersion = remoteVersionOutput.trim();
87-
88-
// Get short remote version
89-
const { stdout: remoteVersionShortOutput } = await execAsync(
90-
`git rev-parse --short ${tempRemoteName}/main`,
91-
{ cwd: installPath, env: execEnv }
92-
);
93-
const remoteVersionShort = remoteVersionShortOutput.trim();
66+
const result = await withTempGitRemote(installPath, sourceUrl, async (tempRemoteName) => {
67+
// Fetch from the temporary remote
68+
await execAsync(`git fetch ${tempRemoteName} main`, {
69+
cwd: installPath,
70+
env: execEnv,
71+
});
9472

95-
// Check if remote is ahead of local (update available)
96-
// git merge-base --is-ancestor <commit1> <commit2> returns 0 if commit1 is ancestor of commit2
97-
let updateAvailable = false;
98-
if (localVersion !== remoteVersion) {
99-
try {
100-
// Check if local is already an ancestor of remote (remote is ahead)
101-
await execAsync(`git merge-base --is-ancestor ${localVersion} ${remoteVersion}`, {
102-
cwd: installPath,
103-
env: execEnv,
104-
});
105-
// If we get here (exit code 0), local is ancestor of remote, so update is available
106-
updateAvailable = true;
107-
} catch {
108-
// Exit code 1 means local is NOT an ancestor of remote
109-
// This means either local is ahead, or branches have diverged
110-
// In either case, we don't show "update available"
111-
updateAvailable = false;
73+
// Get remote version
74+
const { stdout: remoteVersionOutput } = await execAsync(
75+
`git rev-parse ${tempRemoteName}/main`,
76+
{ cwd: installPath, env: execEnv }
77+
);
78+
const remoteVersion = remoteVersionOutput.trim();
79+
80+
// Get short remote version
81+
const { stdout: remoteVersionShortOutput } = await execAsync(
82+
`git rev-parse --short ${tempRemoteName}/main`,
83+
{ cwd: installPath, env: execEnv }
84+
);
85+
const remoteVersionShort = remoteVersionShortOutput.trim();
86+
87+
// Check if remote is ahead of local (update available)
88+
let updateAvailable = false;
89+
if (localVersion !== remoteVersion) {
90+
try {
91+
// Check if local is already an ancestor of remote (remote is ahead)
92+
await execAsync(`git merge-base --is-ancestor ${localVersion} ${remoteVersion}`, {
93+
cwd: installPath,
94+
env: execEnv,
95+
});
96+
// If we get here (exit code 0), local is ancestor of remote, so update is available
97+
updateAvailable = true;
98+
} catch {
99+
// Exit code 1 means local is NOT an ancestor of remote
100+
// This means either local is ahead, or branches have diverged
101+
updateAvailable = false;
102+
}
112103
}
113-
}
114104

115-
const result: UpdateCheckResult = {
116-
updateAvailable,
117-
localVersion,
118-
localVersionShort,
119-
remoteVersion,
120-
remoteVersionShort,
121-
sourceUrl,
122-
installPath,
123-
};
105+
return {
106+
updateAvailable,
107+
localVersion,
108+
localVersionShort,
109+
remoteVersion,
110+
remoteVersionShort,
111+
sourceUrl,
112+
installPath,
113+
} satisfies UpdateCheckResult;
114+
});
124115

125116
res.json({
126117
success: true,
@@ -143,16 +134,6 @@ export function createCheckHandler(settingsService: SettingsService) {
143134
error: `Could not fetch from upstream: ${errorMsg}`,
144135
} satisfies UpdateCheckResult,
145136
});
146-
} finally {
147-
// Always clean up temp remote
148-
try {
149-
await execAsync(`git remote remove ${tempRemoteName}`, {
150-
cwd: installPath,
151-
env: execEnv,
152-
});
153-
} catch {
154-
// Ignore cleanup errors
155-
}
156137
}
157138
} catch (error) {
158139
logError(error, 'Update check failed');

‎apps/server/src/routes/updates/routes/pull.ts‎

Lines changed: 35 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import type { Request, Response } from 'express';
88
import type { SettingsService } from '../../../services/settings-service.js';
99
import type { UpdatePullResult } from '@automaker/types';
10-
import crypto from 'crypto';
1110
import {
1211
execAsync,
1312
execEnv,
@@ -18,6 +17,7 @@ import {
1817
isGitAvailable,
1918
isValidGitUrl,
2019
hasLocalChanges,
20+
withTempGitRemote,
2121
getErrorMessage,
2222
logError,
2323
} from '../common.js';
@@ -68,50 +68,43 @@ export function createPullHandler(settingsService: SettingsService) {
6868
return;
6969
}
7070

71-
// Get current version before pull
72-
const previousVersion = await getCurrentCommit(installPath);
73-
const previousVersionShort = await getShortCommit(installPath);
74-
75-
// Use a random remote name to avoid conflicts with concurrent pulls
76-
const tempRemoteName = `automaker-update-pull-${crypto.randomBytes(8).toString('hex')}`;
77-
7871
try {
79-
// Add temporary remote
80-
await execAsync(`git remote add ${tempRemoteName} "${sourceUrl}"`, {
81-
cwd: installPath,
82-
env: execEnv,
83-
});
84-
85-
// Fetch first
86-
await execAsync(`git fetch ${tempRemoteName} main`, {
87-
cwd: installPath,
88-
env: execEnv,
89-
});
90-
91-
// Merge the fetched changes
92-
const { stdout: mergeOutput } = await execAsync(
93-
`git merge ${tempRemoteName}/main --ff-only`,
94-
{ cwd: installPath, env: execEnv }
95-
);
72+
const result = await withTempGitRemote(installPath, sourceUrl, async (tempRemoteName) => {
73+
// Get current version before pull
74+
const previousVersion = await getCurrentCommit(installPath);
75+
const previousVersionShort = await getShortCommit(installPath);
9676

97-
// Get new version after merge
98-
const newVersion = await getCurrentCommit(installPath);
99-
const newVersionShort = await getShortCommit(installPath);
100-
101-
const alreadyUpToDate =
102-
mergeOutput.includes('Already up to date') || previousVersion === newVersion;
77+
// Fetch first
78+
await execAsync(`git fetch ${tempRemoteName} main`, {
79+
cwd: installPath,
80+
env: execEnv,
81+
});
10382

104-
const result: UpdatePullResult = {
105-
success: true,
106-
previousVersion,
107-
previousVersionShort,
108-
newVersion,
109-
newVersionShort,
110-
alreadyUpToDate,
111-
message: alreadyUpToDate
112-
? 'Already up to date'
113-
: `Updated from ${previousVersionShort} to ${newVersionShort}`,
114-
};
83+
// Merge the fetched changes
84+
const { stdout: mergeOutput } = await execAsync(
85+
`git merge ${tempRemoteName}/main --ff-only`,
86+
{ cwd: installPath, env: execEnv }
87+
);
88+
89+
// Get new version after merge
90+
const newVersion = await getCurrentCommit(installPath);
91+
const newVersionShort = await getShortCommit(installPath);
92+
93+
const alreadyUpToDate =
94+
mergeOutput.includes('Already up to date') || previousVersion === newVersion;
95+
96+
return {
97+
success: true,
98+
previousVersion,
99+
previousVersionShort,
100+
newVersion,
101+
newVersionShort,
102+
alreadyUpToDate,
103+
message: alreadyUpToDate
104+
? 'Already up to date'
105+
: `Updated from ${previousVersionShort} to ${newVersionShort}`,
106+
} satisfies UpdatePullResult;
107+
});
115108

116109
res.json({
117110
success: true,
@@ -143,16 +136,6 @@ export function createPullHandler(settingsService: SettingsService) {
143136
success: false,
144137
error: `Failed to pull updates: ${errorMsg}`,
145138
});
146-
} finally {
147-
// Always clean up temp remote
148-
try {
149-
await execAsync(`git remote remove ${tempRemoteName}`, {
150-
cwd: installPath,
151-
env: execEnv,
152-
});
153-
} catch {
154-
// Ignore cleanup errors
155-
}
156139
}
157140
} catch (error) {
158141
logError(error, 'Update pull failed');

0 commit comments

Comments
 (0)