Skip to content

fix(cli): validate init flags before creating the project directory#1207

Merged
vanceingalls merged 1 commit into
heygen-com:mainfrom
calcarazgre646:fix/init-validate-before-mkdir
Jun 17, 2026
Merged

fix(cli): validate init flags before creating the project directory#1207
vanceingalls merged 1 commit into
heygen-com:mainfrom
calcarazgre646:fix/init-validate-before-mkdir

Conversation

@calcarazgre646

Copy link
Copy Markdown
Contributor

Problem

In non-interactive mode, hyperframes init creates the project directory before validating the source-file flags. Three failure paths exit after the mkdirSync:

  • --video and --audio passed together
  • --video <path> where the file does not exist
  • --audio <path> where the file does not exist

Each one leaves an empty orphan directory behind in the caller's CWD. For CI and agent loops (the audience of --non-interactive) a failed scaffold that still mutates the filesystem is surprising: a retry with a corrected flag passes the "directory already exists and is not empty" check only because the orphan happens to be empty, and an aborted run pollutes the workspace.

The other early validations already get this right: --template, -V, and --resolution all reject before any filesystem write, and their tests assert existsSync(target) === false. The interactive path also validates the source file before its mkdirSync. The non-interactive video/audio path was the only one ordered the other way.

Fix

Reorder the non-interactive block: flag-conflict check and source-file existence checks first, mkdirSync(destDir) after. The existence checks reuse the already-resolved paths, so the video/audio handling below switches from re-resolving videoFlag/audioFlag to using videoPath/audioPath directly. No messages or exit codes changed.

Not touched: the interactive path resolves --video/--audio with else-if precedence (video wins) rather than rejecting the combination. That asymmetry predates this change and is a UX question rather than a bug, so it stays as is.

Tests

  • Strengthened the existing -v missing.mp4 case with the existsSync(target) === false assertion the sibling tests already use (it would have failed before this fix).
  • New: --audio missing.mp3 exits 1 with no directory created.
  • New: --video + --audio together exits 1 with no directory created.

All three fail on main and pass with the reorder. Full cli suite: 690/690.

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small case, but instructive in its discipline. Read at head 4b2762e1, base main, single author hand throughout — and a clean parallel-PR field (#1206, #1208, #1404 each branched independently off main from the same author; no Graphite stack, no stale-base hazard to map).

What the diff actually does

packages/cli/src/commands/init.ts:686-715 reorders the non-interactive block so the three pre-write checks — --video/--audio flag conflict, --video <path> existence, --audio <path> existence — fire before the mkdirSync(destDir, { recursive: true }) at line 715. The pre-existing duplicate-existence checks inside the per-flag branches at init.ts:721-737 are correctly retired in favour of the now-resolved videoPath / audioPath locals, so handleVideoFile(videoPath, destDir, false) at line 724 and copyFileSync(audioPath, ...) at line 735 consume the same already-validated values. No dispatch-chain orphan, no resurrected re-resolve, no silent change of error messages or exit codes.

Why the framing convinces

The PR body's stated invariant — every other early validation already rejects before mkdirSync — survives a walk of the file:

  • --template / -V / --resolution all process.exit(1) at init.ts:64, :125, :139 and :671-680, well above the non-interactive destDir resolution.
  • The existsSync(destDir) && readdirSync(destDir).length > 0 not-empty guard at init.ts:691-694 is correctly the first check inside the non-interactive block and stays first.
  • The interactive twin at init.ts:862-884 resolves and existsSync-checks both videoFlag and audioFlag before its mkdirSync(destDir) calls — the PR body's parallel claim verified. (The videoFlag vs audioFlag precedence asymmetry there — if … else if … rather than a hard conflict — is deliberately scoped out in the body and is, I agree, a UX question rather than a defect of this fix.)

Tests earn their keep

init.test.ts:147 strengthens the pre-existing -v missing.mp4 case with the existsSync(target) === false assertion the sibling tests already use; without the reorder, that assertion fails. Two new cases — --audio missing.mp3 and --video + --audio together — cover the remaining two failure paths the body enumerates, each ending with the same existsSync(target) === false invariant. Three pass with the fix, three would fail without it; the test suite locks the door against future regressions of this exact category. Full CLI suite reported green at 690/690, and every CI check on the rollup is SUCCESS.

Scope honesty, and what is deliberately not fixed

I noticed two further process.exit(1) paths downstream of the mkdirSync that also leave an orphan directory: the scaffold-failure case at init.ts:778 (template fetch / copy failure) and an implicit one if handleVideoFile at :724 throws on probe/transcode. Both are different failure families from "flag validation in non-interactive mode," and the PR body's enumeration of "three failure paths" is correct for the category it claims to fix. Bundling them in would either widen the diff into a structural cleanup (e.g. a try/catch around the whole non-interactive scaffold with cleanup of destDir on failure) or quietly conflate two bug shapes. Leaving them as separate work, with a clear in-PR scope statement, is the right call. Worth mentioning in case the author wants to file a sibling ticket — but not a blocker on this one.

Verdict

Clean — ready for stamp at 4b2762e1.

The reorder is minimal, the dispatch chain reads correctly under the new shape, no defensive-guard downgrades or contradictory rules were introduced, and the tests pin the precise invariant the body claims. Re-verify if HEAD moves before stamp.

Review by Via

@vanceingalls vanceingalls merged commit a4ae3c9 into heygen-com:main Jun 17, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants