feat(flows): fix large asset downloads and add per-file progress - #1459
Merged
Conversation
WalkthroughThe change adds per-file progress callbacks for team-storage asset downloads and passes progress messages through the pull handler to human and agent renderers. JSON output remains unchanged. Signed-URL downloads now use a 30-second no-data stall timeout, stream response chunks into temporary files, and clean up failed downloads. Filesystem write handles and in-memory filesystem coverage support the streaming implementation. Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PullHandler
participant PlatformClient
participant writeAssetSnapshot
participant fetchSignedUrl
participant Renderer
PullHandler->>PlatformClient: syncTeamStorageAssets(onProgress)
PlatformClient->>writeAssetSnapshot: download assets
writeAssetSnapshot->>fetchSignedUrl: fetch signed URL
fetchSignedUrl-->>writeAssetSnapshot: persist downloaded asset
writeAssetSnapshot-->>PlatformClient: current and total counts
PlatformClient-->>PullHandler: progress callback
PullHandler->>Renderer: update progress message
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/shell/platform/fetchSignedUrl.ts`:
- Around line 72-97: Update the download flow around the response body reader to
write each received chunk directly to a temporary destination file instead of
accumulating chunks and constructing a second full-size Uint8Array. After all
reads and the write complete, rename the temporary file to args.dest; ensure
timeout, network, and write failures clean up the temporary file and preserve
their existing error results.
In `@src/shell/platform/teamStorageAssets.test.ts`:
- Line 59: Remove the duplicate declarations causing compilation failures: in
src/shell/platform/teamStorageAssets.test.ts lines 59-59 and
src/shell/platform/teamStorageAssets.reuse.test.ts lines 74-74, retain only one
const progress declaration in each test; in
src/domains/flows/pull/handler.test.ts lines 151-151, retain only one onProgress
property in the options type.
🪄 Autofix
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: ASSERTIVE
Plan: Pro
Run ID: 5ad91122-5294-482d-ad61-ead37154ce28
📒 Files selected for processing (23)
.changeset/asset-download-progress-counter.md.changeset/stall-timeout-signed-url-downloads.mdsrc/core/messages/auth.tssrc/core/messages/flows.tssrc/domains/flows/pull/handler.test.tssrc/domains/flows/pull/handler.tssrc/shell/platform/createPlatformClient.tssrc/shell/platform/describeErrors.test.tssrc/shell/platform/describeErrors.tssrc/shell/platform/fetchSignedUrl.test.tssrc/shell/platform/fetchSignedUrl.tssrc/shell/platform/slowFetch.testUtils.tssrc/shell/platform/teamStorageAssets.reuse.test.tssrc/shell/platform/teamStorageAssets.test.tssrc/shell/platform/teamStorageAssets.tssrc/shell/platform/writeAssetSnapshot.tssrc/shell/ui/renderers/modes/agent.test.tssrc/shell/ui/renderers/modes/agent.tssrc/shell/ui/renderers/modes/human.tssrc/shell/ui/renderers/modes/human.withProgress.update.test.tssrc/shell/ui/renderers/modes/json.test.tssrc/shell/ui/renderers/modes/json.tssrc/shell/ui/renderers/modes/progress.ts
Chase J (chajac)
marked this pull request as ready for review
August 11, 2026 12:25
Chase J (chajac)
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/shell/fs.ts`:
- Around line 99-101: Update the async write method to loop until the entire
chunk has been written, advancing by FileHandle.write()’s bytesWritten result
and retrying the unwritten suffix. If any write reports zero bytes, fail instead
of accepting the next chunk, while preserving sequential chunk processing.
In `@src/shell/platform/fetchSignedUrl.ts`:
- Around line 75-79: Abort the active fetch before returning from both local
filesystem failure handlers in fetchSignedUrl.ts:
src/shell/platform/fetchSignedUrl.ts lines 75-79 and 103-108. In each catch
block, call the existing controller.abort with the caught error, then preserve
the current failure result.
🪄 Autofix
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: ASSERTIVE
Plan: Pro
Run ID: cf3e979d-4995-459d-a6a1-648903944a95
📒 Files selected for processing (9)
.changeset/stream-signed-url-downloads.mdsrc/shell/fs.testUtils.dirTree.test.tssrc/shell/fs.testUtils.syncAndStreams.test.tssrc/shell/fs.testUtils.test.tssrc/shell/fs.testUtils.tssrc/shell/fs.tssrc/shell/memoryFsTree.testUtils.tssrc/shell/platform/fetchSignedUrl.test.tssrc/shell/platform/fetchSignedUrl.ts
💤 Files with no reviewable changes (1)
- src/shell/fs.testUtils.test.ts
Felipe Augusto (felipe-augusto)
approved these changes
Aug 11, 2026
Felipe Augusto (felipe-augusto)
left a comment
Felipe Augusto (felipe-augusto)
left a comment
Member
There was a problem hiding this comment.
I don't see anything worth raising here, but I'm not an domain expert here so might as well request for another pair of eyes on this one
# Conflicts: # src/shell/platform/describeErrors.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shell/platform/describeErrors.ts (1)
12-30: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve
errorBodyin pull failuresAt
src/domains/flows/pull/fetchPhase.ts:27,36andsrc/domains/flows/pull/handler.ts:108,new Error(result.error)discardsresult.errorBody. The command context renderserrorBodyonly for returned failures, so server-provided reasons are lost. Propagate both fields to the command boundary or include both values in the thrown error.🤖 Prompt for 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. In `@src/shell/platform/describeErrors.ts` around lines 12 - 30, Preserve server-provided error details in pull failures by updating the fetch phase and pull handler paths that currently throw new Error(result.error). Propagate result.errorBody alongside result.error to the command boundary, or include both values in the thrown error so returned failures retain the original reason.
🤖 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.
Outside diff comments:
In `@src/shell/platform/describeErrors.ts`:
- Around line 12-30: Preserve server-provided error details in pull failures by
updating the fetch phase and pull handler paths that currently throw new
Error(result.error). Propagate result.errorBody alongside result.error to the
command boundary, or include both values in the thrown error so returned
failures retain the original reason.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 93aa56a6-ce0e-4696-bbb3-82944b59d873
📒 Files selected for processing (2)
src/shell/platform/describeErrors.test.tssrc/shell/platform/describeErrors.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
PR body AI drafted & edited as needed
Overview of Changes
flows pullfailed on environments that have large team-storage assets. Each download had a fixed 30-second deadline. A 1.4 GB asset could not finish in that time, so the pull always stopped with a timeout error. The user also could not see how many files remained during a pull.src/shell/platform/fetchSignedUrl.ts: the download now uses a stall timeout in place of a whole-download deadline. The 30-second timer restarts each time data arrives. A slow download that makes progress can finish at any size. A download that receives no data for 30 seconds still fails. Error classification does not change.fetchSignedUrl.tswith a newFs.openWriteHandleprimitive: the download writes each chunk to a.partfile. The CLI renames the file to its final name when the download completes. Before, the CLI held the full body in memory and briefly needed about two times the file size. A failed download removes the partial file. The stall timer stops while a chunk writes to disk. Because of this, a slow disk cannot cause a false stall. A measured 1.5 GB pull now uses a maximum of about 290 MB of memory. Before, it used 3.35 GB.describeErrors.ts/messages/auth.ts: the timeout messages now say that the download stalled and show the stall time. A download can now run longer than 30 seconds before it fails, so the old "timed out after 30s" text was not correct.shell/uiwithProgress: each task now receives anupdate(message)callback. Human mode replaces the spinner label with each new message. If a task fails, the error line keeps the last message. Verbose mode logs each message as a step line. Agent mode writes one line to stderr for each message. The json output does not change. Tasks that ignore the callback do not change.writeAssetSnapshot.ts/createPlatformClient.ts/ pull handler: the download loop sends progress events to the progress step. The step shows "Downloading team-storage assets (2/45)". The total counts only the files that download. Reused and skipped files are not included.Testing
bun run test bun run typecheck bun run lint bun run format:check bun run knipThis is the
/usr/bin/time -loutput for the streamed 1.5 GB pull:Checklist