Skip to content

feat: add zeabur file pull command#232

Merged
leechenghsiu merged 6 commits intomainfrom
matthewlee/des-536-cli-file-list-read
May 4, 2026
Merged

feat: add zeabur file pull command#232
leechenghsiu merged 6 commits intomainfrom
matthewlee/des-536-cli-file-list-read

Conversation

@leechenghsiu
Copy link
Copy Markdown
Contributor

@leechenghsiu leechenghsiu commented Apr 30, 2026

Summary

  • Add zeabur file list <upload_id> [path] to list files in an upload
  • Add zeabur file read <upload_id> <path> to read file content
  • New FileAPI interface in pkg/api/ with GraphQL queries (files() / fileContent())

This enables the skilled agent to access user-uploaded project files via CLI (bash + skill), instead of requiring SDK tools.

Test plan

  • zeabur file list <upload_id> — lists root directory
  • zeabur file list <upload_id> src/ — lists subdirectory
  • zeabur file read <upload_id> package.json — reads file content
  • --json flag outputs JSON format
  • Tested with real upload IDs against production API

Closes DES-536

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features
    • New file command group for managing uploaded files
    • Introduced file pull subcommand to download uploaded files to your local system
    • Configure custom target directories for downloaded files
    • Preserves remote directory structure during downloads

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This pull request introduces a new CLI command structure for managing uploaded files. It adds a top-level file command with a pull subcommand that downloads project uploads to a local directory, along with supporting API client methods and interface definitions.

Changes

Cohort / File(s) Summary
CLI Command Structure
internal/cmd/file/file.go, internal/cmd/file/pull/pull.go, internal/cmd/root/root.go
Introduces file top-level command and pull subcommand. The pull command accepts uploadID and optional targetDir arguments, prompts for missing upload ID in interactive mode, invokes the API client to download files, and reports results.
API Client Implementation
pkg/api/file.go, pkg/api/interface.go
Adds ListUploadFiles, ReadUploadFile, and PullUploadFiles methods to query and recursively download uploaded files. Includes binary file detection and directory traversal logic. New FileAPI interface embeds into the main Client interface.
Configuration
.gitignore
Excludes .env.local from version control.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI as CLI (pull cmd)
    participant Factory
    participant APIClient
    participant Server

    User->>CLI: Execute: file pull <uploadID> [targetDir]
    CLI->>CLI: Validate/prompt for uploadID
    CLI->>APIClient: PullUploadFiles(ctx, uploadID, targetDir)
    APIClient->>APIClient: Call pullDir recursively
    loop For each file/directory entry
        APIClient->>Server: ListUploadFiles (via GraphQL)
        Server-->>APIClient: File paths
        alt Is directory (path ends with /)
            APIClient->>APIClient: Create local directory
            APIClient->>APIClient: Recurse into subdirectory
        else Is regular file
            alt Not binary extension
                APIClient->>Server: ReadUploadFile (via GraphQL)
                Server-->>APIClient: File content
                APIClient->>APIClient: Write to disk (os.WriteFile)
            else Binary file
                APIClient->>APIClient: Skip binary file
            end
        end
    end
    APIClient-->>CLI: Return file count
    CLI->>User: Report: Pulled N files to targetDir
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% 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.
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.
Title check ✅ Passed The title 'feat: add zeabur file pull command' accurately describes the main change: introducing a new file pull subcommand for downloading uploaded files.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch matthewlee/des-536-cli-file-list-read

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/cmd/file/list/list.go`:
- Around line 27-29: The code only sets opts.path when len(args) > 1 which lets
a previous value persist across runs; update the argument-parsing in list.go so
that when the optional path arg is omitted you explicitly reset opts.path (e.g.,
set opts.path = "" or nil) instead of leaving it untouched—locate the block
around the len(args) > 1 check that assigns opts.path and add an else branch to
clear it (or initialize opts.path to the empty value before parsing) to avoid
leaking previous values between invocations.

In `@internal/cmd/file/read/read.go`:
- Around line 22-27: The command currently forces two args via
cobra.ExactArgs(2) which prevents interactive prompting; change the cobra args
validator to allow 0–2 args (e.g., cobra.RangeArgs(0,2) or
cobra.MaximumNArgs(2)) and, inside the RunE closure where opts.uploadID and
opts.path are set before calling runRead, add logic to populate missing values
by prompting in interactive mode (but skip prompts when the corresponding
flag/option is already provided). Update the RunE flow to: read args if present
into opts.uploadID and opts.path, detect missing fields and invoke the existing
interactive prompt helpers (or prompt here) only when stdin is interactive, and
then call runRead(f, opts).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 83be28df-a97c-444d-be7b-7766c76bcfb1

📥 Commits

Reviewing files that changed from the base of the PR and between bcb65be and f6f3f30.

📒 Files selected for processing (6)
  • internal/cmd/file/file.go
  • internal/cmd/file/list/list.go
  • internal/cmd/file/read/read.go
  • internal/cmd/root/root.go
  • pkg/api/file.go
  • pkg/api/interface.go

Comment thread internal/cmd/file/list/list.go Outdated
Comment thread internal/cmd/file/read/read.go Outdated
Allow reading uploaded project files via CLI, enabling the skilled agent
to access user uploads through bash + skill instead of SDK tools.

- `zeabur file list <upload_id> [path]` — list files in an upload
- `zeabur file read <upload_id> <path>` — read file content

DES-536

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@leechenghsiu leechenghsiu force-pushed the matthewlee/des-536-cli-file-list-read branch from 7a016cd to 8237de0 Compare April 30, 2026 08:21
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
internal/cmd/file/list/list.go (1)

26-33: ⚡ Quick win

Propagate command context instead of using context.Background().

Use cmd.Context() so cancellations/deadlines flow into API calls.

♻️ Proposed refactor
-func runList(f *cmdutil.Factory, opts *Options) error {
+func runList(ctx context.Context, f *cmdutil.Factory, opts *Options) error {
 	var pathPtr *string
 	if opts.path != "" {
 		pathPtr = &opts.path
 	}

-	files, err := f.ApiClient.ListUploadFiles(context.Background(), opts.uploadID, pathPtr)
+	files, err := f.ApiClient.ListUploadFiles(ctx, opts.uploadID, pathPtr)
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts.uploadID = args[0]
 			opts.path = ""
 			if len(args) > 1 {
 				opts.path = args[1]
 			}
-			return runList(f, opts)
+			return runList(cmd.Context(), f, opts)
 		},

Also applies to: 39-46

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/cmd/file/list/list.go` around lines 26 - 33, The RunE handlers
currently call runList (and the similar handler in the following RunE block)
using context.Background(), which prevents cancellations/deadlines from
propagating; update the RunE func(cmd *cobra.Command, args []string) error
blocks to pass cmd.Context() into runList (and any other API-call functions
invoked there) instead of context.Background() so command cancellation/deadlines
flow through; look for the RunE closure and calls to runList (and the second
RunE block at lines 39-46) and replace context.Background() with cmd.Context().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/cmd/file/list/list.go`:
- Around line 22-26: The command currently enforces cobra.RangeArgs(1, 2) and
lacks an interactive fallback for the upload-id; change the argument validation
to allow 0–2 args (e.g., cobra.RangeArgs(0, 2)) and update the RunE handler to
prompt the user for upload-id when it is not supplied positionally and the
upload-id flag (if present) is empty — but skip prompting if a flag value is
provided. Specifically, modify the Args check and inside RunE (the anonymous
func) detect missing args[0], check for an upload-id flag value, and invoke an
interactive prompt to obtain upload-id before proceeding with the existing logic
that uses upload-id and optional path.

---

Nitpick comments:
In `@internal/cmd/file/list/list.go`:
- Around line 26-33: The RunE handlers currently call runList (and the similar
handler in the following RunE block) using context.Background(), which prevents
cancellations/deadlines from propagating; update the RunE func(cmd
*cobra.Command, args []string) error blocks to pass cmd.Context() into runList
(and any other API-call functions invoked there) instead of context.Background()
so command cancellation/deadlines flow through; look for the RunE closure and
calls to runList (and the second RunE block at lines 39-46) and replace
context.Background() with cmd.Context().
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d5454b33-e66e-4861-b8b2-0541d91844d3

📥 Commits

Reviewing files that changed from the base of the PR and between f6f3f30 and 8237de0.

📒 Files selected for processing (7)
  • .gitignore
  • internal/cmd/file/file.go
  • internal/cmd/file/list/list.go
  • internal/cmd/file/read/read.go
  • internal/cmd/root/root.go
  • pkg/api/file.go
  • pkg/api/interface.go
✅ Files skipped from review due to trivial changes (3)
  • .gitignore
  • internal/cmd/root/root.go
  • pkg/api/file.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • internal/cmd/file/file.go
  • internal/cmd/file/read/read.go

Comment thread internal/cmd/file/list/list.go Outdated
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/cmd/file/read/read.go`:
- Around line 18-31: The Options instance is created once (opts := &Options{})
and mutated in the command's RunE, which allows state to leak across multiple
executions; to fix this, move the allocation/reset of Options into the RunE
handler so each invocation uses a fresh value (e.g., create a new opts :=
&Options{} at start of the RunE function or explicitly reset opts.uploadID and
opts.path before reading args), update the code that reads args into
opts.uploadID and opts.path accordingly, and keep the call to runRead(f, opts)
unchanged so runRead receives an isolated Options for each run.
- Around line 24-32: The RunE closure currently calls runRead(f, opts) without
the Cobra command context and runRead uses context.Background() for the API
call; change the RunE invocation to pass cmd.Context() (e.g.,
runRead(cmd.Context(), f, opts)) and update the runRead signature (runRead(ctx
context.Context, f *..., opts ...)) so the HTTP/API call inside runRead uses the
passed ctx instead of context.Background(); update any callers or tests
accordingly and ensure imports include "context" where needed.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 44d371d1-32ef-45e7-a92e-7d0a0e22e519

📥 Commits

Reviewing files that changed from the base of the PR and between 8237de0 and dd53f4b.

📒 Files selected for processing (2)
  • internal/cmd/file/list/list.go
  • internal/cmd/file/read/read.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/cmd/file/list/list.go

Comment thread internal/cmd/file/read/read.go Outdated
Comment thread internal/cmd/file/read/read.go Outdated
@leechenghsiu
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Download entire uploaded project to local filesystem instead of
remote querying. Agent can then use native bash tools (ls, cat, grep).

- `zeabur file pull <upload_id> [target-dir]`
- Recursively downloads all text files, skips binaries
- Maintains directory structure

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/cmd/file/pull/pull.go`:
- Around line 37-42: When prompting for upload ID via f.Prompter.Input in
pull.go, validate that the returned id is non-empty before assigning to uploadID
and before any API call; if id == "" return a descriptive error (e.g., "upload
ID cannot be empty") so the function fails fast instead of calling the API with
an empty uploadID. Ensure this check occurs immediately after f.Prompter.Input
and before any subsequent use of uploadID.

In `@pkg/api/file.go`:
- Around line 74-76: The code currently constructs fullRemote := remotePath +
entry and fullLocal := filepath.Join(localDir, fullRemote) using untrusted
entry, which allows path traversal; fix by cleaning and validating the path
derived from entry before joining: use filepath.Clean on entry (and remove any
leading "/" or "." segments), join against the intended base with
filepath.Join(localDir, cleaned) and then verify that the resulting fullLocal
has localDir as its prefix (compare via filepath.Rel or by checking that
strings.HasPrefix after filepath.Abs) and reject or skip any entries that escape
the base directory; apply the same sanitization/validation for the other
occurrences referenced by fullRemote/fullLocal (lines 100-101).
- Around line 88-90: The loop currently increments count for every file
including ones skipped as binary (binaryExtensions and count), which inflates
the reported pulled count; remove the count++ from the binaryExtensions[ext]
branch (the skip path) and ensure count is only incremented at the point where a
file is actually pulled/added to results (i.e., after the successful pull/save
logic), or alternatively decrement count when skipping so the final count
reflects only pulled files.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fd3296b5-a9a3-4217-9121-28a3f78d08cb

📥 Commits

Reviewing files that changed from the base of the PR and between 8c2ca39 and a836fcf.

📒 Files selected for processing (4)
  • internal/cmd/file/file.go
  • internal/cmd/file/pull/pull.go
  • pkg/api/file.go
  • pkg/api/interface.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/cmd/file/file.go

Comment thread internal/cmd/file/pull/pull.go
Comment thread pkg/api/file.go
Comment thread pkg/api/file.go
@leechenghsiu leechenghsiu requested a review from a team April 30, 2026 10:37
…al, fix binary count

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@canyugs
Copy link
Copy Markdown
Contributor

canyugs commented May 3, 2026

Consolidated Review Summary

Reviewed by 4 parties (CodeRabbit + 3 internal reviewers + AI agent).
Overall recommendation: do not merge yet — 2 blockers remain.

🔴 Blockers

  1. Incomplete path traversal protection (pkg/api/file.go:73)
    strings.HasPrefix(filepath.Clean(fullLocal), baseDir) can be bypassed when baseDir="/foo" collides with /foobar. Switch to filepath.Rel and check for .. prefix.

  2. "Pull entire project" semantics mismatch
    Silently skipping binaries leaves users with an incomplete project. Add a summary/warning of skipped files, or reconsider the command's contract.

🟡 Recommended (same PR)

  • Update title from file list/read to file pull to match implementation
  • Add unit tests for pullDir (path traversal + binary skip cases)

🟢 Already addressed

  • upload-id validation, pulled count, interactive fallback, cmd.Context() propagation — all confirmed fixed in head.

🟡 Follow-up (out of scope)

  • Content-based binary detection (vs extension whitelist)
  • Progress indicator for large pulls
  • --force / overwrite policy

- Replace HasPrefix with filepath.Rel to prevent /foo vs /foobar bypass
- Return skipped binary count and show warning to user

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@canyugs canyugs left a comment

Choose a reason for hiding this comment

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

Both blockers from the consolidated review are resolved in e7c7180f:

  • ✅ Path traversal now uses filepath.Rel + .. prefix check
  • ✅ Skipped binary count is reported to the user

Minor follow-ups (non-blocking):

  • PR title still says file list/read — please update to file pull (squash commit message works)
  • Unit tests for pullDir (path traversal + binary skip) recommended as a follow-up

LGTM 🚀

@leechenghsiu
Copy link
Copy Markdown
Contributor Author

Addressed both blockers:

  1. Path traversal: Replaced strings.HasPrefix with filepath.Rel + .. prefix check
  2. Binary skip warning: PullUploadFiles now returns (pulled, skipped, error), CLI shows "Skipped N binary files" when applicable

Also updated PR title.

@leechenghsiu leechenghsiu changed the title feat: add zeabur file list/read commands feat: add zeabur file pull command May 3, 2026
@leechenghsiu leechenghsiu merged commit fee00fb into main May 4, 2026
5 checks passed
@leechenghsiu leechenghsiu deleted the matthewlee/des-536-cli-file-list-read branch May 4, 2026 02:49
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