Skip to content

fix(cli): undo complete timeline batch receipts atomically - #5279

Merged
miguel-heygen merged 3 commits into
heygen-com:mainfrom
user-github-me:fix/timeline-undo-batch-receipts
Oct 9, 2026
Merged

miguel-heygen merged 3 commits into
heygen-com:mainfrom
user-github-me:fix/timeline-undo-batch-receipts

Conversation

@user-github-me

Copy link
Copy Markdown
Contributor

timeline apply --json and timeline ids --json return receipt arrays, but timeline undo only accepts a single receipt object. Passing an entire saved response fails, and the array returned by undo cannot be used to redo the operation.

Accept single receipts, receipt arrays, and complete command responses, either inline or from a JSON file. Validate the whole batch and prepare every restore before one call to the existing atomic mutation helper. A stale file refuses the entire batch; successful undo returns receipts that can redo it. Existing single-receipt output stays compatible, and empty batches are no-ops.

Validation:

  • Six new regressions fail on main; all 87 timeline tests and seven atomic-mutation tests pass with the fix.
  • Built CLI under Node 22: undo a two-file batch from its saved response, redo using the whole undo response, accept an unchanged batch, and preserve both files when the last one is stale.
  • CLI build/typecheck, root lint, formatting, test reachability, comment checks, and all pre-commit checks pass.

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

Thanks. apply and ids return receipt arrays that undo could not accept, so multi-file edits had no undo path. Validating the whole batch and restoring it through one atomic mutation call is the right shape, and a stale file now refuses the entire batch instead of throwing. The single-receipt output is unchanged. This conflicts with #5240 in the timeline e2e test file, so one of the two will need a rebase.

— Rames

@jrusso1020
jrusso1020 enabled auto-merge October 8, 2026 20:05
@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 8, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 8, 2026
@miguel-heygen

Copy link
Copy Markdown
Collaborator

Thanks for this fix, it is approved. main has moved since, and packages/cli/src/timeline/timeline.e2e.test.ts now conflicts with #5240. Could you merge the latest main into this branch? Once CI is green on the new head we will take it through the merge queue.

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

Approve at 7062933e. The merge from main didn't change the fix. A batch undo still restores every file or none, and required CI is green.

What changed since my approval at 1e9512fd

  • a2Commands.ts: the PR's patch is line-for-line the same as before the merge.
  • aecb135e (test only): it carries main's #5240 project-inspection tests and its -- separator test into this file. That resolves the conflict I flagged.
  • The merge (7062933e): the PR's diff against main is still just a2Commands.ts and timeline.e2e.test.ts, and it removes none of main's lines. #5239 and #5240 are both kept.

All-or-nothing, checked against the CLI at this head

  • One stale file: I applied a two-file batch, edited the second file, then ran undo on the whole receipt. It exits 2 with "file changed since the timeline was read", and both files are byte-identical before and after.
  • Missing backup: with the second receipt's backup removed, undo fails while still reading inputs, before any write, and both files are unchanged.
  • Why it holds: applyFileMutations checks every file's version before writing any file, and rolls back already-written files if a later write throws.
  • Mutants:
    • Undoing receipts one at a time leaves the first file reverted on the stale case. The test that keeps the whole batch untouched when a later receipt is stale catches it.
    • Validating only the first receipt is caught by the invalid-receipt case.

Nits (non-blocking)

  • A missing backup or target file still surfaces as a raw ENOENT with exit 1, not a JSON refusal. That was already true for single receipts, and nothing is written.
  • Undoing an empty receipt ([]) reports ok: true with no files. That's tested and intended.

Tests: src/timeline/ 103/103. oxfmt --check and oxlint are clean on both files; CI's lint+format preflight was path-skipped on this run. Every required check passes.

— Rames

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 9, 2026
Merged via the queue into heygen-com:main with commit d03e41c Oct 9, 2026
80 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.

3 participants