Skip to content

fix(e2e): restore backup directories file-by-file#3619

Merged
cv merged 2 commits into
NVIDIA:mainfrom
jyaunches:fix/backup-restore-dir-upload
May 15, 2026
Merged

fix(e2e): restore backup directories file-by-file#3619
cv merged 2 commits into
NVIDIA:mainfrom
jyaunches:fix/backup-restore-dir-upload

Conversation

@jyaunches

@jyaunches jyaunches commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • restore backup workspace directories by creating remote parents and uploading each file individually
  • preserve nested paths like memory/2026-04-20.md instead of relying on ambiguous openshell sandbox upload directory semantics
  • keep restore counts accurate for partially restored directories

Validation

  • bash -n scripts/backup-workspace.sh
  • npm test -- --run test/snapshot.test.ts
  • local mocked openshell restore verified nested files land under /sandbox/.openclaw/workspace/memory/...

Context

Addresses nightly state-backup-restore-e2e failure where backup captured memory/2026-04-20.md but restore dropped the nested directory.

Summary by CodeRabbit

  • Bug Fixes

    • Improved workspace restore reliability with per-file upload handling and clearer warnings when restores partially or fully fail.
  • Chores

    • Enhanced backup script diagnostics and tracking so restore progress and outcomes are reported more accurately.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9c4c0f81-d74a-4464-8dea-971ddc8214b0

📥 Commits

Reviewing files that changed from the base of the PR and between 5e02f84 and 51bb21c.

📒 Files selected for processing (1)
  • scripts/backup-workspace.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/backup-workspace.sh

📝 Walkthrough

Walkthrough

The PR refactors the workspace backup script's directory restoration to upload files individually rather than bulk-uploading directories. It adds shell_quote() for safe path quoting and restore_directory() to enumerate, create remote subdirectories, upload files, and set RESTORE_DIR_COUNT; do_restore() now uses that helper and reports per-directory outcomes.

Changes

Directory Restoration Refactor

Layer / File(s) Summary
Shell quoting and directory restore helper
scripts/backup-workspace.sh
shell_quote() escapes path characters; restore_directory(sandbox, src_dir, dir_name) enumerates files under src_dir, ensures quoted remote subdirectories via openshell, uploads each file individually, and updates RESTORE_DIR_COUNT to reflect successful uploads.
Restore flow integration
scripts/backup-workspace.sh
do_restore() now calls restore_directory() for each entry in DIRS, adds RESTORE_DIR_COUNT to the total count, and emits distinct warnings for skipped empty directories vs directories where one or more file uploads failed.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

Poem

🐰 I hop through scripts with careful quotes,
I count each file that safely floats,
Subfolders born where paths align,
One-by-one, restores refine,
A tiny hop, a tidy clone.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(e2e): restore backup directories file-by-file' directly and specifically summarizes the main change: refactoring directory restoration to process files individually instead of bulk upload.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/backup-workspace.sh`:
- Around line 58-59: The call to dirname when computing rel_parent can
misinterpret filenames beginning with - as options; update the invocation that
sets rel_parent to pass a double-dash separator so the filename is treated as an
operand (i.e. use dirname -- "$rel"). Modify the assignment that references
rel_parent/rel and ensure the unique symbols mentioned (rel_parent, rel,
dirname) are updated accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a5058eed-95e8-4166-894d-570e2fe3d597

📥 Commits

Reviewing files that changed from the base of the PR and between 0964a7e and 5e02f84.

📒 Files selected for processing (1)
  • scripts/backup-workspace.sh

Comment thread scripts/backup-workspace.sh Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 25936478345
Target ref: 51bb21cb150a5664e8997cb165417b11ad9babaa
Workflow ref: main
Requested jobs: state-backup-restore-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
state-backup-restore-e2e ✅ success

@cv cv merged commit 5d083f1 into NVIDIA:main May 15, 2026
27 checks passed
@miyoungc miyoungc mentioned this pull request May 16, 2026
12 tasks
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants