fix(cli): give worktree provisioning a timeout that fits the work
Creating a session with a worktree reported "Request to /api/openchamber/control timed out after 4000ms" while the worktree was in fact created, leaving the user with a failure message, a real worktree, and no session id. Reported alongside worktree creation appearing to take forever. The client HTTP timeout was extended only when the caller asked to wait for the session. Provisioning a worktree is slow on its own: it runs git against the repository and prepares a new directory. Measured on a cold path immediately after a restart it takes about four seconds, which lands exactly on the four second default and explains why this failed intermittently rather than always. A warm run finishes in well under two. The timeout now follows the work being requested rather than only the wait flag, and covers whichever of the two windows is longer. The server always completed the operation, so nothing about the outcome changes: only the client stops abandoning it. Verified by creating a worktree on the cold path immediately after a restart, which previously failed here: 4004 ms and 1376 ms, both reported ok.
This commit is contained in:
@@ -10,14 +10,27 @@ const asNonEmptyString = (value) => {
|
||||
const DEFAULT_WAIT_TIMEOUT_SECONDS = 600;
|
||||
const WAIT_HTTP_TIMEOUT_BUFFER_MS = 30_000;
|
||||
|
||||
// Provisioning a worktree is not one of the instant control calls the short
|
||||
// default is sized for: it runs git against the repository and prepares a new
|
||||
// directory, which on a cold path takes longer than the default allows. The
|
||||
// server finishes the work regardless of the client giving up, so a client-side
|
||||
// timeout here reported a failure for a worktree that was in fact created.
|
||||
const WORKTREE_PROVISION_TIMEOUT_MS = 120_000;
|
||||
|
||||
// The control service blocks server-side while wait is set, so the client
|
||||
// HTTP timeout must outlive the requested wait window instead of the short
|
||||
// default used for instant control calls.
|
||||
export const resolveControlTimeoutMs = (input, options) => {
|
||||
if (Number.isFinite(options?.timeoutMs) && options.timeoutMs > 0) return options.timeoutMs;
|
||||
if (input?.wait !== true) return undefined;
|
||||
const provisionsWorktree = asNonEmptyString(input?.worktree) !== null;
|
||||
if (input?.wait !== true) {
|
||||
return provisionsWorktree ? WORKTREE_PROVISION_TIMEOUT_MS : undefined;
|
||||
}
|
||||
const waitSeconds = Number(input?.timeout) > 0 ? Number(input.timeout) : DEFAULT_WAIT_TIMEOUT_SECONDS;
|
||||
return (waitSeconds * 1000) + WAIT_HTTP_TIMEOUT_BUFFER_MS;
|
||||
const waitTimeoutMs = (waitSeconds * 1000) + WAIT_HTTP_TIMEOUT_BUFFER_MS;
|
||||
// Waiting for the session and provisioning its worktree are additive, so the
|
||||
// window must cover whichever is longer rather than only the wait.
|
||||
return provisionsWorktree ? Math.max(waitTimeoutMs, WORKTREE_PROVISION_TIMEOUT_MS) : waitTimeoutMs;
|
||||
};
|
||||
|
||||
export const requestControlAction = async (port, action, input, options = {}) => {
|
||||
|
||||
@@ -19,4 +19,20 @@ describe('resolveControlTimeoutMs', () => {
|
||||
it('never shrinks an explicitly requested HTTP timeout', () => {
|
||||
expect(resolveControlTimeoutMs({ wait: true, timeout: 30 }, { timeoutMs: 5000 })).toBe(5000);
|
||||
});
|
||||
|
||||
it('allows a worktree to be provisioned without waiting for the session', () => {
|
||||
expect(resolveControlTimeoutMs({ worktree: 'feature' }, {})).toBe(120_000);
|
||||
});
|
||||
|
||||
it('ignores a blank worktree name', () => {
|
||||
expect(resolveControlTimeoutMs({ worktree: ' ' }, {})).toBeUndefined();
|
||||
});
|
||||
|
||||
it('covers worktree provisioning even when the wait window is shorter', () => {
|
||||
expect(resolveControlTimeoutMs({ wait: true, timeout: 30, worktree: 'feature' }, {})).toBe(120_000);
|
||||
});
|
||||
|
||||
it('keeps a longer wait window when it outlasts worktree provisioning', () => {
|
||||
expect(resolveControlTimeoutMs({ wait: true, worktree: 'feature' }, {})).toBe(630_000);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user