Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot commented Jul 19, 2025

Implement brev copy command for file transfer

Summary

This PR implements a new brev copy command that enables copying files between local machine and remote workspaces using scp, following the same patterns as the existing brev shell command.

Key features:

  • Bidirectional copying: brev copy workspace:/remote/path /local/path and brev copy /local/path workspace:/remote/path
  • Automatic workspace startup if stopped (same as shell command)
  • SSH connection handling with proper waiting and refresh logic
  • --host flag to copy to/from host machine instead of container
  • Aliases: cp, scp
  • Appears in SSH Commands section alongside shell, open, etc.

Implementation approach:

  • Created new pkg/cmd/copy/copy.go following shell.go patterns
  • Reuses workspace resolution, SSH setup, and polling utilities
  • Uses strings.Split for workspace:path parsing (similar to portforward command)
  • Refactored into smaller functions to pass linting (function length limits)

Review & Testing Checklist for Human

Critical items requiring manual verification:

  • End-to-end file transfer testing: Test actual copying in both directions with real workspaces - this is the core functionality that couldn't be tested locally
  • Workspace startup logic: Verify stopped workspaces start correctly before copying (test with brev stop then brev copy)
  • Error handling verification: Test invalid workspace names, non-existent paths, SSH failures, permission errors
  • Argument parsing edge cases: Test paths containing colons, special characters, spaces (e.g. workspace:/path:with:colons, paths with spaces)
  • Host flag functionality: Verify --host flag correctly targets host vs container

Recommended test plan:

  1. Create test files locally and in a workspace
  2. Test upload: brev copy /local/test.txt myworkspace:/tmp/test.txt
  3. Test download: brev copy myworkspace:/tmp/test.txt /local/downloaded.txt
  4. Test with stopped workspace to verify auto-start
  5. Test --host flag behavior
  6. Test error scenarios (invalid paths, non-existent workspace)

Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    main.go --> cmd.go["pkg/cmd/cmd.go<br/>(Minor Edit)"]
    cmd.go --> copy.go["pkg/cmd/copy/copy.go<br/>(Major Edit - New File)"]
    
    copy.go --> util.go["pkg/cmd/util/util.go<br/>(Context)"]
    copy.go --> shell.go["pkg/cmd/shell/shell.go<br/>(Context)"]
    copy.go --> refresh.go["pkg/cmd/refresh/refresh.go<br/>(Context)"]
    
    copy.go -.->|"reuses patterns"| shell.go
    copy.go -.->|"workspace resolution"| util.go
    copy.go -.->|"SSH refresh logic"| refresh.go
    
    cmd.go -->|"registers command"| copy.go
    
    subgraph Legend
        L1["Major Edit"]:::major-edit
        L2["Minor Edit"]:::minor-edit  
        L3["Context/No Edit"]:::context
    end
    
    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB
    classDef context fill:#FFFFFF
Loading

Notes

  • Security consideration: Uses exec.Command with user input but sshAlias is validated through workspace resolution
  • Pattern consistency: Closely follows shell.go patterns for workspace handling, SSH setup, and error handling
  • Linting compliance: Refactored into smaller functions to meet function length requirements
  • Future enhancement: User mentioned considering rsync for v2 - current scp implementation provides foundation

Link to Devin run: https://app.devin.ai/sessions/e7fbaf33368343fc9d32002721a55ea0
Requested by: Alec Fong (@theFong)

…e workspaces

- Add new copy command in pkg/cmd/copy/copy.go following shell command patterns
- Support bidirectional copying: local->remote and remote->local
- Use scp for file transfer with workspace:path syntax parsing
- Include workspace startup logic and SSH connection handling
- Register command in SSH Commands section with aliases cp, scp
- Add --host flag for copying to/from host machine vs container

Co-Authored-By: Alec Fong <alecsanf@usc.edu>
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

devin-ai-integration bot and others added 3 commits July 19, 2025 18:46
- Change copyLong description to use 'remote instance'
- Update copyExample to use 'instance_name' in examples
- Update error messages to reference 'instance path' format
- Maintain consistency with Brev CLI terminology

Co-Authored-By: Alec Fong <alecsanf@usc.edu>
- Validate local file exists before attempting workspace connection
- Provide clear error message when file doesn't exist
- Improves UX by failing fast on missing files
- Only validates for upload operations (local-to-remote)

Co-Authored-By: Alec Fong <alecsanf@usc.edu>
- Track transfer duration using time.Now() and time.Since()
- Display success message showing source → destination with timing
- Use terminal.Green() for colored success output following CLI patterns
- Updated runSCP function signature to accept terminal parameter
- Improves UX by confirming successful transfers with clear feedback

Co-Authored-By: Alec Fong <alecsanf@usc.edu>
@theFong
Copy link
Member

theFong commented Jul 19, 2025

the success message looks like the following

⣽ waiting for instance to be ready... ✓ Successfully copied %!!(string=go.mod)s(MISSING) → %!!(string=test-deploy:~)s(MISSING) (%!!(time.Duration=1695000000)v(MISSING))

we need to put it on a new line and use the proper func to print.

devin-ai-integration bot and others added 2 commits July 19, 2025 19:03
- Use t.Vprint with fmt.Sprintf instead of t.Vprintf to fix printf formatting errors
- Ensures proper formatting of source, destination, and duration values
- Addresses GitHub comment feedback about MISSING values in output

Co-Authored-By: Alec Fong <alecsanf@usc.edu>
- Add s.Stop() call at end of pollUntil function
- Add s.Stop() call in error path to ensure spinner is always stopped
- Ensures success message appears on new line instead of same line as spinner
- Addresses GitHub comment feedback about message formatting

Co-Authored-By: Alec Fong <alecsanf@usc.edu>
@theFong
Copy link
Member

theFong commented Jul 19, 2025

what I meant by the new line is that I want to format like

⣻ waiting for SSH connection to be available 
✓ Successfully copied go.mod → test-deploy:~ (1.668s)

- Add fmt.Print("\n") before success message to ensure clean line separation
- Success message now appears on completely separate line after spinner stops
- Addresses GitHub comment feedback requesting proper formatting:
  spinner line -> newline -> success message
- Follows pattern used in other commands like ollama

Co-Authored-By: Alec Fong <alecsanf@usc.edu>
@theFong
Copy link
Member

theFong commented Jul 19, 2025

Output: Warning: Permanently added 'ec2-54-91-119-233.compute-1.amazonaws.com' (ED25519) to the list of known hosts.
scp: local "my-dir" is not a regular file
scp: failed to upload file my-dir to ~

can we support directories automatically?

- Detect directories using os.Stat() and IsDir()
- Add -r flag to scp when copying directories for uploads
- Always use -r flag for downloads to handle both files and directories
- Update validation messages to mention 'file or directory'
- Update help text and examples to include directory copying
- Addresses GitHub comment requesting directory support

Resolves scp error: 'local "my-dir" is not a regular file'
by automatically detecting directories and using recursive flag

Co-Authored-By: Alec Fong <alecsanf@usc.edu>
@theFong theFong merged commit b603505 into main Jul 19, 2025
8 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