Download image assets uploaded to GitLab review threads#17
Download image assets uploaded to GitLab review threads#17stefanvictora wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughGitLab thread image assets can now be downloaded into ChangesReview asset localization flow
Sequence Diagram(s)sequenceDiagram
participant Orchestrator
participant GitLabProvider
participant WorkspaceManager
Orchestrator->>GitLabProvider: localizeReviewAssets(ref, threads, options)
GitLabProvider-->>Orchestrator: assetRewrites
Orchestrator->>WorkspaceManager: createBundle(..., { latestPatchContent, assetRewrites })
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/providers/gitlab/gitlab-provider.ts (1)
456-469: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a timeout on the binary download fetch.
requestBinaryUrlhas no timeout/abort. A stalled upload response can hanglocalizeReviewAssets(and thusprepare) indefinitely. AnAbortSignal.timeout(...)keeps the best-effort download bounded.🤖 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/providers/gitlab/gitlab-provider.ts` around lines 456 - 469, The requestBinaryUrl helper currently performs fetch without any timeout, so a stalled binary download can block localizeReviewAssets and prepare indefinitely. Update requestBinaryUrl to use an AbortSignal timeout (or equivalent abort controller) when calling fetch, and make sure the timeout is applied alongside the existing headers and optional fetchDispatcher handling so the download remains bounded.
🤖 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.
Nitpick comments:
In `@src/providers/gitlab/gitlab-provider.ts`:
- Around line 456-469: The requestBinaryUrl helper currently performs fetch
without any timeout, so a stalled binary download can block localizeReviewAssets
and prepare indefinitely. Update requestBinaryUrl to use an AbortSignal timeout
(or equivalent abort controller) when calling fetch, and make sure the timeout
is applied alongside the existing headers and optional fetchDispatcher handling
so the download remains bounded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 37a10892-9103-4a96-aeb5-99f00ca3e76a
📒 Files selected for processing (7)
CHANGELOG.mdsrc/orchestration/orchestrator.tssrc/providers/gitlab/gitlab-provider.test.tssrc/providers/gitlab/gitlab-provider.tssrc/providers/provider.tssrc/workspace/workspace-manager.test.tssrc/workspace/workspace-manager.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 334ece16f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| res = await fetch(url, { | ||
| headers, | ||
| ...(this.fetchDispatcher ? { dispatcher: this.fetchDispatcher } : {}), | ||
| } as RequestInit); |
There was a problem hiding this comment.
Avoid forwarding GitLab tokens on asset redirects
In GitLab instances backed by object storage or a CDN, upload downloads can return a 302 to a different host; this fetch follows redirects by default while reusing headers, and Node/undici preserves custom headers like PRIVATE-TOKEN across those redirects. That means preparing a bundle for a thread image can send the user's GitLab token to the storage/CDN origin, so redirects need to be handled manually and auth headers kept only on same-origin GitLab requests.
Useful? React with 👍 / 👎.
Summary by CodeRabbit