From b7bf7fba07a8ad7314629b346d64d7c93a420afd Mon Sep 17 00:00:00 2001 From: Jack Hsu Date: Tue, 24 Mar 2026 21:37:53 -0400 Subject: [PATCH] fix(core): add timeouts to GitHub push flow to prevent CLI hangs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Current Behavior The `pushToGitHub` flow calls `gh api user` and `gh repo create --push` with no timeout via `spawnAndWait()`/`execAndWait()`. When gh CLI is wrapped by 1Password SSH agent, credential managers with GUI prompts, SSH keys with passphrases, or corporate SSO, these calls hang indefinitely—freezing the CLI after workspace creation with no indication of what's happening. YTD data shows a 96.3% failure rate (16,426 failures out of 17,058 attempts). ## Expected Behavior - The initial `gh` auth check (`gh api user`) has a 2-second timeout. If gh is slow to respond (hung on credential prompt), the push flow is skipped gracefully instead of freezing. - The `gh repo create --push` command has a 60-second timeout. If the push hangs, the process is killed and a helpful message is shown. - If `gh` CLI is not installed, the push flow is skipped immediately without attempting any network calls. - Error messages now include actionable fallback: "You can push manually with: git push -u origin main". ## Related Issue(s) Fixes NXC-4141 --- .../bin/create-nx-workspace.ts | 1 + .../src/create-workspace.ts | 22 ++- .../src/utils/child-process-utils.ts | 27 +++- .../create-nx-workspace/src/utils/git/git.ts | 151 ++++++++++++------ 4 files changed, 144 insertions(+), 57 deletions(-) diff --git a/packages/create-nx-workspace/bin/create-nx-workspace.ts b/packages/create-nx-workspace/bin/create-nx-workspace.ts index d2acba2f06c..2a5d2520157 100644 --- a/packages/create-nx-workspace/bin/create-nx-workspace.ts +++ b/packages/create-nx-workspace/bin/create-nx-workspace.ts @@ -424,6 +424,7 @@ async function main(parsedArgs: yargs.Arguments) { nxCloudArg: parsedArgs.nxCloud ?? '', nxCloudArgRaw: rawArgs.nxCloud ?? '', pushedToVcs: workspaceInfo.pushedToVcs ?? '', + pushFailReason: workspaceInfo.pushFailReason ?? '', template: chosenTemplate ?? '', preset: chosenPreset ?? '', connectUrl: workspaceInfo.connectUrl ?? '', diff --git a/packages/create-nx-workspace/src/create-workspace.ts b/packages/create-nx-workspace/src/create-workspace.ts index 0fad971332b..4e32887ee57 100644 --- a/packages/create-nx-workspace/src/create-workspace.ts +++ b/packages/create-nx-workspace/src/create-workspace.ts @@ -7,6 +7,7 @@ import { CreateWorkspaceOptions } from './create-workspace-options'; import { setupCI } from './utils/ci/setup-ci'; import { mapErrorToBodyLines } from './utils/error-utils'; import { + GitHubPushError, initializeGitRepo, pushToGitHub, VcsPushStatus, @@ -205,6 +206,7 @@ export async function createWorkspace( } let pushedToVcs = VcsPushStatus.SkippedGit; + let pushFailReason: string | undefined; if (!skipGit) { const aiMode = isAiAgent(); @@ -231,14 +233,29 @@ export async function createWorkspace( }); } } catch (e) { - if (e instanceof Error) { + if (e instanceof GitHubPushError) { + // GitHub push issues are never fatal — CNW always succeeds. + // All reasons are logged in telemetry via pushFailReason. + pushedToVcs = VcsPushStatus.FailedToPushToVcs; + pushFailReason = e.reason; + + // Only show the push hint when the user actually attempted a push + // and it failed. Pre-push issues (gh not installed, auth failed, + // timed out during auth) are silent — no point telling the user + // about a push they never asked for. + if (e.reason === 'push-failed' || e.reason === 'push-timeout') { + const githubNewUrl = `https://github.com/new?name=${encodeURIComponent(name)}`; + output.log({ + title: `Push your repo to GitHub: ${githubNewUrl}`, + }); + } + } else if (e instanceof Error) { if (!aiMode) { output.error({ title: 'Could not initialize git repository', bodyLines: mapErrorToBodyLines(e), }); } - // In AI mode, error will be handled by the caller } else { console.error(e); } @@ -305,6 +322,7 @@ export async function createWorkspace( nxCloudInfo, directory, pushedToVcs, + pushFailReason, connectUrl, }; } diff --git a/packages/create-nx-workspace/src/utils/child-process-utils.ts b/packages/create-nx-workspace/src/utils/child-process-utils.ts index bb905a2e775..cfcd0c5ef90 100644 --- a/packages/create-nx-workspace/src/utils/child-process-utils.ts +++ b/packages/create-nx-workspace/src/utils/child-process-utils.ts @@ -6,7 +6,12 @@ import { CnwError } from './error-utils'; /** * Use spawn only for interactive shells */ -export function spawnAndWait(command: string, args: string[], cwd: string) { +export function spawnAndWait( + command: string, + args: string[], + cwd: string, + timeout?: number +) { return new Promise((res, rej) => { // Combine command and args into a single string to avoid DEP0190 warning // (passing args with shell: true is deprecated) @@ -28,7 +33,21 @@ export function spawnAndWait(command: string, args: string[], cwd: string) { windowsHide: true, }); + let timedOut = false; + let timer: ReturnType | undefined; + if (timeout) { + timer = setTimeout(() => { + timedOut = true; + childProcess.kill('SIGTERM'); + }, timeout); + } + childProcess.on('exit', (code, signal) => { + if (timer) clearTimeout(timer); + if (timedOut) { + rej({ code: 1, timedOut: true }); + return; + } if (code === null) code = signalToCode(signal); if (code !== 0) { rej({ code: code }); @@ -42,7 +61,8 @@ export function spawnAndWait(command: string, args: string[], cwd: string) { export function execAndWait( command: string, cwd: string, - silenceErrors = false + silenceErrors = false, + timeout?: number ) { return new Promise<{ code: number; stdout: string }>((res, rej) => { exec( @@ -52,11 +72,12 @@ export function execAndWait( env: { ...process.env, NX_DAEMON: 'false' }, windowsHide: true, maxBuffer: 1024 * 1024 * 10, // 10MB — default 1MB can be exceeded by verbose PM output + ...(timeout ? { timeout } : {}), }, (error, stdout, stderr) => { if (error) { if (silenceErrors) { - rej(); + rej(error.killed ? { timedOut: true } : undefined); } else { const logFile = join(cwd, 'error.log'); writeFileSync(logFile, `${stdout}\n${stderr}`); diff --git a/packages/create-nx-workspace/src/utils/git/git.ts b/packages/create-nx-workspace/src/utils/git/git.ts index 0ec9114ecdb..b9759c586bd 100644 --- a/packages/create-nx-workspace/src/utils/git/git.ts +++ b/packages/create-nx-workspace/src/utils/git/git.ts @@ -1,7 +1,7 @@ import { execSync } from 'child_process'; import { deduceDefaultBase } from './default-base'; import { output } from '../output'; -import { execAndWait, spawnAndWait } from '../child-process-utils'; +import { execAndWait } from '../child-process-utils'; import enquirer from 'enquirer'; export enum VcsPushStatus { @@ -11,12 +11,18 @@ export enum VcsPushStatus { SkippedGit = 'SkippedGit', } -export class GitHubPushSkippedError extends Error { - public readonly title = 'Push your workspace'; - - constructor(message: string) { +export class GitHubPushError extends Error { + constructor( + message: string, + public readonly reason: + | 'gh-not-installed' + | 'gh-auth-failed' + | 'push-timeout' + | 'push-failed' + | 'env-skip' + ) { super(message); - this.name = 'GitHubPushSkippedError'; + this.name = 'GitHubPushError'; } } @@ -43,26 +49,47 @@ export function isGitAvailable(): boolean { } } +// 1 second timeout for gh CLI pre-flight checks (version, auth). If gh is +// wrapped by 1Password, a credential manager, or corporate SSO the call can +// hang indefinitely. Better to skip the push than freeze the CLI. +const GH_CLI_TIMEOUT_MS = 1_000; +// 10 second timeout for repo listing (runs in background while user answers prompts). +const GH_LIST_TIMEOUT_MS = 10_000; +// 30 second timeout for the actual repo create + push operation. +// Longer than other gh commands to account for slow networks. +const GH_PUSH_TIMEOUT_MS = 30_000; + /** * Synchronously checks if GitHub CLI (gh) is available on the system. - * Returns true if gh command can be executed, false otherwise. + * Returns true if gh command can be executed within 2 seconds, false otherwise. */ export function isGhCliAvailable(): boolean { try { - execSync('gh --version', { stdio: 'ignore', windowsHide: true }); + execSync('gh --version', { + stdio: 'ignore', + windowsHide: true, + timeout: GH_CLI_TIMEOUT_MS, + }); return true; } catch { return false; } } -async function getGitHubUsername(directory: string): Promise { - const result = await execAndWait('gh api user --jq .login', directory); - const username = result.stdout.trim(); - if (!username) { - throw new GitHubPushSkippedError('GitHub CLI is not authenticated'); +async function getGitHubUsername(directory: string): Promise { + try { + const result = await execAndWait( + 'gh api user --jq .login', + directory, + true, // silenceErrors — gh failures should never write error.log (#34482) + GH_CLI_TIMEOUT_MS + ); + return result.stdout.trim() || null; + } catch { + // gh is optional — auth failures, timeouts, or missing credentials + // are silently ignored. The push flow will be skipped. + return null; } - return username; } // Module-level promise for background repo fetching @@ -81,9 +108,16 @@ async function getUserRepositories(directory: string): Promise> { const [userRepos, orgsResult] = await Promise.all([ execAndWait( 'gh repo list --limit 100 --json nameWithOwner --jq ".[].nameWithOwner"', - directory + directory, + true, // silenceErrors — gh failures should never write error.log + GH_LIST_TIMEOUT_MS + ), + execAndWait( + 'gh api user/orgs --jq ".[].login"', + directory, + true, + GH_LIST_TIMEOUT_MS ), - execAndWait('gh api user/orgs --jq ".[].login"', directory), ]); // Add user's personal repos @@ -104,7 +138,9 @@ async function getUserRepositories(directory: string): Promise> { try { const orgRepos = await execAndWait( `gh repo list ${org} --limit 100 --json nameWithOwner --jq ".[].nameWithOwner"`, - directory + directory, + true, // silenceErrors + GH_LIST_TIMEOUT_MS ); return orgRepos.stdout .trim() @@ -200,16 +236,24 @@ export async function pushToGitHub( } ): Promise { try { + // Pre-flight gates — gh is optional, so any failure here throws + // GitHubPushError which the caller handles silently (no user output) + // while still recording the reason in telemetry. if (process.env['NX_SKIP_GH_PUSH'] === 'true') { - throw new GitHubPushSkippedError( - 'NX_SKIP_GH_PUSH is true so skipping GitHub push.' - ); + throw new GitHubPushError('NX_SKIP_GH_PUSH is true', 'env-skip'); + } + if (!isGhCliAvailable()) { + throw new GitHubPushError('gh CLI is not installed', 'gh-not-installed'); } - // Note: This call can throw an error even if user hasn't opted in to push yet, - // which could be confusing as they haven't been asked about GitHub push at this point. - // We check gh authentication early to provide a better error message. + // Check gh authentication with a short timeout. If gh is wrapped by + // 1Password, a credential manager, or corporate SSO this call can hang + // indefinitely. A 2 s timeout catches that and skips the push gracefully + // instead of freezing the CLI. const username = await getGitHubUsername(directory); + if (!username) { + throw new GitHubPushError('gh auth failed', 'gh-auth-failed'); + } // Start fetching existing repositories in the background immediately // This runs while user is answering prompts, so validation is usually instant @@ -259,46 +303,49 @@ export async function pushToGitHub( }, ]); - // Create GitHub repository using gh CLI from the workspace directory - // This will automatically add remote origin and push the current branch + // Create GitHub repository and push using gh CLI. + // Uses execAndWait (not spawnAndWait) so output is captured rather than + // streamed to the terminal. This prevents git push output from bleeding + // into the terminal after CNW exits, and ensures the timeout properly + // kills the entire process tree. output.log({ title: 'Creating GitHub repository and pushing (this may take a moment)...', }); - await spawnAndWait( - 'gh', - [ - 'repo', - 'create', - repoName, - '--private', - '--push', - '--source', - directory, - ], - directory - ); + const cmd = `gh repo create ${repoName} --private --push --source "${directory}"`; + await execAndWait(cmd, directory, true, GH_PUSH_TIMEOUT_MS); // Get the actual repository URL from GitHub CLI (it could be different from github.com) - const repoResult = await execAndWait( - 'gh repo view --json url -q .url', - directory - ); - const repoUrl = repoResult.stdout.trim(); + let repoUrl = `https://github.com/${repoName}`; + try { + const repoResult = await execAndWait( + 'gh repo view --json url -q .url', + directory, + true, // silenceErrors + GH_CLI_TIMEOUT_MS + ); + if (repoResult.stdout.trim()) { + repoUrl = repoResult.stdout.trim(); + } + } catch { + // Fall back to constructed URL + } output.success({ title: `Successfully pushed to GitHub repository: ${repoUrl}`, }); return VcsPushStatus.PushedToVcs; } catch (e) { - // Error code 127 means gh wasn't installed - // GitHubPushSkippedError means user hasn't opted in or we couldn't authenticate - const title = - e instanceof GitHubPushSkippedError || (e as any)?.code === 127 - ? 'Push your workspace to GitHub.' - : 'Could not push.'; - - output.log({ title }); - return VcsPushStatus.FailedToPushToVcs; + // Re-throw GitHubPushError as-is (gh not installed, auth failed, env skip) + if (e instanceof GitHubPushError) throw e; + + // Wrap other failures (push timeout, push command failed) as GitHubPushError + const isTimedOut = (e as any)?.timedOut === true; + if (isTimedOut) { + throw new GitHubPushError('gh push timed out', 'push-timeout'); + } + + const msg = e instanceof Error ? e.message : String(e); + throw new GitHubPushError(msg.split('\n')[0].slice(0, 200), 'push-failed'); } }