From 9d8598255d714222a098dbc2b06257437d231669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Thu, 8 Oct 2026 14:30:13 +0200 Subject: [PATCH 1/5] fix(init): stop the name prompt looping when stdin is not a TTY `apify init` with no name and a non-TTY stdin never terminated: the retry loop around the prompt treated a permanent failure as retryable and wrote ~28 MB/s to stderr until the process was killed. - --yes now uses the current directory name, as the help already promised. - The name prompt validates inside inquirer, so a typo re-asks and a prompt that cannot be answered fails once with a message naming the escape hatch. - The directory confirmation no longer turns "cannot ask" into "no", which exited 0 having done nothing. Co-Authored-By: Claude Opus 5 --- src/commands/init.ts | 50 ++++--- test/e2e/__helpers__/run-cli-bounded.ts | 112 +++++++++++++++ test/e2e/commands/init-non-tty.test.ts | 84 +++++++++++ .../commands/init-non-interactive.test.ts | 130 ++++++++++++++++++ 4 files changed, 360 insertions(+), 16 deletions(-) create mode 100644 test/e2e/__helpers__/run-cli-bounded.ts create mode 100644 test/e2e/commands/init-non-tty.test.ts create mode 100644 test/local/commands/init-non-interactive.test.ts diff --git a/src/commands/init.ts b/src/commands/init.ts index 87f02ceb0..ee2aef4ae 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -102,7 +102,7 @@ export class InitCommand extends ApifyCommand { const confirmed = await useYesNoConfirm({ message: 'Do you want to continue?', - providedConfirmFromStdin: this.flags.yes, + errorMessageForStdin: 'Confirmation is required to continue. Pass --yes to skip the prompt.', }); if (!confirmed) { @@ -124,25 +124,43 @@ export class InitCommand extends ApifyCommand { } if (!actorName) { - let response = actorConfig.isOkAnd((cfg) => cfg.exists) ? (actorConfig.unwrap().config.name as string) : null; - - // TODO: see if we can use promptActorName instead - while (!response) { + const existingName = actorConfig.isOkAnd((cfg) => cfg.exists) + ? (actorConfig.unwrap().config.name as string) + : null; + + if (existingName) { + actorName = existingName; + } else if (this.flags.yes) { + // A directory name is not necessarily a legal Actor name, and --yes has no prompt to + // fall back to. try { - const answer = await useUserInput({ - message: 'Actor name:', - default: defaultActorName, - }); - - validateActorName(answer); - - response = answer; + validateActorName(defaultActorName); } catch (err) { - error({ message: (err as Error).message }); + throw new Error( + `'${defaultActorName}' cannot be used as the Actor name. ${(err as Error).message} Run 'apify init ' to set it.`, + ); } - } - actorName = response; + actorName = defaultActorName; + } else { + // Validating inside the prompt lets inquirer re-ask on a typo. Retrying around the + // prompt instead would also retry a prompt that cannot be answered at all. + actorName = await useUserInput({ + message: 'Actor name:', + default: defaultActorName, + validate: (value) => { + try { + validateActorName(value); + } catch (err) { + return (err as Error).message; + } + + return true; + }, + errorMessageForStdin: + "Actor name is required. Run 'apify init ', or pass --yes to use the current directory name.", + }); + } } // Migrate apify.json to .actor/actor.json diff --git a/test/e2e/__helpers__/run-cli-bounded.ts b/test/e2e/__helpers__/run-cli-bounded.ts new file mode 100644 index 000000000..00432b189 --- /dev/null +++ b/test/e2e/__helpers__/run-cli-bounded.ts @@ -0,0 +1,112 @@ +import { spawn } from 'node:child_process'; +import { mkdir } from 'node:fs/promises'; +import process from 'node:process'; +import { fileURLToPath } from 'node:url'; + +const ProjectRoot = new URL('../../../', import.meta.url); +const DistApify = fileURLToPath(new URL('./dist/apify.js', ProjectRoot)); +const DistActor = fileURLToPath(new URL('./dist/actor.js', ProjectRoot)); + +export interface RunCliBoundedOptions { + cwd: string; + env?: Record; + /** Wall-clock budget. The child is killed once it elapses. */ + timeoutMs?: number; + /** Combined byte budget for stdout and stderr. The child is killed once it is passed. */ + outputCapBytes?: number; +} + +export interface RunCliBoundedResult { + stdout: string; + stderr: string; + exitCode: number | null; + signal: NodeJS.Signals | null; + /** The command was still running when `timeoutMs` elapsed. */ + timedOut: boolean; + /** The command wrote past `outputCapBytes`, which only a runaway loop does. */ + outputCapExceeded: boolean; + /** Bytes written across both streams, up to the point the child was killed. */ + bytesWritten: number; +} + +const DefaultTimeoutMs = 15_000; + +/** Two orders of magnitude above any command's real output, and a few milliseconds of a loop. */ +const DefaultOutputCapBytes = 256 * 1024; + +/** + * Run a CLI command against the freshly built `./dist` (requires `pnpm run build`) with stdin + * pointed at `/dev/null` — the non-TTY, no-data case that CI runners, agent shells and + * `cmd < /dev/null` produce, and the one every prompting command must survive. + * + * A hang and a retry loop both look like "the command never finished", so a deadline alone cannot + * tell them apart and costs the full deadline either way. The output cap separates them and trips + * in milliseconds: `outputCapExceeded` means the command retried something it can never succeed at. + * + * Use this for any command that prompts. `runCli` stays the right helper everywhere else — it + * buffers without a cap, so a loop there costs tens of seconds before the timeout fires. + */ +export async function runCliBounded( + binary: 'apify' | 'actor', + args: string[], + options: RunCliBoundedOptions, +): Promise { + const { cwd, env, timeoutMs = DefaultTimeoutMs, outputCapBytes = DefaultOutputCapBytes } = options; + + await mkdir(cwd, { recursive: true }); + + const child = spawn(process.execPath, [binary === 'actor' ? DistActor : DistApify, ...args], { + cwd, + stdio: ['ignore', 'pipe', 'pipe'], + env: { + ...process.env, + APIFY_CLI_DISABLE_TELEMETRY: '1', + APIFY_CLI_SKIP_UPDATE_CHECK: '1', + APIFY_CLI_SKIP_RENTAL_SUNSET_NOTICE: '1', + APIFY_DISABLE_KEYRING: '1', + APIFY_TOKEN: '', + ...env, + }, + }); + + const captured = { stdout: [] as Buffer[], stderr: [] as Buffer[] }; + let bytesWritten = 0; + let outputCapExceeded = false; + let timedOut = false; + + const capture = (stream: 'stdout' | 'stderr') => (chunk: Buffer) => { + bytesWritten += chunk.length; + + // Keep the head only: past the cap the tail is the same line repeated, and holding it costs + // hundreds of megabytes before the kill lands. + if (!outputCapExceeded) captured[stream].push(chunk); + + if (bytesWritten > outputCapBytes && !outputCapExceeded) { + outputCapExceeded = true; + child.kill('SIGKILL'); + } + }; + + child.stdout.on('data', capture('stdout')); + child.stderr.on('data', capture('stderr')); + + const timer = setTimeout(() => { + timedOut = true; + child.kill('SIGKILL'); + }, timeoutMs); + + const [exitCode, signal] = await new Promise<[number | null, NodeJS.Signals | null]>((resolve, reject) => { + child.once('error', reject); + child.once('close', (code, sig) => resolve([code, sig])); + }).finally(() => clearTimeout(timer)); + + return { + stdout: Buffer.concat(captured.stdout).toString('utf8'), + stderr: Buffer.concat(captured.stderr).toString('utf8'), + exitCode, + signal, + timedOut, + outputCapExceeded, + bytesWritten, + }; +} diff --git a/test/e2e/commands/init-non-tty.test.ts b/test/e2e/commands/init-non-tty.test.ts new file mode 100644 index 000000000..dff7f2bf2 --- /dev/null +++ b/test/e2e/commands/init-non-tty.test.ts @@ -0,0 +1,84 @@ +import { randomBytes } from 'node:crypto'; +import { access, mkdir, rm, writeFile } from 'node:fs/promises'; +import path from 'node:path'; + +import { runCliBounded } from '../__helpers__/run-cli-bounded.js'; +import { TestTmpRoot } from '../__helpers__/tmp.js'; + +// The command itself finishes well under a second. A deadline only has to separate "exits" from +// "never exits"; a retry loop trips the output cap long before this. +const EXIT_DEADLINE_MS = 15_000; + +const MissingNameError = 'Error: Actor name is required'; + +function countOccurrences(haystack: string, needle: string) { + return haystack.split(needle).length - 1; +} + +async function makeNodeProject(dir: string) { + await mkdir(dir, { recursive: true }); + await writeFile(path.join(dir, 'package.json'), JSON.stringify({ name: 'p', version: '1.0.0', main: 'index.js' })); + await writeFile(path.join(dir, 'index.js'), 'console.log(1);\n'); +} + +describe('[e2e] apify init with no usable stdin', () => { + let root: string; + + beforeAll(async () => { + root = path.join(TestTmpRoot, `e2e-init-non-tty-${randomBytes(6).toString('hex')}`); + await mkdir(root, { recursive: true }); + }); + + afterAll(async () => { + await rm(root, { recursive: true, force: true }); + }); + + it('--yes with no name initializes with the directory name', async () => { + const cwd = path.join(root, 'yes-default-name'); + await makeNodeProject(cwd); + + const result = await runCliBounded('apify', ['init', '--yes'], { cwd, timeoutMs: EXIT_DEADLINE_MS }); + + expect(result.outputCapExceeded, `the prompt retried instead of failing once (${result.bytesWritten} bytes)`).toBe( + false, + ); + expect(result.timedOut, `stderr: ${result.stderr}`).toBe(false); + expect(result.exitCode, `stderr: ${result.stderr}`).toBe(0); + expect(result.stderr).toContain('The Actor has been initialized in the current directory.'); + + await expect(access(path.join(cwd, '.actor', 'actor.json'))).resolves.toBeUndefined(); + }); + + it('without --yes fails once instead of retrying the name prompt', async () => { + const cwd = path.join(root, 'no-yes-missing-name'); + await makeNodeProject(cwd); + + const result = await runCliBounded('apify', ['init'], { cwd, timeoutMs: EXIT_DEADLINE_MS }); + + expect(result.outputCapExceeded, `the prompt retried instead of failing once (${result.bytesWritten} bytes)`).toBe( + false, + ); + expect(result.timedOut, `stderr: ${result.stderr}`).toBe(false); + expect(result.exitCode).not.toBe(0); + expect(countOccurrences(result.stderr, MissingNameError)).toBe(1); + expect(result.stderr).toContain('apify init '); + expect(result.stderr).toContain('--yes'); + + await expect(access(path.join(cwd, '.actor', 'actor.json'))).rejects.toThrow(); + }); + + it('without --yes fails in a directory that is not a Node.js or Python project', async () => { + const cwd = path.join(root, 'no-yes-unknown-project'); + await mkdir(cwd, { recursive: true }); + + const result = await runCliBounded('apify', ['init'], { cwd, timeoutMs: EXIT_DEADLINE_MS }); + + expect(result.outputCapExceeded, `the prompt retried instead of failing once (${result.bytesWritten} bytes)`).toBe( + false, + ); + expect(result.timedOut, `stderr: ${result.stderr}`).toBe(false); + expect(result.exitCode).not.toBe(0); + + await expect(access(path.join(cwd, '.actor', 'actor.json'))).rejects.toThrow(); + }); +}); diff --git a/test/local/commands/init-non-interactive.test.ts b/test/local/commands/init-non-interactive.test.ts new file mode 100644 index 000000000..fa58f7111 --- /dev/null +++ b/test/local/commands/init-non-interactive.test.ts @@ -0,0 +1,130 @@ +import { existsSync, readFileSync, writeFileSync } from 'node:fs'; +import { mkdir } from 'node:fs/promises'; +import { join } from 'node:path'; +import process from 'node:process'; + +// `isCI` short-circuits every prompt in `stdinCheckWrapper` into a throw, which is what a non-TTY +// stdin does in the field. See src/lib/hooks/user-confirmations/_stdinCheckWrapper.ts. +vitest.mock('ci-info', async (importOriginal) => { + const original = await importOriginal(); + return { ...original, isCI: true }; +}); + +// The global `restoreMocks` would undo useTempPath's process.cwd spy after the first test, pointing +// every later test at the repo root instead of the temp directory. +vitest.setConfig({ restoreMocks: false }); + +import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; +import { LOCAL_CONFIG_PATH } from '../../../src/lib/consts.js'; +import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; + +const { tmpPath, beforeAllCalls, afterAllCalls, forceNewCwd } = useTempPath('init-non-interactive', { + create: true, + remove: true, + cwd: true, + cwdParent: false, +}); + +const { InitCommand } = await import('../../../src/commands/init.js'); + +// A retry loop around a prompt that can never succeed writes until the process is killed. The cap +// turns that into a failed assertion in milliseconds instead of a test timeout after minutes. +const MAX_ERROR_LINES = 20; + +function captureErrorsWithCap() { + const lines: string[] = []; + + vitest.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + lines.push(args.map(String).join(' ')); + + if (lines.length > MAX_ERROR_LINES) { + throw new Error(`Prompt retry loop: console.error was called more than ${MAX_ERROR_LINES} times.`); + } + }); + + vitest.spyOn(console, 'log').mockImplementation(() => {}); + + return lines; +} + +function errorLines(lines: string[]) { + return lines.filter((line) => line.includes('Error:')); +} + +// `resetCwdCaches` resolves the hook modules outside the `node:process` mock graph, so it cannot +// clear the caches the command actually reads. A directory per test keeps the cache keys apart. +async function useFreshCwd(name: string) { + forceNewCwd(name); + await mkdir(join(tmpPath, name), { recursive: true }); + + return (...paths: string[]) => join(tmpPath, name, ...paths); +} + +describe('apify init without a usable stdin', () => { + beforeEach(async () => { + await beforeAllCalls(); + process.exitCode = undefined; + }); + + afterEach(async () => { + process.exitCode = undefined; + await afterAllCalls(); + }); + + it('--yes with no name uses the current directory name', async () => { + const joinCwd = await useFreshCwd('yes-default-name'); + writeFileSync(joinCwd('package.json'), JSON.stringify({ name: 'pkg', version: '1.0.0' })); + + const lines = captureErrorsWithCap(); + + await testRunCommand(InitCommand, { flags_yes: true }); + + expect(errorLines(lines)).toStrictEqual([]); + expect(process.exitCode ?? 0).toBe(0); + expect(JSON.parse(readFileSync(joinCwd(LOCAL_CONFIG_PATH), 'utf8')).name).toBe('yes-default-name'); + }); + + it('--yes fails when the directory name is not a legal Actor name', async () => { + const joinCwd = await useFreshCwd('yes_bad_default_name'); + writeFileSync(joinCwd('package.json'), JSON.stringify({ name: 'pkg', version: '1.0.0' })); + + const lines = captureErrorsWithCap(); + + await testRunCommand(InitCommand, { flags_yes: true }); + + expect(errorLines(lines)).toHaveLength(1); + expect(errorLines(lines)[0]).toContain('apify init '); + expect(process.exitCode).toBeDefined(); + expect(process.exitCode).not.toBe(0); + expect(existsSync(joinCwd(LOCAL_CONFIG_PATH))).toBe(false); + }); + + it('without --yes reports the missing name once and exits non-zero', async () => { + const joinCwd = await useFreshCwd('no-yes-missing-name'); + writeFileSync(joinCwd('package.json'), JSON.stringify({ name: 'pkg', version: '1.0.0' })); + + const lines = captureErrorsWithCap(); + + await testRunCommand(InitCommand, {}); + + expect(errorLines(lines)).toHaveLength(1); + expect(errorLines(lines)[0]).toContain('apify init '); + expect(errorLines(lines)[0]).toContain('--yes'); + expect(process.exitCode).toBeDefined(); + expect(process.exitCode).not.toBe(0); + expect(existsSync(joinCwd(LOCAL_CONFIG_PATH))).toBe(false); + }); + + it('does not exit 0 when the directory confirmation cannot be answered', async () => { + const joinCwd = await useFreshCwd('no-yes-unknown-project'); + + const lines = captureErrorsWithCap(); + + await testRunCommand(InitCommand, {}); + + expect(errorLines(lines)).toHaveLength(1); + expect(process.exitCode).toBeDefined(); + expect(process.exitCode).not.toBe(0); + expect(existsSync(joinCwd(LOCAL_CONFIG_PATH))).toBe(false); + }); +}); From fcb5931669433127844227ec0cbb796d74c1d89c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Thu, 8 Oct 2026 15:17:25 +0200 Subject: [PATCH 2/5] fix(init): sanitize the directory name before using it as the default A directory called `my_project` or `scraper.v2` is not a legal Actor name, so --yes failed where the help says it should succeed. The Python branch already sanitized its package name; do the same for the directory name, and keep the clean error for names that sanitizing cannot save. Also deduplicate the dist paths and spawn environment shared by the two e2e runners. Co-Authored-By: Claude Opus 5 --- src/commands/init.ts | 15 +++++-------- test/e2e/__helpers__/run-cli-bounded.ts | 16 +++++--------- test/e2e/__helpers__/run-cli.ts | 22 +++++++++++-------- .../commands/init-non-interactive.test.ts | 17 ++++++++++++-- 4 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/commands/init.ts b/src/commands/init.ts index ee2aef4ae..c2dec88ab 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -82,7 +82,7 @@ export class InitCommand extends ApifyCommand { } } - let defaultActorName = basename(cwd); + let defaultActorName = sanitizeActorName(basename(cwd)); if (project.type === ProjectLanguage.Python && project.entrypoint?.path) { const entryPath = project.entrypoint.path; // Extract the actual package name (last segment of dotted path) @@ -131,20 +131,17 @@ export class InitCommand extends ApifyCommand { if (existingName) { actorName = existingName; } else if (this.flags.yes) { - // A directory name is not necessarily a legal Actor name, and --yes has no prompt to - // fall back to. + // Sanitizing covers the usual separators, but a name of only dashes or non-ASCII + // leaves nothing behind, and --yes has no prompt to fall back to. try { validateActorName(defaultActorName); - } catch (err) { - throw new Error( - `'${defaultActorName}' cannot be used as the Actor name. ${(err as Error).message} Run 'apify init ' to set it.`, - ); + } catch { + throw new Error(`Cannot derive an Actor name from '${basename(cwd)}'. Run 'apify init ' to set it.`); } actorName = defaultActorName; } else { - // Validating inside the prompt lets inquirer re-ask on a typo. Retrying around the - // prompt instead would also retry a prompt that cannot be answered at all. + // Validating inside the prompt lets inquirer re-ask on a typo. actorName = await useUserInput({ message: 'Actor name:', default: defaultActorName, diff --git a/test/e2e/__helpers__/run-cli-bounded.ts b/test/e2e/__helpers__/run-cli-bounded.ts index 00432b189..65a308119 100644 --- a/test/e2e/__helpers__/run-cli-bounded.ts +++ b/test/e2e/__helpers__/run-cli-bounded.ts @@ -1,11 +1,8 @@ import { spawn } from 'node:child_process'; import { mkdir } from 'node:fs/promises'; import process from 'node:process'; -import { fileURLToPath } from 'node:url'; -const ProjectRoot = new URL('../../../', import.meta.url); -const DistApify = fileURLToPath(new URL('./dist/apify.js', ProjectRoot)); -const DistActor = fileURLToPath(new URL('./dist/actor.js', ProjectRoot)); +import { CliTestEnv, DistActor, DistApify } from './run-cli.js'; export interface RunCliBoundedOptions { cwd: string; @@ -43,8 +40,8 @@ const DefaultOutputCapBytes = 256 * 1024; * tell them apart and costs the full deadline either way. The output cap separates them and trips * in milliseconds: `outputCapExceeded` means the command retried something it can never succeed at. * - * Use this for any command that prompts. `runCli` stays the right helper everywhere else — it - * buffers without a cap, so a loop there costs tens of seconds before the timeout fires. + * Use this for any command that prompts. `runCli` stays the right helper everywhere else: execa's + * own `maxBuffer` can cap output too, but it takes seconds to trip where this takes milliseconds. */ export async function runCliBounded( binary: 'apify' | 'actor', @@ -58,13 +55,10 @@ export async function runCliBounded( const child = spawn(process.execPath, [binary === 'actor' ? DistActor : DistApify, ...args], { cwd, stdio: ['ignore', 'pipe', 'pipe'], + // `spawn` replaces the environment wholesale, unlike execa's `extendEnv`. env: { ...process.env, - APIFY_CLI_DISABLE_TELEMETRY: '1', - APIFY_CLI_SKIP_UPDATE_CHECK: '1', - APIFY_CLI_SKIP_RENTAL_SUNSET_NOTICE: '1', - APIFY_DISABLE_KEYRING: '1', - APIFY_TOKEN: '', + ...CliTestEnv, ...env, }, }); diff --git a/test/e2e/__helpers__/run-cli.ts b/test/e2e/__helpers__/run-cli.ts index 3915832f8..746bb9a25 100644 --- a/test/e2e/__helpers__/run-cli.ts +++ b/test/e2e/__helpers__/run-cli.ts @@ -4,8 +4,18 @@ import { fileURLToPath } from 'node:url'; import { execa } from 'execa'; const ProjectRoot = new URL('../../../', import.meta.url); -const DistApify = fileURLToPath(new URL('./dist/apify.js', ProjectRoot)); -const DistActor = fileURLToPath(new URL('./dist/actor.js', ProjectRoot)); +export const DistApify = fileURLToPath(new URL('./dist/apify.js', ProjectRoot)); +export const DistActor = fileURLToPath(new URL('./dist/actor.js', ProjectRoot)); + +export const CliTestEnv = { + APIFY_CLI_DISABLE_TELEMETRY: '1', + APIFY_CLI_SKIP_UPDATE_CHECK: '1', + APIFY_CLI_SKIP_RENTAL_SUNSET_NOTICE: '1', + // Pin the file backend so e2e subprocesses don't share the host's OS keyring across tests. + APIFY_DISABLE_KEYRING: '1', + // The resolver prefers APIFY_TOKEN, so a token on the host would pick the account. + APIFY_TOKEN: '', +}; export interface RunCliOptions { cwd?: string | URL; @@ -48,13 +58,7 @@ export async function runCli( stdin: options.stdin ? 'pipe' : 'ignore', input: options.stdin, env: { - APIFY_CLI_DISABLE_TELEMETRY: '1', - APIFY_CLI_SKIP_UPDATE_CHECK: '1', - APIFY_CLI_SKIP_RENTAL_SUNSET_NOTICE: '1', - // Pin the file backend so e2e subprocesses don't share the host's OS keyring across tests. - APIFY_DISABLE_KEYRING: '1', - // The resolver prefers APIFY_TOKEN, so a token on the host would pick the account. - APIFY_TOKEN: '', + ...CliTestEnv, ...options.env, }, }); diff --git a/test/local/commands/init-non-interactive.test.ts b/test/local/commands/init-non-interactive.test.ts index fa58f7111..a90ede5c0 100644 --- a/test/local/commands/init-non-interactive.test.ts +++ b/test/local/commands/init-non-interactive.test.ts @@ -84,8 +84,21 @@ describe('apify init without a usable stdin', () => { expect(JSON.parse(readFileSync(joinCwd(LOCAL_CONFIG_PATH), 'utf8')).name).toBe('yes-default-name'); }); - it('--yes fails when the directory name is not a legal Actor name', async () => { - const joinCwd = await useFreshCwd('yes_bad_default_name'); + it('--yes sanitizes a directory name that is not a legal Actor name', async () => { + const joinCwd = await useFreshCwd('yes_sanitized.name'); + writeFileSync(joinCwd('package.json'), JSON.stringify({ name: 'pkg', version: '1.0.0' })); + + const lines = captureErrorsWithCap(); + + await testRunCommand(InitCommand, { flags_yes: true }); + + expect(errorLines(lines)).toStrictEqual([]); + expect(process.exitCode ?? 0).toBe(0); + expect(JSON.parse(readFileSync(joinCwd(LOCAL_CONFIG_PATH), 'utf8')).name).toBe('yes-sanitized-name'); + }); + + it('--yes fails when nothing is left of the directory name', async () => { + const joinCwd = await useFreshCwd('___'); writeFileSync(joinCwd('package.json'), JSON.stringify({ name: 'pkg', version: '1.0.0' })); const lines = captureErrorsWithCap(); From 607115e802fec318360d64339ddc36f15a0acb4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Thu, 8 Oct 2026 15:32:33 +0200 Subject: [PATCH 3/5] test(init): drop a local test that never reached the confirmation branch The local harness resolves the project type against the repo root, so the "directory confirmation" test was a duplicate of the missing-name one under a misleading name. The e2e test covers that path in a real subprocess, and now asserts the message rather than just a non-zero exit. Co-Authored-By: Claude Opus 5 --- src/commands/init.ts | 3 +-- test/e2e/__helpers__/run-cli-bounded.ts | 10 +++------- test/e2e/commands/init-non-tty.test.ts | 3 +-- .../local/commands/init-non-interactive.test.ts | 17 ----------------- 4 files changed, 5 insertions(+), 28 deletions(-) diff --git a/src/commands/init.ts b/src/commands/init.ts index c2dec88ab..0760f3b80 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -131,8 +131,7 @@ export class InitCommand extends ApifyCommand { if (existingName) { actorName = existingName; } else if (this.flags.yes) { - // Sanitizing covers the usual separators, but a name of only dashes or non-ASCII - // leaves nothing behind, and --yes has no prompt to fall back to. + // Sanitizing a name of only dashes or non-ASCII leaves nothing behind. try { validateActorName(defaultActorName); } catch { diff --git a/test/e2e/__helpers__/run-cli-bounded.ts b/test/e2e/__helpers__/run-cli-bounded.ts index 65a308119..3613d7c2b 100644 --- a/test/e2e/__helpers__/run-cli-bounded.ts +++ b/test/e2e/__helpers__/run-cli-bounded.ts @@ -6,7 +6,6 @@ import { CliTestEnv, DistActor, DistApify } from './run-cli.js'; export interface RunCliBoundedOptions { cwd: string; - env?: Record; /** Wall-clock budget. The child is killed once it elapses. */ timeoutMs?: number; /** Combined byte budget for stdout and stderr. The child is killed once it is passed. */ @@ -17,7 +16,6 @@ export interface RunCliBoundedResult { stdout: string; stderr: string; exitCode: number | null; - signal: NodeJS.Signals | null; /** The command was still running when `timeoutMs` elapsed. */ timedOut: boolean; /** The command wrote past `outputCapBytes`, which only a runaway loop does. */ @@ -48,7 +46,7 @@ export async function runCliBounded( args: string[], options: RunCliBoundedOptions, ): Promise { - const { cwd, env, timeoutMs = DefaultTimeoutMs, outputCapBytes = DefaultOutputCapBytes } = options; + const { cwd, timeoutMs = DefaultTimeoutMs, outputCapBytes = DefaultOutputCapBytes } = options; await mkdir(cwd, { recursive: true }); @@ -59,7 +57,6 @@ export async function runCliBounded( env: { ...process.env, ...CliTestEnv, - ...env, }, }); @@ -89,16 +86,15 @@ export async function runCliBounded( child.kill('SIGKILL'); }, timeoutMs); - const [exitCode, signal] = await new Promise<[number | null, NodeJS.Signals | null]>((resolve, reject) => { + const exitCode = await new Promise((resolve, reject) => { child.once('error', reject); - child.once('close', (code, sig) => resolve([code, sig])); + child.once('close', resolve); }).finally(() => clearTimeout(timer)); return { stdout: Buffer.concat(captured.stdout).toString('utf8'), stderr: Buffer.concat(captured.stderr).toString('utf8'), exitCode, - signal, timedOut, outputCapExceeded, bytesWritten, diff --git a/test/e2e/commands/init-non-tty.test.ts b/test/e2e/commands/init-non-tty.test.ts index dff7f2bf2..90693f36a 100644 --- a/test/e2e/commands/init-non-tty.test.ts +++ b/test/e2e/commands/init-non-tty.test.ts @@ -5,8 +5,6 @@ import path from 'node:path'; import { runCliBounded } from '../__helpers__/run-cli-bounded.js'; import { TestTmpRoot } from '../__helpers__/tmp.js'; -// The command itself finishes well under a second. A deadline only has to separate "exits" from -// "never exits"; a retry loop trips the output cap long before this. const EXIT_DEADLINE_MS = 15_000; const MissingNameError = 'Error: Actor name is required'; @@ -78,6 +76,7 @@ describe('[e2e] apify init with no usable stdin', () => { ); expect(result.timedOut, `stderr: ${result.stderr}`).toBe(false); expect(result.exitCode).not.toBe(0); + expect(result.stderr).toContain('Confirmation is required to continue'); await expect(access(path.join(cwd, '.actor', 'actor.json'))).rejects.toThrow(); }); diff --git a/test/local/commands/init-non-interactive.test.ts b/test/local/commands/init-non-interactive.test.ts index a90ede5c0..f6ddc217c 100644 --- a/test/local/commands/init-non-interactive.test.ts +++ b/test/local/commands/init-non-interactive.test.ts @@ -10,10 +10,6 @@ vitest.mock('ci-info', async (importOriginal) => { return { ...original, isCI: true }; }); -// The global `restoreMocks` would undo useTempPath's process.cwd spy after the first test, pointing -// every later test at the repo root instead of the temp directory. -vitest.setConfig({ restoreMocks: false }); - import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; import { LOCAL_CONFIG_PATH } from '../../../src/lib/consts.js'; import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; @@ -127,17 +123,4 @@ describe('apify init without a usable stdin', () => { expect(process.exitCode).not.toBe(0); expect(existsSync(joinCwd(LOCAL_CONFIG_PATH))).toBe(false); }); - - it('does not exit 0 when the directory confirmation cannot be answered', async () => { - const joinCwd = await useFreshCwd('no-yes-unknown-project'); - - const lines = captureErrorsWithCap(); - - await testRunCommand(InitCommand, {}); - - expect(errorLines(lines)).toHaveLength(1); - expect(process.exitCode).toBeDefined(); - expect(process.exitCode).not.toBe(0); - expect(existsSync(joinCwd(LOCAL_CONFIG_PATH))).toBe(false); - }); }); From f918384c07c8962c9a840eb5aabe13119329291f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Thu, 8 Oct 2026 15:46:34 +0200 Subject: [PATCH 4/5] fix(init): reject an Actor name the platform cannot accept The positional name was never validated, so `apify init 'Bad Name!!'` exited 0 and wrote a name that only failed later, on push. Both new non-interactive errors point the user at that argument, so it has to hold up. Also share the prompt validator with `apify create` and track the name source so the --yes failure quotes what it actually read. Co-Authored-By: Claude Opus 5 --- src/commands/init.ts | 35 +++++++++++-------- src/lib/create-utils.ts | 11 ++---- src/lib/utils.ts | 11 ++++++ test/e2e/__helpers__/run-cli-bounded.ts | 16 +++------ test/e2e/commands/init-non-tty.test.ts | 8 ++--- .../commands/init-non-interactive.test.ts | 10 +++--- test/local/commands/init.test.ts | 12 +++++++ 7 files changed, 58 insertions(+), 45 deletions(-) diff --git a/src/commands/init.ts b/src/commands/init.ts index 0760f3b80..d23718700 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -12,7 +12,13 @@ import { useYesNoConfirm } from '../lib/hooks/user-confirmations/useYesNoConfirm import { createPrefilledInputFileFromInputSchema } from '../lib/input_schema.js'; import { error, info, success, warning } from '../lib/outputs.js'; import { wrapScrapyProject } from '../lib/projects/scrapy/wrapScrapyProject.js'; -import { sanitizeActorName, setLocalConfig, setLocalEnv, validateActorName } from '../lib/utils.js'; +import { + sanitizeActorName, + setLocalConfig, + setLocalEnv, + validateActorName, + validateActorNameForPrompt, +} from '../lib/utils.js'; export class InitCommand extends ApifyCommand { static override name = 'init' as const; @@ -66,6 +72,12 @@ export class InitCommand extends ApifyCommand { let { actorName } = this.args; const cwd = process.cwd(); + // Both non-interactive failures point the user here, and an unusable name only surfaced on + // the next push. + if (actorName) { + validateActorName(actorName); + } + const projectResult = await useCwdProject(); // TODO: use direct .unwrap() once we migrate to yargs @@ -82,14 +94,15 @@ export class InitCommand extends ApifyCommand { } } - let defaultActorName = sanitizeActorName(basename(cwd)); + let actorNameSource = basename(cwd); if (project.type === ProjectLanguage.Python && project.entrypoint?.path) { const entryPath = project.entrypoint.path; // Extract the actual package name (last segment of dotted path) - const packageName = entryPath.includes('.') ? entryPath.split('.').pop()! : entryPath; - defaultActorName = sanitizeActorName(packageName); + actorNameSource = entryPath.includes('.') ? entryPath.split('.').pop()! : entryPath; } + const defaultActorName = sanitizeActorName(actorNameSource); + if (project.type === ProjectLanguage.Scrapy) { info({ message: 'The current directory looks like a Scrapy project. Using automatic project wrapping.' }); this.telemetryData.actorWrapper = 'scrapy'; @@ -135,7 +148,9 @@ export class InitCommand extends ApifyCommand { try { validateActorName(defaultActorName); } catch { - throw new Error(`Cannot derive an Actor name from '${basename(cwd)}'. Run 'apify init ' to set it.`); + throw new Error( + `Cannot derive an Actor name from '${actorNameSource}'. Run 'apify init ' to set it.`, + ); } actorName = defaultActorName; @@ -144,15 +159,7 @@ export class InitCommand extends ApifyCommand { actorName = await useUserInput({ message: 'Actor name:', default: defaultActorName, - validate: (value) => { - try { - validateActorName(value); - } catch (err) { - return (err as Error).message; - } - - return true; - }, + validate: validateActorNameForPrompt, errorMessageForStdin: "Actor name is required. Run 'apify init ', or pass --yes to use the current directory name.", }); diff --git a/src/lib/create-utils.ts b/src/lib/create-utils.ts index 867ec8dc9..a5d463532 100644 --- a/src/lib/create-utils.ts +++ b/src/lib/create-utils.ts @@ -20,7 +20,7 @@ import { } from './templates/consts.js'; import { getTemplateRecommendation } from './templates/getTemplateRecommendation.js'; import { buildTemplateChoiceList, NON_EXACT_SEPARATOR_LABEL } from './templates/templateChoices.js'; -import { validateActorName } from './utils.js'; +import { validateActorName, validateActorNameForPrompt } from './utils.js'; /** Filters coming from the `--use-case` / `--language` flags. `undefined` means "not provided". */ export interface TemplateFilters { @@ -165,14 +165,7 @@ async function executePrompts(manifest: Manifest, filters: TemplateFilters) { async function promptActorName() { const answer = await useUserInput({ message: 'Name of your new Actor:', - validate: (promptText) => { - try { - validateActorName(promptText); - } catch (err) { - return (err as Error).message; - } - return true; - }, + validate: validateActorNameForPrompt, }); return answer; diff --git a/src/lib/utils.ts b/src/lib/utils.ts index b1ce8e476..8d3656dc9 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -686,6 +686,17 @@ export const validateActorName = (actorName: string) => { } }; +/** Adapts {@link validateActorName} to the `validate` contract of the inquirer prompts. */ +export const validateActorNameForPrompt = (actorName: string) => { + try { + validateActorName(actorName); + } catch (err) { + return (err as Error).message; + } + + return true; +}; + export const sanitizeActorName = (actorName: string) => { let sanitizedName = actorName.replaceAll(/[^a-zA-Z0-9-]/g, '-'); diff --git a/test/e2e/__helpers__/run-cli-bounded.ts b/test/e2e/__helpers__/run-cli-bounded.ts index 3613d7c2b..4059020e3 100644 --- a/test/e2e/__helpers__/run-cli-bounded.ts +++ b/test/e2e/__helpers__/run-cli-bounded.ts @@ -30,16 +30,11 @@ const DefaultTimeoutMs = 15_000; const DefaultOutputCapBytes = 256 * 1024; /** - * Run a CLI command against the freshly built `./dist` (requires `pnpm run build`) with stdin - * pointed at `/dev/null` — the non-TTY, no-data case that CI runners, agent shells and - * `cmd < /dev/null` produce, and the one every prompting command must survive. + * Run a built CLI command (requires `pnpm run build`) with stdin pointed at `/dev/null`. * - * A hang and a retry loop both look like "the command never finished", so a deadline alone cannot - * tell them apart and costs the full deadline either way. The output cap separates them and trips - * in milliseconds: `outputCapExceeded` means the command retried something it can never succeed at. - * - * Use this for any command that prompts. `runCli` stays the right helper everywhere else: execa's - * own `maxBuffer` can cap output too, but it takes seconds to trip where this takes milliseconds. + * A deadline alone cannot tell a hang from a retry loop, and costs the full deadline either way. + * The output cap separates them in milliseconds: `outputCapExceeded` means the command retried + * something it can never succeed at. Use `runCli` for everything that does not prompt. */ export async function runCliBounded( binary: 'apify' | 'actor', @@ -68,8 +63,7 @@ export async function runCliBounded( const capture = (stream: 'stdout' | 'stderr') => (chunk: Buffer) => { bytesWritten += chunk.length; - // Keep the head only: past the cap the tail is the same line repeated, and holding it costs - // hundreds of megabytes before the kill lands. + // Keep the head only: buffering the tail costs hundreds of megabytes before the kill lands. if (!outputCapExceeded) captured[stream].push(chunk); if (bytesWritten > outputCapBytes && !outputCapExceeded) { diff --git a/test/e2e/commands/init-non-tty.test.ts b/test/e2e/commands/init-non-tty.test.ts index 90693f36a..82be7427f 100644 --- a/test/e2e/commands/init-non-tty.test.ts +++ b/test/e2e/commands/init-non-tty.test.ts @@ -5,8 +5,6 @@ import path from 'node:path'; import { runCliBounded } from '../__helpers__/run-cli-bounded.js'; import { TestTmpRoot } from '../__helpers__/tmp.js'; -const EXIT_DEADLINE_MS = 15_000; - const MissingNameError = 'Error: Actor name is required'; function countOccurrences(haystack: string, needle: string) { @@ -35,7 +33,7 @@ describe('[e2e] apify init with no usable stdin', () => { const cwd = path.join(root, 'yes-default-name'); await makeNodeProject(cwd); - const result = await runCliBounded('apify', ['init', '--yes'], { cwd, timeoutMs: EXIT_DEADLINE_MS }); + const result = await runCliBounded('apify', ['init', '--yes'], { cwd }); expect(result.outputCapExceeded, `the prompt retried instead of failing once (${result.bytesWritten} bytes)`).toBe( false, @@ -51,7 +49,7 @@ describe('[e2e] apify init with no usable stdin', () => { const cwd = path.join(root, 'no-yes-missing-name'); await makeNodeProject(cwd); - const result = await runCliBounded('apify', ['init'], { cwd, timeoutMs: EXIT_DEADLINE_MS }); + const result = await runCliBounded('apify', ['init'], { cwd }); expect(result.outputCapExceeded, `the prompt retried instead of failing once (${result.bytesWritten} bytes)`).toBe( false, @@ -69,7 +67,7 @@ describe('[e2e] apify init with no usable stdin', () => { const cwd = path.join(root, 'no-yes-unknown-project'); await mkdir(cwd, { recursive: true }); - const result = await runCliBounded('apify', ['init'], { cwd, timeoutMs: EXIT_DEADLINE_MS }); + const result = await runCliBounded('apify', ['init'], { cwd }); expect(result.outputCapExceeded, `the prompt retried instead of failing once (${result.bytesWritten} bytes)`).toBe( false, diff --git a/test/local/commands/init-non-interactive.test.ts b/test/local/commands/init-non-interactive.test.ts index f6ddc217c..0ff91e585 100644 --- a/test/local/commands/init-non-interactive.test.ts +++ b/test/local/commands/init-non-interactive.test.ts @@ -3,8 +3,7 @@ import { mkdir } from 'node:fs/promises'; import { join } from 'node:path'; import process from 'node:process'; -// `isCI` short-circuits every prompt in `stdinCheckWrapper` into a throw, which is what a non-TTY -// stdin does in the field. See src/lib/hooks/user-confirmations/_stdinCheckWrapper.ts. +// `isCI` short-circuits every prompt in `stdinCheckWrapper` into a throw, same as a non-TTY stdin. vitest.mock('ci-info', async (importOriginal) => { const original = await importOriginal(); return { ...original, isCI: true }; @@ -23,8 +22,7 @@ const { tmpPath, beforeAllCalls, afterAllCalls, forceNewCwd } = useTempPath('ini const { InitCommand } = await import('../../../src/commands/init.js'); -// A retry loop around a prompt that can never succeed writes until the process is killed. The cap -// turns that into a failed assertion in milliseconds instead of a test timeout after minutes. +// Turns a retry loop into a failed assertion in milliseconds instead of a test timeout. const MAX_ERROR_LINES = 20; function captureErrorsWithCap() { @@ -47,8 +45,8 @@ function errorLines(lines: string[]) { return lines.filter((line) => line.includes('Error:')); } -// `resetCwdCaches` resolves the hook modules outside the `node:process` mock graph, so it cannot -// clear the caches the command actually reads. A directory per test keeps the cache keys apart. +// `resetCwdCaches` resolves the hooks outside the `node:process` mock graph, so it cannot clear the +// caches the command reads. A directory per test keeps the cache keys apart. async function useFreshCwd(name: string) { forceNewCwd(name); await mkdir(join(tmpPath, name), { recursive: true }); diff --git a/test/local/commands/init.test.ts b/test/local/commands/init.test.ts index 782ef874e..40e1bf866 100644 --- a/test/local/commands/init.test.ts +++ b/test/local/commands/init.test.ts @@ -1,5 +1,6 @@ import { existsSync, readFileSync, writeFileSync } from 'node:fs'; import { mkdir } from 'node:fs/promises'; +import process from 'node:process'; import { KEY_VALUE_STORE_KEYS } from '@apify/consts'; @@ -24,9 +25,20 @@ describe('apify init', () => { }); afterEach(async () => { + process.exitCode = undefined; await afterAllCalls(); }); + it('rejects an Actor name that the platform cannot accept', async () => { + await testRunCommand(InitCommand, { + args_actorName: 'Bad Name!!', + flags_yes: true, + }); + + expect(process.exitCode).toBe(1); + expect(existsSync(joinPath(LOCAL_CONFIG_PATH))).toBeFalsy(); + }); + it('correctly creates basic structure with empty INPUT.json', async () => { await testRunCommand(InitCommand, { args_actorName: actName, From 702987fff89c302127b235f1fcc2e34340907860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Thu, 8 Oct 2026 16:03:19 +0200 Subject: [PATCH 5/5] test(init): assert the default name the e2e test claims to check Co-Authored-By: Claude Opus 5 --- src/commands/init.ts | 3 +-- test/e2e/__helpers__/run-cli-bounded.ts | 2 +- test/e2e/commands/init-non-tty.test.ts | 5 +++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/commands/init.ts b/src/commands/init.ts index d23718700..8b34d94b8 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -72,8 +72,7 @@ export class InitCommand extends ApifyCommand { let { actorName } = this.args; const cwd = process.cwd(); - // Both non-interactive failures point the user here, and an unusable name only surfaced on - // the next push. + // Nothing downstream validates an explicit name, so an unusable one only surfaced on the next push. if (actorName) { validateActorName(actorName); } diff --git a/test/e2e/__helpers__/run-cli-bounded.ts b/test/e2e/__helpers__/run-cli-bounded.ts index 4059020e3..238f11254 100644 --- a/test/e2e/__helpers__/run-cli-bounded.ts +++ b/test/e2e/__helpers__/run-cli-bounded.ts @@ -63,7 +63,7 @@ export async function runCliBounded( const capture = (stream: 'stdout' | 'stderr') => (chunk: Buffer) => { bytesWritten += chunk.length; - // Keep the head only: buffering the tail costs hundreds of megabytes before the kill lands. + // Keep the head only: the tail after the cap is unbounded and carries nothing the assertions read. if (!outputCapExceeded) captured[stream].push(chunk); if (bytesWritten > outputCapBytes && !outputCapExceeded) { diff --git a/test/e2e/commands/init-non-tty.test.ts b/test/e2e/commands/init-non-tty.test.ts index 82be7427f..8513182a8 100644 --- a/test/e2e/commands/init-non-tty.test.ts +++ b/test/e2e/commands/init-non-tty.test.ts @@ -1,5 +1,5 @@ import { randomBytes } from 'node:crypto'; -import { access, mkdir, rm, writeFile } from 'node:fs/promises'; +import { access, mkdir, readFile, rm, writeFile } from 'node:fs/promises'; import path from 'node:path'; import { runCliBounded } from '../__helpers__/run-cli-bounded.js'; @@ -42,7 +42,8 @@ describe('[e2e] apify init with no usable stdin', () => { expect(result.exitCode, `stderr: ${result.stderr}`).toBe(0); expect(result.stderr).toContain('The Actor has been initialized in the current directory.'); - await expect(access(path.join(cwd, '.actor', 'actor.json'))).resolves.toBeUndefined(); + const config = JSON.parse(await readFile(path.join(cwd, '.actor', 'actor.json'), 'utf8')); + expect(config.name).toBe('yes-default-name'); }); it('without --yes fails once instead of retrying the name prompt', async () => {