Skip to content

fix(cli): preserve directory basename for filtered uploads#1028

Merged
johntmyers merged 1 commit intomainfrom
fix/sandbox-upload-preserve-directory
Apr 29, 2026
Merged

fix(cli): preserve directory basename for filtered uploads#1028
johntmyers merged 1 commit intomainfrom
fix/sandbox-upload-preserve-directory

Conversation

@johntmyers
Copy link
Copy Markdown
Collaborator

Summary

Make named directory uploads preserve the source directory basename consistently, including the default git-filtered upload path. .gitignore filtering now only decides which files are included; it no longer changes where those files land in the sandbox.

Related Issue

Fixes #885

Changes

  • Reused one archive writer for single-path and git-filtered upload archives.
  • Added an optional archive prefix for git-filtered file lists so named directories land under <dest>/<basename>/....
  • Kept flat archive layout for upload . and file uploads.
  • Documented that filtering affects inclusion, not destination layout.

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)
  • Architecture docs updated (if applicable)

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>
@johntmyers johntmyers requested a review from a team as a code owner April 28, 2026 23:23
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Apr 28, 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.

@mjamiv
Copy link
Copy Markdown
Contributor

mjamiv commented Apr 29, 2026

Sorry for the noise on #976. My follow-up commit for the repo-backed/git-filtered path landed on my flag branch, not the PR branch John was reviewing. Thanks for carrying the #885 fix forward here.

I fetched this PR locally at cc51dff and ran the focused upload coverage:

  • cargo test -p openshell-cli archive — 4 passed
  • cargo test -p openshell-cli directory_upload_prefix — 2 passed

The behavior here matches the #885 production footgun we hit: named directory uploads preserve <dest>/<basename>/..., while upload . / basename-less paths keep the legacy flat behavior. The current Rust CI failure looks unrelated to this diff; it is in openshell-sandbox process identity ambiguity handling.

@johntmyers johntmyers merged commit 20ffc72 into main Apr 29, 2026
22 of 26 checks passed
@johntmyers johntmyers deleted the fix/sandbox-upload-preserve-directory branch April 29, 2026 01:42
drew added a commit that referenced this pull request Apr 29, 2026
PR #1028 (preserve directory basename for filtered uploads) changed
the on-disk layout so that filtered directory uploads land under
`<dest>/<basename>/...` rather than `<dest>/...`, matching the
unfiltered upload semantics.

The accompanying e2e test was not updated and now fails because it
still expects `tracked.txt` directly under the download root. Update
the assertions to look under the preserved `repo/` basename.
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.

bug: openshell sandbox upload <local-dir> <remote-dir> flattens contents instead of creating named subdirectory

3 participants