Skip to content

fix(cli): refuse undo when a target or backup is missing - #5365

Merged
jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/timeline-undo-missing-files
Oct 24, 2026
Merged

jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/timeline-undo-missing-files

Conversation

@user-github-me

@user-github-me user-github-me commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

If a timeline undo receipt refers to a removed target or backup, the command exits 1 with a raw filesystem error instead of its normal refusal payload. A missing index.html also trips project discovery before the receipt is checked. This follows the nonblocking review feedback in #5279.

Catch missing-file errors while preparing the entire undo batch and return exit 2 with {ok: false, reason, fix} on stderr in JSON mode. Include the missing project-relative path and a recovery hint. Resolve undo projects without requiring index.html so a missing root target receives the same refusal. Preparation still completes before mutations begin.

Validation: six new single/batch cases fail on unchanged main, then all 109 timeline tests and seven atomic-write tests pass. Eight controls through the rebuilt CLI on Node 22 verify JSON and text refusals, unchanged source and backup bytes, and successful batch undo/redo. Workspace and CLI builds, CLI typecheck, repository lint, formatting, test reachability, comment checks and normal pre-commit hooks pass. The overlapping test patch in #5315 still applies cleanly.

Related: #5279.

@jrusso1020 jrusso1020 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.

Verified against the full runUndo and applyFileMutations. Moving the readFileSync calls for target and backup inside the try means any missing file in the batch fails before applyFileMutations writes anything, so the refusal really does leave sources and backups untouched, and the e2e cases assert those exact bytes. An ENOENT raised later inside applyFileMutations (a race on statSync during the atomic replace) is also mapped to the refusal, but that path rolls back the earlier writes first, and a failed rollback surfaces as an AggregateError without code, which still rethrows. So the 'nothing changed' reading holds. requireIndex: false is safe here because runUndo never reads project.indexPath. The PR merges cleanly over #5867 on main.

— Rames

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
@jrusso1020
jrusso1020 removed this pull request from the merge queue due to a manual request Oct 24, 2026
@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
Merged via the queue into heygen-com:main with commit 9ab4de4 Oct 24, 2026
211 of 213 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