Fix timeout implementation and address review feedback

- Kill git process on timeout: use child_process.spawn directly for
  timeout-eligible operations so we have a ChildProcess handle to send
  SIGTERM (then SIGKILL after 5s). On Windows, SIGTERM is a forced kill
  so the SIGKILL fallback is effectively a no-op there.

- Fix timeout:0 not working: replace falsy || coalescion with explicit
  empty-string check so that '0' is not replaced by the default '300'.

- Refactor execGit to use an options object instead of 5 positional
  parameters, eliminating error-prone filler args (false, false, {}).

- Pass allowAllExitCodes through to execGitWithTimeout so both code
  paths have consistent behavior for non-zero exit codes.

- Add settled guard to prevent double-reject when both close and error
  events fire on the spawned process.

- Handle null exit code (process killed by signal) as an error rather
  than silently treating it as success.

- Capture stderr in error messages for the timeout path, matching the
  information level of the non-timeout exec path.

- Log SIGKILL failures at debug level instead of empty catch block.

- Warn on customListeners being ignored in the timeout path.

- Emit core.warning() when invalid input values are silently replaced
  with defaults, so users know their configuration was rejected.

- Add input validation in setTimeout (reject negative values).

- Clarify retry-max-attempts semantics: total attempts including the
  initial attempt (3 = 1 initial + 2 retries).

- Remove Kubernetes probe references from descriptions.

- Use non-exhaustive list (e.g.) for network operations in docs to
  avoid staleness if new operations are added.

- Add tests for timeout/retry input parsing (defaults, timeout:0,
  custom values, invalid input with warnings, backoff clamping) and
  command manager configuration (setTimeout, setRetryConfig, fetch).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Anatoly Rabkin 2026-03-18 19:04:59 +02:00
parent 5df58a66d1
commit 33c1ac53d1
8 changed files with 495 additions and 117 deletions

View File

@ -155,20 +155,20 @@ Please refer to the [release page](https://github.com/actions/checkout/releases/
# Default: true # Default: true
set-safe-directory: '' set-safe-directory: ''
# Timeout in seconds for each git network operation attempt (fetch, lfs-fetch, # Timeout in seconds for each git network operation attempt (e.g. fetch,
# ls-remote). If a single attempt exceeds this, it is killed and retried. Set to 0 # lfs-fetch, ls-remote). If a single attempt exceeds this, the process is
# to disable. Default is 300 (5 minutes). Similar to Kubernetes probe # terminated. If retries are configured (see retry-max-attempts), the operation
# timeoutSeconds. # will be retried. Set to 0 to disable. Default is 300 (5 minutes).
# Default: 300 # Default: 300
timeout: '' timeout: ''
# Maximum number of retry attempts for failed git network operations. Similar to # Total number of attempts for each git network operation (including the initial
# Kubernetes probe failureThreshold. # attempt). For example, 3 means one initial attempt plus up to 2 retries.
# Default: 3 # Default: 3
retry-max-attempts: '' retry-max-attempts: ''
# Minimum backoff time in seconds between retry attempts. The actual backoff is # Minimum backoff time in seconds between retry attempts. The actual backoff is
# randomly chosen between min and max. Similar to Kubernetes probe periodSeconds. # randomly chosen between min and max.
# Default: 10 # Default: 10
retry-min-backoff: '' retry-min-backoff: ''

View File

@ -5,6 +5,17 @@ import * as commandManager from '../lib/git-command-manager'
let git: commandManager.IGitCommandManager let git: commandManager.IGitCommandManager
let mockExec = jest.fn() let mockExec = jest.fn()
function createMockGit(): Promise<commandManager.IGitCommandManager> {
mockExec.mockImplementation((path, args, options) => {
if (args.includes('version')) {
options.listeners.stdout(Buffer.from('2.18'))
}
return 0
})
jest.spyOn(exec, 'exec').mockImplementation(mockExec)
return commandManager.createCommandManager('test', false, false)
}
describe('git-auth-helper tests', () => { describe('git-auth-helper tests', () => {
beforeAll(async () => {}) beforeAll(async () => {})
@ -494,3 +505,41 @@ describe('git user-agent with orchestration ID', () => {
) )
}) })
}) })
describe('timeout and retry configuration', () => {
beforeEach(async () => {
jest.spyOn(fshelper, 'fileExistsSync').mockImplementation(jest.fn())
jest.spyOn(fshelper, 'directoryExistsSync').mockImplementation(jest.fn())
})
afterEach(() => {
jest.restoreAllMocks()
})
it('setTimeout converts seconds to milliseconds', async () => {
git = await createMockGit()
// Should not throw
git.setTimeout(30)
git.setTimeout(0)
})
it('setRetryConfig accepts valid parameters', async () => {
git = await createMockGit()
// Should not throw
git.setRetryConfig(5, 2, 15)
})
it('fetch without timeout uses exec', async () => {
git = await createMockGit()
// timeout defaults to 0 (disabled)
mockExec.mockClear()
await git.fetch(['refs/heads/main'], {})
// exec.exec is used (via retryHelper) when no timeout
const fetchCalls = mockExec.mock.calls.filter(
(call: any[]) => (call[1] as string[]).includes('fetch')
)
expect(fetchCalls.length).toBeGreaterThan(0)
})
})

View File

@ -144,4 +144,55 @@ describe('input-helper tests', () => {
const settings: IGitSourceSettings = await inputHelper.getInputs() const settings: IGitSourceSettings = await inputHelper.getInputs()
expect(settings.workflowOrganizationId).toBe(123456) expect(settings.workflowOrganizationId).toBe(123456)
}) })
it('sets timeout and retry defaults', async () => {
const settings: IGitSourceSettings = await inputHelper.getInputs()
expect(settings.timeout).toBe(300)
expect(settings.retryMaxAttempts).toBe(3)
expect(settings.retryMinBackoff).toBe(10)
expect(settings.retryMaxBackoff).toBe(20)
})
it('allows timeout 0 to disable', async () => {
inputs.timeout = '0'
const settings: IGitSourceSettings = await inputHelper.getInputs()
expect(settings.timeout).toBe(0)
})
it('parses custom timeout and retry values', async () => {
inputs.timeout = '30'
inputs['retry-max-attempts'] = '5'
inputs['retry-min-backoff'] = '2'
inputs['retry-max-backoff'] = '15'
const settings: IGitSourceSettings = await inputHelper.getInputs()
expect(settings.timeout).toBe(30)
expect(settings.retryMaxAttempts).toBe(5)
expect(settings.retryMinBackoff).toBe(2)
expect(settings.retryMaxBackoff).toBe(15)
})
it('clamps retry-max-backoff to min when less than min', async () => {
inputs['retry-min-backoff'] = '20'
inputs['retry-max-backoff'] = '5'
const settings: IGitSourceSettings = await inputHelper.getInputs()
expect(settings.retryMaxBackoff).toBe(20)
})
it('defaults invalid timeout to 300 and warns', async () => {
inputs.timeout = 'garbage'
const settings: IGitSourceSettings = await inputHelper.getInputs()
expect(settings.timeout).toBe(300)
expect(core.warning).toHaveBeenCalledWith(
expect.stringContaining("Invalid value 'garbage' for 'timeout'")
)
})
it('defaults negative retry-max-attempts to 3 and warns', async () => {
inputs['retry-max-attempts'] = '-1'
const settings: IGitSourceSettings = await inputHelper.getInputs()
expect(settings.retryMaxAttempts).toBe(3)
expect(core.warning).toHaveBeenCalledWith(
expect.stringContaining("Invalid value '-1' for 'retry-max-attempts'")
)
})
}) })

View File

@ -97,21 +97,20 @@ inputs:
default: true default: true
timeout: timeout:
description: > description: >
Timeout in seconds for each git network operation attempt (fetch, lfs-fetch, ls-remote). Timeout in seconds for each git network operation attempt (e.g. fetch, lfs-fetch, ls-remote).
If a single attempt exceeds this, it is killed and retried. If a single attempt exceeds this, the process is terminated.
If retries are configured (see retry-max-attempts), the operation will be retried.
Set to 0 to disable. Default is 300 (5 minutes). Set to 0 to disable. Default is 300 (5 minutes).
Similar to Kubernetes probe timeoutSeconds.
default: 300 default: 300
retry-max-attempts: retry-max-attempts:
description: > description: >
Maximum number of retry attempts for failed git network operations. Total number of attempts for each git network operation (including the initial attempt).
Similar to Kubernetes probe failureThreshold. For example, 3 means one initial attempt plus up to 2 retries.
default: 3 default: 3
retry-min-backoff: retry-min-backoff:
description: > description: >
Minimum backoff time in seconds between retry attempts. Minimum backoff time in seconds between retry attempts.
The actual backoff is randomly chosen between min and max. The actual backoff is randomly chosen between min and max.
Similar to Kubernetes probe periodSeconds.
default: 10 default: 10
retry-max-backoff: retry-max-backoff:
description: > description: >

206
dist/index.js vendored
View File

@ -655,6 +655,7 @@ const io = __importStar(__nccwpck_require__(7436));
const path = __importStar(__nccwpck_require__(1017)); const path = __importStar(__nccwpck_require__(1017));
const regexpHelper = __importStar(__nccwpck_require__(3120)); const regexpHelper = __importStar(__nccwpck_require__(3120));
const retryHelper = __importStar(__nccwpck_require__(2155)); const retryHelper = __importStar(__nccwpck_require__(2155));
const child_process_1 = __nccwpck_require__(2081);
const git_version_1 = __nccwpck_require__(3142); const git_version_1 = __nccwpck_require__(3142);
// Auth header not supported before 2.9 // Auth header not supported before 2.9
// Wire protocol v2 not supported before 2.18 // Wire protocol v2 not supported before 2.18
@ -737,7 +738,7 @@ class GitCommandManager {
} }
}; };
// Suppress the output in order to avoid flooding annotations with innocuous errors. // Suppress the output in order to avoid flooding annotations with innocuous errors.
yield this.execGit(args, false, true, listeners); yield this.execGit(args, { silent: true, customListeners: listeners });
core.debug(`stderr callback is: ${stderr}`); core.debug(`stderr callback is: ${stderr}`);
core.debug(`errline callback is: ${errline}`); core.debug(`errline callback is: ${errline}`);
core.debug(`stdout callback is: ${stdout}`); core.debug(`stdout callback is: ${stdout}`);
@ -825,7 +826,7 @@ class GitCommandManager {
'--name-only', '--name-only',
'--get-regexp', '--get-regexp',
pattern pattern
], true); ], { allowAllExitCodes: true });
return output.exitCode === 0; return output.exitCode === 0;
}); });
} }
@ -854,7 +855,7 @@ class GitCommandManager {
} }
const that = this; const that = this;
yield this.networkRetryHelper.execute(() => __awaiter(this, void 0, void 0, function* () { yield this.networkRetryHelper.execute(() => __awaiter(this, void 0, void 0, function* () {
yield that.execGit(args, false, false, {}, that.timeoutMs); yield that.execGit(args, { timeoutMs: that.timeoutMs });
})); }));
}); });
} }
@ -869,7 +870,7 @@ class GitCommandManager {
'--symref', '--symref',
repositoryUrl, repositoryUrl,
'HEAD' 'HEAD'
], false, false, {}, this.timeoutMs); ], { timeoutMs: this.timeoutMs });
})); }));
if (output) { if (output) {
// Satisfy compiler, will always be set // Satisfy compiler, will always be set
@ -906,7 +907,7 @@ class GitCommandManager {
isDetached() { isDetached() {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
// Note, "branch --show-current" would be simpler but isn't available until Git 2.22 // Note, "branch --show-current" would be simpler but isn't available until Git 2.22
const output = yield this.execGit(['rev-parse', '--symbolic-full-name', '--verify', '--quiet', 'HEAD'], true); const output = yield this.execGit(['rev-parse', '--symbolic-full-name', '--verify', '--quiet', 'HEAD'], { allowAllExitCodes: true });
return !output.stdout.trim().startsWith('refs/heads/'); return !output.stdout.trim().startsWith('refs/heads/');
}); });
} }
@ -915,7 +916,7 @@ class GitCommandManager {
const args = ['lfs', 'fetch', 'origin', ref]; const args = ['lfs', 'fetch', 'origin', ref];
const that = this; const that = this;
yield this.networkRetryHelper.execute(() => __awaiter(this, void 0, void 0, function* () { yield this.networkRetryHelper.execute(() => __awaiter(this, void 0, void 0, function* () {
yield that.execGit(args, false, false, {}, that.timeoutMs); yield that.execGit(args, { timeoutMs: that.timeoutMs });
})); }));
}); });
} }
@ -927,8 +928,8 @@ class GitCommandManager {
log1(format) { log1(format) {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
const args = format ? ['log', '-1', format] : ['log', '-1']; const args = format ? ['log', '-1', format] : ['log', '-1'];
const silent = format ? false : true; const silent = !format;
const output = yield this.execGit(args, false, silent); const output = yield this.execGit(args, { silent });
return output.stdout; return output.stdout;
}); });
} }
@ -958,7 +959,7 @@ class GitCommandManager {
shaExists(sha) { shaExists(sha) {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
const args = ['rev-parse', '--verify', '--quiet', `${sha}^{object}`]; const args = ['rev-parse', '--verify', '--quiet', `${sha}^{object}`];
const output = yield this.execGit(args, true); const output = yield this.execGit(args, { allowAllExitCodes: true });
return output.exitCode === 0; return output.exitCode === 0;
}); });
} }
@ -997,7 +998,7 @@ class GitCommandManager {
} }
submoduleStatus() { submoduleStatus() {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
const output = yield this.execGit(['submodule', 'status'], true); const output = yield this.execGit(['submodule', 'status'], { allowAllExitCodes: true });
core.debug(output.stdout); core.debug(output.stdout);
return output.exitCode === 0; return output.exitCode === 0;
}); });
@ -1010,7 +1011,7 @@ class GitCommandManager {
} }
tryClean() { tryClean() {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
const output = yield this.execGit(['clean', '-ffdx'], true); const output = yield this.execGit(['clean', '-ffdx'], { allowAllExitCodes: true });
return output.exitCode === 0; return output.exitCode === 0;
}); });
} }
@ -1021,7 +1022,7 @@ class GitCommandManager {
globalConfig ? '--global' : '--local', globalConfig ? '--global' : '--local',
'--unset-all', '--unset-all',
configKey configKey
], true); ], { allowAllExitCodes: true });
return output.exitCode === 0; return output.exitCode === 0;
}); });
} }
@ -1035,19 +1036,19 @@ class GitCommandManager {
args.push(globalConfig ? '--global' : '--local'); args.push(globalConfig ? '--global' : '--local');
} }
args.push('--unset', configKey, configValue); args.push('--unset', configKey, configValue);
const output = yield this.execGit(args, true); const output = yield this.execGit(args, { allowAllExitCodes: true });
return output.exitCode === 0; return output.exitCode === 0;
}); });
} }
tryDisableAutomaticGarbageCollection() { tryDisableAutomaticGarbageCollection() {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
const output = yield this.execGit(['config', '--local', 'gc.auto', '0'], true); const output = yield this.execGit(['config', '--local', 'gc.auto', '0'], { allowAllExitCodes: true });
return output.exitCode === 0; return output.exitCode === 0;
}); });
} }
tryGetFetchUrl() { tryGetFetchUrl() {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
const output = yield this.execGit(['config', '--local', '--get', 'remote.origin.url'], true); const output = yield this.execGit(['config', '--local', '--get', 'remote.origin.url'], { allowAllExitCodes: true });
if (output.exitCode !== 0) { if (output.exitCode !== 0) {
return ''; return '';
} }
@ -1068,7 +1069,7 @@ class GitCommandManager {
args.push(globalConfig ? '--global' : '--local'); args.push(globalConfig ? '--global' : '--local');
} }
args.push('--get-all', configKey); args.push('--get-all', configKey);
const output = yield this.execGit(args, true); const output = yield this.execGit(args, { allowAllExitCodes: true });
if (output.exitCode !== 0) { if (output.exitCode !== 0) {
return []; return [];
} }
@ -1088,7 +1089,7 @@ class GitCommandManager {
args.push(globalConfig ? '--global' : '--local'); args.push(globalConfig ? '--global' : '--local');
} }
args.push('--name-only', '--get-regexp', pattern); args.push('--name-only', '--get-regexp', pattern);
const output = yield this.execGit(args, true); const output = yield this.execGit(args, { allowAllExitCodes: true });
if (output.exitCode !== 0) { if (output.exitCode !== 0) {
return []; return [];
} }
@ -1100,7 +1101,7 @@ class GitCommandManager {
} }
tryReset() { tryReset() {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
const output = yield this.execGit(['reset', '--hard', 'HEAD'], true); const output = yield this.execGit(['reset', '--hard', 'HEAD'], { allowAllExitCodes: true });
return output.exitCode === 0; return output.exitCode === 0;
}); });
} }
@ -1109,9 +1110,22 @@ class GitCommandManager {
return this.gitVersion; return this.gitVersion;
}); });
} }
/**
* Sets the timeout for network git operations.
* @param timeoutSeconds Timeout in seconds. 0 disables the timeout.
*/
setTimeout(timeoutSeconds) { setTimeout(timeoutSeconds) {
if (timeoutSeconds < 0) {
throw new Error(`Timeout must be non-negative, got ${timeoutSeconds}`);
}
this.timeoutMs = timeoutSeconds * 1000; this.timeoutMs = timeoutSeconds * 1000;
} }
/**
* Configures retry behavior for network git operations.
* @param maxAttempts Total attempts including the initial one. Must be >= 1.
* @param minBackoffSeconds Minimum backoff between retries. Must be <= maxBackoffSeconds.
* @param maxBackoffSeconds Maximum backoff between retries.
*/
setRetryConfig(maxAttempts, minBackoffSeconds, maxBackoffSeconds) { setRetryConfig(maxAttempts, minBackoffSeconds, maxBackoffSeconds) {
this.networkRetryHelper = new retryHelper.RetryHelper(maxAttempts, minBackoffSeconds, maxBackoffSeconds); this.networkRetryHelper = new retryHelper.RetryHelper(maxAttempts, minBackoffSeconds, maxBackoffSeconds);
} }
@ -1123,8 +1137,19 @@ class GitCommandManager {
}); });
} }
execGit(args_1) { execGit(args_1) {
return __awaiter(this, arguments, void 0, function* (args, allowAllExitCodes = false, silent = false, customListeners = {}, timeoutMs = 0) { return __awaiter(this, arguments, void 0, function* (args, options = {}) {
const { allowAllExitCodes = false, silent = false, customListeners = {}, timeoutMs = 0 } = options;
fshelper.directoryExistsSync(this.workingDirectory, true); fshelper.directoryExistsSync(this.workingDirectory, true);
// Use child_process.spawn directly when timeout is set,
// so we can kill the process on timeout and avoid orphaned git processes.
// Note: customListeners are not supported in the timeout path.
if (timeoutMs > 0) {
if (customListeners &&
Object.keys(customListeners).length > 0) {
core.debug('customListeners are not supported with timeoutMs and will be ignored');
}
return yield this.execGitWithTimeout(args, timeoutMs, silent, allowAllExitCodes);
}
const result = new GitOutput(); const result = new GitOutput();
const env = {}; const env = {};
for (const key of Object.keys(process.env)) { for (const key of Object.keys(process.env)) {
@ -1140,37 +1165,120 @@ class GitCommandManager {
}; };
const mergedListeners = Object.assign(Object.assign({}, defaultListener), customListeners); const mergedListeners = Object.assign(Object.assign({}, defaultListener), customListeners);
const stdout = []; const stdout = [];
const options = { const execOptions = {
cwd: this.workingDirectory, cwd: this.workingDirectory,
env, env,
silent, silent,
ignoreReturnCode: allowAllExitCodes, ignoreReturnCode: allowAllExitCodes,
listeners: mergedListeners listeners: mergedListeners
}; };
const execPromise = exec.exec(`"${this.gitPath}"`, args, options); result.exitCode = yield exec.exec(`"${this.gitPath}"`, args, execOptions);
if (timeoutMs > 0) {
let timer;
const timeoutPromise = new Promise((_, reject) => {
timer = global.setTimeout(() => {
reject(new Error(`Git operation timed out after ${timeoutMs / 1000} seconds: git ${args.slice(0, 3).join(' ')}...`));
}, timeoutMs);
});
try {
result.exitCode = yield Promise.race([execPromise, timeoutPromise]);
}
finally {
clearTimeout(timer);
}
}
else {
result.exitCode = yield execPromise;
}
result.stdout = stdout.join(''); result.stdout = stdout.join('');
core.debug(result.exitCode.toString()); core.debug(result.exitCode.toString());
core.debug(result.stdout); core.debug(result.stdout);
return result; return result;
}); });
} }
/**
* Executes a git command with a timeout. Uses child_process.spawn directly
* (instead of @actions/exec) so we can kill the process on timeout and
* terminate it cleanly. Does not support customListeners.
*/
execGitWithTimeout(args, timeoutMs, silent, allowAllExitCodes) {
return __awaiter(this, void 0, void 0, function* () {
const result = new GitOutput();
const env = {};
for (const key of Object.keys(process.env)) {
env[key] = process.env[key];
}
for (const key of Object.keys(this.gitEnv)) {
env[key] = this.gitEnv[key];
}
const stdout = [];
const stderr = [];
return new Promise((resolve, reject) => {
var _a;
const child = (0, child_process_1.spawn)(this.gitPath, args, {
cwd: this.workingDirectory,
env,
stdio: ['ignore', 'pipe', 'pipe']
});
(_a = child.stdout) === null || _a === void 0 ? void 0 : _a.on('data', (data) => {
stdout.push(data.toString());
});
if (child.stderr) {
child.stderr.on('data', (data) => {
stderr.push(data.toString());
if (!silent) {
process.stderr.write(data);
}
});
}
let settled = false;
let timedOut = false;
let forceKillTimer;
const cleanup = () => {
clearTimeout(timer);
if (forceKillTimer) {
clearTimeout(forceKillTimer);
}
};
const timer = global.setTimeout(() => {
timedOut = true;
// SIGTERM first, then force SIGKILL after 5 seconds.
// On Windows, SIGTERM is equivalent to a forced kill, so
// the SIGKILL fallback is effectively a no-op there.
child.kill('SIGTERM');
forceKillTimer = global.setTimeout(() => {
try {
child.kill('SIGKILL');
}
catch (killErr) {
core.debug(`Failed to SIGKILL git process: ${killErr}`);
}
}, 5000);
if (forceKillTimer.unref) {
forceKillTimer.unref();
}
}, timeoutMs);
child.on('close', (code) => {
if (settled)
return;
settled = true;
cleanup();
if (timedOut) {
reject(new Error(`Git operation timed out after ${timeoutMs / 1000} seconds: git ${args.slice(0, 3).join(' ')}...`));
return;
}
// null code means killed by signal (e.g. OOM killer, external SIGTERM)
if (code === null) {
const stderrText = stderr.join('').trim();
reject(new Error(`The process 'git' was killed by a signal` +
(stderrText ? `\n${stderrText}` : '')));
return;
}
if (code !== 0 && !allowAllExitCodes) {
const stderrText = stderr.join('').trim();
reject(new Error(`The process 'git' failed with exit code ${code}` +
(stderrText ? `\n${stderrText}` : '')));
return;
}
result.exitCode = code;
result.stdout = stdout.join('');
core.debug(result.exitCode.toString());
core.debug(result.stdout);
resolve(result);
});
child.on('error', (err) => {
if (settled)
return;
settled = true;
cleanup();
reject(err);
});
});
});
}
initializeCommandManager(workingDirectory, lfs, doSparseCheckout) { initializeCommandManager(workingDirectory, lfs, doSparseCheckout) {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
this.workingDirectory = workingDirectory; this.workingDirectory = workingDirectory;
@ -2124,26 +2232,34 @@ function getInputs() {
// Determine the GitHub URL that the repository is being hosted from // Determine the GitHub URL that the repository is being hosted from
result.githubServerUrl = core.getInput('github-server-url'); result.githubServerUrl = core.getInput('github-server-url');
core.debug(`GitHub Host URL = ${result.githubServerUrl}`); core.debug(`GitHub Host URL = ${result.githubServerUrl}`);
// Timeout (per-attempt, like k8s timeoutSeconds) // Timeout per network operation attempt
result.timeout = Math.floor(Number(core.getInput('timeout') || '300')); const timeoutInput = core.getInput('timeout');
result.timeout = Math.floor(Number(timeoutInput !== '' ? timeoutInput : '300'));
if (isNaN(result.timeout) || result.timeout < 0) { if (isNaN(result.timeout) || result.timeout < 0) {
core.warning(`Invalid value '${timeoutInput}' for 'timeout' input. Using default: 300 seconds.`);
result.timeout = 300; result.timeout = 300;
} }
core.debug(`timeout = ${result.timeout}`); core.debug(`timeout = ${result.timeout}`);
// Retry max attempts (like k8s failureThreshold) // Retry max attempts (total attempts including initial)
result.retryMaxAttempts = Math.floor(Number(core.getInput('retry-max-attempts') || '3')); const retryMaxAttemptsInput = core.getInput('retry-max-attempts');
result.retryMaxAttempts = Math.floor(Number(retryMaxAttemptsInput !== '' ? retryMaxAttemptsInput : '3'));
if (isNaN(result.retryMaxAttempts) || result.retryMaxAttempts < 1) { if (isNaN(result.retryMaxAttempts) || result.retryMaxAttempts < 1) {
core.warning(`Invalid value '${retryMaxAttemptsInput}' for 'retry-max-attempts' input. Using default: 3.`);
result.retryMaxAttempts = 3; result.retryMaxAttempts = 3;
} }
core.debug(`retry max attempts = ${result.retryMaxAttempts}`); core.debug(`retry max attempts = ${result.retryMaxAttempts}`);
// Retry backoff (like k8s periodSeconds, but as a min/max range) // Retry backoff range
result.retryMinBackoff = Math.floor(Number(core.getInput('retry-min-backoff') || '10')); const retryMinBackoffInput = core.getInput('retry-min-backoff');
result.retryMinBackoff = Math.floor(Number(retryMinBackoffInput !== '' ? retryMinBackoffInput : '10'));
if (isNaN(result.retryMinBackoff) || result.retryMinBackoff < 0) { if (isNaN(result.retryMinBackoff) || result.retryMinBackoff < 0) {
core.warning(`Invalid value '${retryMinBackoffInput}' for 'retry-min-backoff' input. Using default: 10 seconds.`);
result.retryMinBackoff = 10; result.retryMinBackoff = 10;
} }
core.debug(`retry min backoff = ${result.retryMinBackoff}`); core.debug(`retry min backoff = ${result.retryMinBackoff}`);
result.retryMaxBackoff = Math.floor(Number(core.getInput('retry-max-backoff') || '20')); const retryMaxBackoffInput = core.getInput('retry-max-backoff');
result.retryMaxBackoff = Math.floor(Number(retryMaxBackoffInput !== '' ? retryMaxBackoffInput : '20'));
if (isNaN(result.retryMaxBackoff) || result.retryMaxBackoff < 0) { if (isNaN(result.retryMaxBackoff) || result.retryMaxBackoff < 0) {
core.warning(`Invalid value '${retryMaxBackoffInput}' for 'retry-max-backoff' input. Using default: 20 seconds.`);
result.retryMaxBackoff = 20; result.retryMaxBackoff = 20;
} }
if (result.retryMaxBackoff < result.retryMinBackoff) { if (result.retryMaxBackoff < result.retryMinBackoff) {

View File

@ -7,6 +7,7 @@ import * as path from 'path'
import * as refHelper from './ref-helper' import * as refHelper from './ref-helper'
import * as regexpHelper from './regexp-helper' import * as regexpHelper from './regexp-helper'
import * as retryHelper from './retry-helper' import * as retryHelper from './retry-helper'
import {spawn} from 'child_process'
import {GitVersion} from './git-version' import {GitVersion} from './git-version'
// Auth header not supported before 2.9 // Auth header not supported before 2.9
@ -176,7 +177,7 @@ class GitCommandManager {
} }
// Suppress the output in order to avoid flooding annotations with innocuous errors. // Suppress the output in order to avoid flooding annotations with innocuous errors.
await this.execGit(args, false, true, listeners) await this.execGit(args, {silent: true, customListeners: listeners})
core.debug(`stderr callback is: ${stderr}`) core.debug(`stderr callback is: ${stderr}`)
core.debug(`errline callback is: ${errline}`) core.debug(`errline callback is: ${errline}`)
@ -277,7 +278,7 @@ class GitCommandManager {
'--get-regexp', '--get-regexp',
pattern pattern
], ],
true {allowAllExitCodes: true}
) )
return output.exitCode === 0 return output.exitCode === 0
} }
@ -321,7 +322,7 @@ class GitCommandManager {
const that = this const that = this
await this.networkRetryHelper.execute(async () => { await this.networkRetryHelper.execute(async () => {
await that.execGit(args, false, false, {}, that.timeoutMs) await that.execGit(args, {timeoutMs: that.timeoutMs})
}) })
} }
@ -337,10 +338,7 @@ class GitCommandManager {
repositoryUrl, repositoryUrl,
'HEAD' 'HEAD'
], ],
false, {timeoutMs: this.timeoutMs}
false,
{},
this.timeoutMs
) )
}) })
@ -386,7 +384,7 @@ class GitCommandManager {
// Note, "branch --show-current" would be simpler but isn't available until Git 2.22 // Note, "branch --show-current" would be simpler but isn't available until Git 2.22
const output = await this.execGit( const output = await this.execGit(
['rev-parse', '--symbolic-full-name', '--verify', '--quiet', 'HEAD'], ['rev-parse', '--symbolic-full-name', '--verify', '--quiet', 'HEAD'],
true {allowAllExitCodes: true}
) )
return !output.stdout.trim().startsWith('refs/heads/') return !output.stdout.trim().startsWith('refs/heads/')
} }
@ -396,7 +394,7 @@ class GitCommandManager {
const that = this const that = this
await this.networkRetryHelper.execute(async () => { await this.networkRetryHelper.execute(async () => {
await that.execGit(args, false, false, {}, that.timeoutMs) await that.execGit(args, {timeoutMs: that.timeoutMs})
}) })
} }
@ -406,8 +404,8 @@ class GitCommandManager {
async log1(format?: string): Promise<string> { async log1(format?: string): Promise<string> {
const args = format ? ['log', '-1', format] : ['log', '-1'] const args = format ? ['log', '-1', format] : ['log', '-1']
const silent = format ? false : true const silent = !format
const output = await this.execGit(args, false, silent) const output = await this.execGit(args, {silent})
return output.stdout return output.stdout
} }
@ -436,7 +434,7 @@ class GitCommandManager {
async shaExists(sha: string): Promise<boolean> { async shaExists(sha: string): Promise<boolean> {
const args = ['rev-parse', '--verify', '--quiet', `${sha}^{object}`] const args = ['rev-parse', '--verify', '--quiet', `${sha}^{object}`]
const output = await this.execGit(args, true) const output = await this.execGit(args, {allowAllExitCodes: true})
return output.exitCode === 0 return output.exitCode === 0
} }
@ -475,7 +473,7 @@ class GitCommandManager {
} }
async submoduleStatus(): Promise<boolean> { async submoduleStatus(): Promise<boolean> {
const output = await this.execGit(['submodule', 'status'], true) const output = await this.execGit(['submodule', 'status'], {allowAllExitCodes: true})
core.debug(output.stdout) core.debug(output.stdout)
return output.exitCode === 0 return output.exitCode === 0
} }
@ -486,7 +484,7 @@ class GitCommandManager {
} }
async tryClean(): Promise<boolean> { async tryClean(): Promise<boolean> {
const output = await this.execGit(['clean', '-ffdx'], true) const output = await this.execGit(['clean', '-ffdx'], {allowAllExitCodes: true})
return output.exitCode === 0 return output.exitCode === 0
} }
@ -501,7 +499,7 @@ class GitCommandManager {
'--unset-all', '--unset-all',
configKey configKey
], ],
true {allowAllExitCodes: true}
) )
return output.exitCode === 0 return output.exitCode === 0
} }
@ -520,14 +518,14 @@ class GitCommandManager {
} }
args.push('--unset', configKey, configValue) args.push('--unset', configKey, configValue)
const output = await this.execGit(args, true) const output = await this.execGit(args, {allowAllExitCodes: true})
return output.exitCode === 0 return output.exitCode === 0
} }
async tryDisableAutomaticGarbageCollection(): Promise<boolean> { async tryDisableAutomaticGarbageCollection(): Promise<boolean> {
const output = await this.execGit( const output = await this.execGit(
['config', '--local', 'gc.auto', '0'], ['config', '--local', 'gc.auto', '0'],
true {allowAllExitCodes: true}
) )
return output.exitCode === 0 return output.exitCode === 0
} }
@ -535,7 +533,7 @@ class GitCommandManager {
async tryGetFetchUrl(): Promise<string> { async tryGetFetchUrl(): Promise<string> {
const output = await this.execGit( const output = await this.execGit(
['config', '--local', '--get', 'remote.origin.url'], ['config', '--local', '--get', 'remote.origin.url'],
true {allowAllExitCodes: true}
) )
if (output.exitCode !== 0) { if (output.exitCode !== 0) {
@ -563,7 +561,7 @@ class GitCommandManager {
} }
args.push('--get-all', configKey) args.push('--get-all', configKey)
const output = await this.execGit(args, true) const output = await this.execGit(args, {allowAllExitCodes: true})
if (output.exitCode !== 0) { if (output.exitCode !== 0) {
return [] return []
@ -588,7 +586,7 @@ class GitCommandManager {
} }
args.push('--name-only', '--get-regexp', pattern) args.push('--name-only', '--get-regexp', pattern)
const output = await this.execGit(args, true) const output = await this.execGit(args, {allowAllExitCodes: true})
if (output.exitCode !== 0) { if (output.exitCode !== 0) {
return [] return []
@ -601,7 +599,7 @@ class GitCommandManager {
} }
async tryReset(): Promise<boolean> { async tryReset(): Promise<boolean> {
const output = await this.execGit(['reset', '--hard', 'HEAD'], true) const output = await this.execGit(['reset', '--hard', 'HEAD'], {allowAllExitCodes: true})
return output.exitCode === 0 return output.exitCode === 0
} }
@ -609,10 +607,23 @@ class GitCommandManager {
return this.gitVersion return this.gitVersion
} }
/**
* Sets the timeout for network git operations.
* @param timeoutSeconds Timeout in seconds. 0 disables the timeout.
*/
setTimeout(timeoutSeconds: number): void { setTimeout(timeoutSeconds: number): void {
if (timeoutSeconds < 0) {
throw new Error(`Timeout must be non-negative, got ${timeoutSeconds}`)
}
this.timeoutMs = timeoutSeconds * 1000 this.timeoutMs = timeoutSeconds * 1000
} }
/**
* Configures retry behavior for network git operations.
* @param maxAttempts Total attempts including the initial one. Must be >= 1.
* @param minBackoffSeconds Minimum backoff between retries. Must be <= maxBackoffSeconds.
* @param maxBackoffSeconds Maximum backoff between retries.
*/
setRetryConfig( setRetryConfig(
maxAttempts: number, maxAttempts: number,
minBackoffSeconds: number, minBackoffSeconds: number,
@ -641,13 +652,42 @@ class GitCommandManager {
private async execGit( private async execGit(
args: string[], args: string[],
allowAllExitCodes = false, options: {
silent = false, allowAllExitCodes?: boolean
customListeners = {}, silent?: boolean
timeoutMs = 0 customListeners?: {}
timeoutMs?: number
} = {}
): Promise<GitOutput> { ): Promise<GitOutput> {
const {
allowAllExitCodes = false,
silent = false,
customListeners = {},
timeoutMs = 0
} = options
fshelper.directoryExistsSync(this.workingDirectory, true) fshelper.directoryExistsSync(this.workingDirectory, true)
// Use child_process.spawn directly when timeout is set,
// so we can kill the process on timeout and avoid orphaned git processes.
// Note: customListeners are not supported in the timeout path.
if (timeoutMs > 0) {
if (
customListeners &&
Object.keys(customListeners).length > 0
) {
core.debug(
'customListeners are not supported with timeoutMs and will be ignored'
)
}
return await this.execGitWithTimeout(
args,
timeoutMs,
silent,
allowAllExitCodes
)
}
const result = new GitOutput() const result = new GitOutput()
const env = {} const env = {}
@ -667,7 +707,7 @@ class GitCommandManager {
const mergedListeners = {...defaultListener, ...customListeners} const mergedListeners = {...defaultListener, ...customListeners}
const stdout: string[] = [] const stdout: string[] = []
const options = { const execOptions = {
cwd: this.workingDirectory, cwd: this.workingDirectory,
env, env,
silent, silent,
@ -675,27 +715,7 @@ class GitCommandManager {
listeners: mergedListeners listeners: mergedListeners
} }
const execPromise = exec.exec(`"${this.gitPath}"`, args, options) result.exitCode = await exec.exec(`"${this.gitPath}"`, args, execOptions)
if (timeoutMs > 0) {
let timer: ReturnType<typeof setTimeout>
const timeoutPromise = new Promise<never>((_, reject) => {
timer = global.setTimeout(() => {
reject(
new Error(
`Git operation timed out after ${timeoutMs / 1000} seconds: git ${args.slice(0, 3).join(' ')}...`
)
)
}, timeoutMs)
})
try {
result.exitCode = await Promise.race([execPromise, timeoutPromise])
} finally {
clearTimeout(timer!)
}
} else {
result.exitCode = await execPromise
}
result.stdout = stdout.join('') result.stdout = stdout.join('')
@ -705,6 +725,134 @@ class GitCommandManager {
return result return result
} }
/**
* Executes a git command with a timeout. Uses child_process.spawn directly
* (instead of @actions/exec) so we can kill the process on timeout and
* terminate it cleanly. Does not support customListeners.
*/
private async execGitWithTimeout(
args: string[],
timeoutMs: number,
silent: boolean,
allowAllExitCodes: boolean
): Promise<GitOutput> {
const result = new GitOutput()
const env: {[key: string]: string} = {}
for (const key of Object.keys(process.env)) {
env[key] = process.env[key] as string
}
for (const key of Object.keys(this.gitEnv)) {
env[key] = this.gitEnv[key]
}
const stdout: string[] = []
const stderr: string[] = []
return new Promise<GitOutput>((resolve, reject) => {
const child = spawn(this.gitPath, args, {
cwd: this.workingDirectory,
env,
stdio: ['ignore', 'pipe', 'pipe']
})
child.stdout?.on('data', (data: Buffer) => {
stdout.push(data.toString())
})
if (child.stderr) {
child.stderr.on('data', (data: Buffer) => {
stderr.push(data.toString())
if (!silent) {
process.stderr.write(data)
}
})
}
let settled = false
let timedOut = false
let forceKillTimer: ReturnType<typeof setTimeout> | undefined
const cleanup = (): void => {
clearTimeout(timer)
if (forceKillTimer) {
clearTimeout(forceKillTimer)
}
}
const timer = global.setTimeout(() => {
timedOut = true
// SIGTERM first, then force SIGKILL after 5 seconds.
// On Windows, SIGTERM is equivalent to a forced kill, so
// the SIGKILL fallback is effectively a no-op there.
child.kill('SIGTERM')
forceKillTimer = global.setTimeout(() => {
try {
child.kill('SIGKILL')
} catch (killErr) {
core.debug(
`Failed to SIGKILL git process: ${killErr}`
)
}
}, 5000)
if (forceKillTimer.unref) {
forceKillTimer.unref()
}
}, timeoutMs)
child.on('close', (code: number | null) => {
if (settled) return
settled = true
cleanup()
if (timedOut) {
reject(
new Error(
`Git operation timed out after ${timeoutMs / 1000} seconds: git ${args.slice(0, 3).join(' ')}...`
)
)
return
}
// null code means killed by signal (e.g. OOM killer, external SIGTERM)
if (code === null) {
const stderrText = stderr.join('').trim()
reject(
new Error(
`The process 'git' was killed by a signal` +
(stderrText ? `\n${stderrText}` : '')
)
)
return
}
if (code !== 0 && !allowAllExitCodes) {
const stderrText = stderr.join('').trim()
reject(
new Error(
`The process 'git' failed with exit code ${code}` +
(stderrText ? `\n${stderrText}` : '')
)
)
return
}
result.exitCode = code
result.stdout = stdout.join('')
core.debug(result.exitCode.toString())
core.debug(result.stdout)
resolve(result)
})
child.on('error', (err: Error) => {
if (settled) return
settled = true
cleanup()
reject(err)
})
})
}
private async initializeCommandManager( private async initializeCommandManager(
workingDirectory: string, workingDirectory: string,
lfs: boolean, lfs: boolean,

View File

@ -120,20 +120,19 @@ export interface IGitSourceSettings {
githubServerUrl: string | undefined githubServerUrl: string | undefined
/** /**
* Timeout in seconds for each network git operation attempt (fetch, lfs-fetch, ls-remote). * Timeout in seconds for each network git operation attempt (e.g. fetch, lfs-fetch, ls-remote).
* 0 means no timeout. Similar to Kubernetes probe timeoutSeconds. * 0 means no timeout.
*/ */
timeout: number timeout: number
/** /**
* Maximum number of retry attempts for failed network git operations. * Total number of attempts for each network git operation (including the initial attempt).
* Similar to Kubernetes probe failureThreshold. * For example, 3 means one initial attempt plus up to 2 retries.
*/ */
retryMaxAttempts: number retryMaxAttempts: number
/** /**
* Minimum backoff time in seconds between retry attempts. * Minimum backoff time in seconds between retry attempts.
* Similar to Kubernetes probe periodSeconds.
*/ */
retryMinBackoff: number retryMinBackoff: number

View File

@ -161,35 +161,51 @@ export async function getInputs(): Promise<IGitSourceSettings> {
result.githubServerUrl = core.getInput('github-server-url') result.githubServerUrl = core.getInput('github-server-url')
core.debug(`GitHub Host URL = ${result.githubServerUrl}`) core.debug(`GitHub Host URL = ${result.githubServerUrl}`)
// Timeout (per-attempt, like k8s timeoutSeconds) // Timeout per network operation attempt
result.timeout = Math.floor(Number(core.getInput('timeout') || '300')) const timeoutInput = core.getInput('timeout')
result.timeout = Math.floor(Number(timeoutInput !== '' ? timeoutInput : '300'))
if (isNaN(result.timeout) || result.timeout < 0) { if (isNaN(result.timeout) || result.timeout < 0) {
core.warning(
`Invalid value '${timeoutInput}' for 'timeout' input. Using default: 300 seconds.`
)
result.timeout = 300 result.timeout = 300
} }
core.debug(`timeout = ${result.timeout}`) core.debug(`timeout = ${result.timeout}`)
// Retry max attempts (like k8s failureThreshold) // Retry max attempts (total attempts including initial)
const retryMaxAttemptsInput = core.getInput('retry-max-attempts')
result.retryMaxAttempts = Math.floor( result.retryMaxAttempts = Math.floor(
Number(core.getInput('retry-max-attempts') || '3') Number(retryMaxAttemptsInput !== '' ? retryMaxAttemptsInput : '3')
) )
if (isNaN(result.retryMaxAttempts) || result.retryMaxAttempts < 1) { if (isNaN(result.retryMaxAttempts) || result.retryMaxAttempts < 1) {
core.warning(
`Invalid value '${retryMaxAttemptsInput}' for 'retry-max-attempts' input. Using default: 3.`
)
result.retryMaxAttempts = 3 result.retryMaxAttempts = 3
} }
core.debug(`retry max attempts = ${result.retryMaxAttempts}`) core.debug(`retry max attempts = ${result.retryMaxAttempts}`)
// Retry backoff (like k8s periodSeconds, but as a min/max range) // Retry backoff range
const retryMinBackoffInput = core.getInput('retry-min-backoff')
result.retryMinBackoff = Math.floor( result.retryMinBackoff = Math.floor(
Number(core.getInput('retry-min-backoff') || '10') Number(retryMinBackoffInput !== '' ? retryMinBackoffInput : '10')
) )
if (isNaN(result.retryMinBackoff) || result.retryMinBackoff < 0) { if (isNaN(result.retryMinBackoff) || result.retryMinBackoff < 0) {
core.warning(
`Invalid value '${retryMinBackoffInput}' for 'retry-min-backoff' input. Using default: 10 seconds.`
)
result.retryMinBackoff = 10 result.retryMinBackoff = 10
} }
core.debug(`retry min backoff = ${result.retryMinBackoff}`) core.debug(`retry min backoff = ${result.retryMinBackoff}`)
const retryMaxBackoffInput = core.getInput('retry-max-backoff')
result.retryMaxBackoff = Math.floor( result.retryMaxBackoff = Math.floor(
Number(core.getInput('retry-max-backoff') || '20') Number(retryMaxBackoffInput !== '' ? retryMaxBackoffInput : '20')
) )
if (isNaN(result.retryMaxBackoff) || result.retryMaxBackoff < 0) { if (isNaN(result.retryMaxBackoff) || result.retryMaxBackoff < 0) {
core.warning(
`Invalid value '${retryMaxBackoffInput}' for 'retry-max-backoff' input. Using default: 20 seconds.`
)
result.retryMaxBackoff = 20 result.retryMaxBackoff = 20
} }
if (result.retryMaxBackoff < result.retryMinBackoff) { if (result.retryMaxBackoff < result.retryMinBackoff) {