Skip to content

fix(cli): fall back to regular upload when git filtering excludes all files#1783

Open
russellb wants to merge 2 commits into
NVIDIA:mainfrom
russellb:fix/upload-empty-gitignore-fallback
Open

fix(cli): fall back to regular upload when git filtering excludes all files#1783
russellb wants to merge 2 commits into
NVIDIA:mainfrom
russellb:fix/upload-empty-gitignore-fallback

Conversation

@russellb
Copy link
Copy Markdown
Contributor

@russellb russellb commented Jun 5, 2026

Summary

When uploading a directory whose contents are entirely excluded by .gitignore, git_sync_files returns an empty file list. The git-aware upload path silently accepted this and printed "Upload complete" without transferring any files. Now both call sites check for an empty file list and fall through to the regular, unfiltered upload path instead.

Related Issue

Closes #1778

Changes

  • main.rs: Added !files.is_empty() guard to the git-aware upload branch so an empty git file list falls through to the regular upload
  • run.rs: Added the same guard in sandbox_upload_plan() so the sandbox run upload path also falls back correctly
  • Added test sandbox_upload_plan_falls_back_when_all_files_gitignored confirming a gitignored directory produces a Regular upload plan

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

… files

When uploading a directory whose contents are entirely excluded by
.gitignore, git_sync_files returns an empty file list. The git-aware
upload path silently accepted this and printed "Upload complete" without
transferring any files. Now both call sites (direct CLI handler and
sandbox_upload_plan) check for an empty file list and fall through to the
regular, unfiltered upload path instead.

Closes NVIDIA#1778

Signed-off-by: Russell Bryant <rbryant@nvidia.com>
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Jun 5, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

maxamillion
maxamillion previously approved these changes Jun 5, 2026
…#1778)

Reproduces the reported workflow: download a file from a sandbox into a
local git repo where the target directory is gitignored, then re-upload
that directory without --no-git-ignore. Before the fix the upload
silently succeeded with zero files transferred.

Signed-off-by: Russell Bryant <rbryant@nvidia.com>
@TaylorMutch
Copy link
Copy Markdown
Collaborator

/ok to test 6d322fd

@johntmyers johntmyers added the gator:in-review Gator is reviewing or awaiting PR review feedback label Jun 5, 2026
@johntmyers
Copy link
Copy Markdown
Collaborator

johntmyers commented Jun 5, 2026

gator-agent

PR Review Status

Validation: This PR is project-valid because it directly fixes validated issue #1778 for sandbox upload silently doing a zero-file transfer when git filtering excludes every file.
Head SHA: 6d322fdbffc4aeb3e8b1df5061456f301679d67b

Independent review: completed in this cycle. No critical correctness blockers were found in the core fallback or regression coverage.

Review feedback still needs author or maintainer resolution before pipeline handoff:

  • The fallback from git-filtered upload to unfiltered upload is silent. Because CLI help says .gitignore filtering applies by default, this can surprise users and may upload ignored files they expected to stay local. Please either:
    • print a short stderr notice when git filtering returns an empty list and OpenShell falls back to unfiltered upload, including the sandbox create --upload path; or
    • add a maintainer-confirmed rationale that silent fallback is intentional.
  • Docs: this is a direct CLI/sandbox upload behavior change. Please add a short note under docs/sandboxes/manage-sandboxes.mdx explaining default gitignore filtering and this empty-filter fallback, or have a maintainer state that docs are intentionally unnecessary.

E2E: This change affects sandbox file transfer behavior and adds an E2E regression test, so test:e2e should be applied once review feedback is resolved.

Next state: gator:in-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gator:in-review Gator is reviewing or awaiting PR review feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An issue on "openshell sandbox upload/download"

4 participants