migrate(chat): repoint /api/upload to recoup-api#1740
Open
arpitgupta1214 wants to merge 8 commits into
Open
Conversation
Cuts over `usePureFileAttachments` and `lib/arweave/uploadFile.tsx` to the new dedicated `POST /api/upload` on api (Arweave bytes proxy parity), and removes the local route. The API response shape is 1:1, so this is a base-URL swap. `lib/arweave/uploadToArweave.ts` is kept since it's still imported by `lib/txtGeneration.ts` and `lib/ai/generateImage.ts`.
Phase-1 of the api-side Arweave→Supabase upload migration (recoupable/api#492) returns Supabase public bucket URLs from POST /api/upload. Add the hostname to next.config.mjs remotePatterns so Next/Image can render them.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR migrates file uploads from unauthenticated Arweave storage to authenticated client API uploads secured with Privy access tokens. A new authenticated ChangesAuthenticated File Upload Migration
🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 4 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Migrates file uploads to external API; high blast radius if API not ready or fails. Dependency on api PR landing first. Requires human review for CORS, error handling, and fallback behavior.
The api migration broke every direct Arweave consumer in chat. The remaining callers of lib/arweave/* were three orphans (lib/txtGeneration.ts, lib/ai/generateImage.ts, lib/email/generateTxtFileEmail.ts) imported by nothing, plus uploadFile.tsx itself which was repointed to api in this PR and has nothing Arweave-specific left. - Move lib/arweave/uploadFile.tsx → lib/files/uploadFile.tsx (and rename ArweaveUploadResponse → UploadFileResponse). - Update the 4 hook imports (useUser, useOrgSettings, useSaveKnowledgeEdit, useArtistSetting). - Delete the orphaned consumers. - Delete lib/arweave/ entirely. Net: -415 / +7 lines.
There was a problem hiding this comment.
0 issues found across 14 files (changes from recent commits).
Requires human review: This is a significant migration of file upload infrastructure from local Arweave to an external API, affecting multiple critical paths (chat, profile, settings). High-risk and requires human review to
Threads accessToken through lib/files/uploadFile, which now requires it and attaches Authorization: Bearer to the cross-origin fetch. Updates the five hooks that call uploadFile (useUser, useOrgSettings × 2, useArtistSetting × 2, useSaveKnowledgeEdit) to grab the token via usePrivy().getAccessToken() at call time. Inline fetch in usePureFileAttachments gets the same Bearer header, plus a robust error parse (json().catch(() => null), check both !res.ok and !data.success) consistent with the rest of the upload flow. Pairs with recoupable/api#540 which now requires auth and stamps uploaded_by metadata onto the storage object. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/useOrgSettings.ts">
<violation number="1" location="hooks/useOrgSettings.ts:98">
P2: Handle the null-token case before uploading knowledges to avoid throwing from `uploadFile` mid-loop.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| try { | ||
| const { uri } = await uploadFile(file); | ||
| const accessToken = await getAccessToken(); | ||
| const { uri } = await uploadFile(file, accessToken); |
There was a problem hiding this comment.
P2: Handle the null-token case before uploading knowledges to avoid throwing from uploadFile mid-loop.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useOrgSettings.ts, line 98:
<comment>Handle the null-token case before uploading knowledges to avoid throwing from `uploadFile` mid-loop.</comment>
<file context>
@@ -94,13 +94,14 @@ const useOrgSettings = (orgId: string | null) => {
try {
- const { uri } = await uploadFile(file);
+ const accessToken = await getAccessToken();
+ const { uri } = await uploadFile(file, accessToken);
setImage(uri);
} finally {
</file context>
Collaborator
Author
End-to-end verificationTested via the
API-side endpoint verification: recoupable/api#540. |
After the migration to the dedicated /api/upload (api#540 +
chat#1740 Bundle A), no chat code uploads to Arweave anymore.
Removes the leftover code that still referenced it:
- delete lib/generateAndProcessImage.ts (orphan helper; only file
importing the `arweave` npm package, no internal consumers)
- drop the `arweave` dep from package.json
- rename TxtFileGenerationResult.arweaveUrl → txtUrl to match the
MCP tool's actual response shape (which returns { success, txtUrl,
message }); also drop the now-unused smartAccountAddress /
transactionHash / blockExplorerUrl fields it never populates. The
card was rendering nothing before this fix because the prop name
didn't match.
- strip the stale "e.g., arweave.net" example from the audio
transcription prompt
- update the obsolete "Arweave URL" comment in usePureFileAttachments
Kept: next.config.mjs `arweave.net` entry in images.remotePatterns —
legacy ar:// URLs persisted in the DB (chat history, account avatars
predating the migration) still need to render via the api's
arweaveGatewayUrl read-side helper.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="components/ui/TxtFileResult.tsx">
<violation number="1" location="components/ui/TxtFileResult.tsx:24">
P2: A new `txtUrl` can be ignored because the fetch is blocked when prior `fileContent` exists, causing stale file preview data.</violation>
<violation number="2" location="components/ui/TxtFileResult.tsx:61">
P2: Use `noopener,noreferrer` when opening external URLs in a new tab to avoid opener-based tabnabbing risks.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| if (result.arweaveUrl) { | ||
| window.open(result.arweaveUrl, "_blank"); | ||
| if (result.txtUrl) { | ||
| window.open(result.txtUrl, "_blank"); |
There was a problem hiding this comment.
P2: Use noopener,noreferrer when opening external URLs in a new tab to avoid opener-based tabnabbing risks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/ui/TxtFileResult.tsx, line 61:
<comment>Use `noopener,noreferrer` when opening external URLs in a new tab to avoid opener-based tabnabbing risks.</comment>
<file context>
@@ -60,13 +57,13 @@ export function TxtFileResult({ result }: TxtFileResultProps) {
- if (result.arweaveUrl) {
- window.open(result.arweaveUrl, "_blank");
+ if (result.txtUrl) {
+ window.open(result.txtUrl, "_blank");
}
};
</file context>
Suggested change
| window.open(result.txtUrl, "_blank"); | |
| window.open(result.txtUrl, "_blank", "noopener,noreferrer"); |
Comment on lines
+24
to
26
| if (result.txtUrl && !fileContent) { | ||
| setLoading(true); | ||
| setFetchError(null); |
There was a problem hiding this comment.
P2: A new txtUrl can be ignored because the fetch is blocked when prior fileContent exists, causing stale file preview data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/ui/TxtFileResult.tsx, line 24:
<comment>A new `txtUrl` can be ignored because the fetch is blocked when prior `fileContent` exists, causing stale file preview data.</comment>
<file context>
@@ -24,12 +21,12 @@ export function TxtFileResult({ result }: TxtFileResultProps) {
useEffect(() => {
- if (result.arweaveUrl && !fileContent) {
+ if (result.txtUrl && !fileContent) {
setLoading(true);
setFetchError(null);
</file context>
Suggested change
| if (result.txtUrl && !fileContent) { | |
| setLoading(true); | |
| setFetchError(null); | |
| if (result.txtUrl) { | |
| setFileContent(null); | |
| setLoading(true); | |
| setFetchError(null); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/ui/TxtFileResult.tsx (1)
24-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix stale TXT fetching and download security in
TxtFileResult
- The fetch effect is blocked by
!fileContent, so whenresult.txtUrlchanges whilefileContentis already populated, the component won’t refetch and will display stale contents (lines 23-42).window.open(result.txtUrl, "_blank")is missingnoopener/noreferrer, enabling reverse-tabnabbing (lines 59-63).💡 Proposed fix
useEffect(() => { - if (result.txtUrl && !fileContent) { - setLoading(true); - setFetchError(null); - fetch(result.txtUrl) + if (!result.txtUrl) { + setFileContent(null); + setFetchError(null); + return; + } + + setLoading(true); + setFetchError(null); + setFileContent(null); + fetch(result.txtUrl) .then((res) => { if (!res.ok) throw new Error("Failed to fetch file"); return res.text(); }) .then((text) => { setFileContent(text); setLoading(false); }) .catch((err) => { setFetchError(err.message || "Error fetching file"); setLoading(false); }); - } - // eslint-disable-next-line react-hooks/exhaustive-deps }, [result.txtUrl]);🤖 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 `@components/ui/TxtFileResult.tsx` around lines 24 - 42, The effect in the TxtFileResult component currently gates refetching on !fileContent and can return stale text when result.txtUrl changes; change the useEffect so it depends only on result.txtUrl (remove fileContent from the condition) and use an AbortController to cancel any in-flight fetch when result.txtUrl changes or the component unmounts, keeping the existing setFileContent/setLoading/setFetchError flow for success/error; also fix the download/open logic that calls window.open(result.txtUrl, "_blank") (used in the download handler for the component) to open the URL safely by adding noopener/noreferrer (e.g., use window.open(result.txtUrl, "_blank", "noopener,noreferrer") or create an anchor with rel="noopener noreferrer" and click it) to prevent reverse-tabnabbing.
🤖 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 `@components/ui/TxtFileResult.tsx`:
- Around line 60-62: The window.open calls in the TxtFileResult component
(window.open(result.txtUrl, "_blank")) and in TaskRecentRunsSection that open
links with "_blank" are vulnerable to reverse-tabnabbing; change each call to
include noopener and noreferrer and null out the opener as a fallback—e.g.,
replace window.open(url, "_blank") with: const w = window.open(url, "_blank",
"noopener,noreferrer"); if (w) w.opener = null;—update the calls in the
TxtFileResult component (result.txtUrl) and the TaskRecentRunsSection
window.open usage accordingly.
In `@lib/files/uploadFile.tsx`:
- Around line 30-33: The returned object in lib/files/uploadFile.tsx maps id to
json.id but the API now returns url (not id); update the mapping in the function
that constructs the UploadFileResponse so UploadFileResponse.id is populated
from json.url (or otherwise derive a stable id from json.url) and keep uri as
json.url; ensure references to UploadFileResponse.id use that json.url value
instead of json.id.
---
Outside diff comments:
In `@components/ui/TxtFileResult.tsx`:
- Around line 24-42: The effect in the TxtFileResult component currently gates
refetching on !fileContent and can return stale text when result.txtUrl changes;
change the useEffect so it depends only on result.txtUrl (remove fileContent
from the condition) and use an AbortController to cancel any in-flight fetch
when result.txtUrl changes or the component unmounts, keeping the existing
setFileContent/setLoading/setFetchError flow for success/error; also fix the
download/open logic that calls window.open(result.txtUrl, "_blank") (used in the
download handler for the component) to open the URL safely by adding
noopener/noreferrer (e.g., use window.open(result.txtUrl, "_blank",
"noopener,noreferrer") or create an anchor with rel="noopener noreferrer" and
click it) to prevent reverse-tabnabbing.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bf08b845-4f66-4ea7-b544-2c45e7bcf1df
⛔ Files ignored due to path filters (3)
next.config.mjsis excluded by none and included by nonepackage.jsonis excluded by none and included by nonepnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by none
📒 Files selected for processing (20)
app/api/upload/route.tscomponents/ui/TxtFileResult.tsxhooks/useArtistSetting.tsxhooks/useOrgSettings.tshooks/usePureFileAttachments.tshooks/useSaveKnowledgeEdit.tshooks/useUser.tsxlib/ai/generateImage.tslib/arweave/arweave.tslib/arweave/gateway.tslib/arweave/ipfs.tslib/arweave/uploadFile.tsxlib/arweave/uploadLinkToArweave.tslib/arweave/uploadMetadataJson.tslib/arweave/uploadToArweave.tslib/email/generateTxtFileEmail.tslib/files/uploadFile.tsxlib/generateAndProcessImage.tslib/prompts/getSystemPrompt.tslib/txtGeneration.ts
💤 Files with no reviewable changes (12)
- lib/email/generateTxtFileEmail.ts
- app/api/upload/route.ts
- lib/arweave/uploadToArweave.ts
- lib/arweave/uploadFile.tsx
- lib/arweave/ipfs.ts
- lib/arweave/gateway.ts
- lib/ai/generateImage.ts
- lib/txtGeneration.ts
- lib/generateAndProcessImage.ts
- lib/arweave/uploadMetadataJson.ts
- lib/arweave/arweave.ts
- lib/arweave/uploadLinkToArweave.ts
Comment on lines
+60
to
62
| if (result.txtUrl) { | ||
| window.open(result.txtUrl, "_blank"); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the reported file around the target lines
echo "== TxtFileResult.tsx (around lines 40-90) =="
sed -n '40,90p' components/ui/TxtFileResult.tsx | cat -n
echo
echo "== Search for window.open in the repo (limit output) =="
rg -n "window\.open\(" --glob='**/*.{ts,tsx,js,jsx}' -S . || true
echo
echo "== Search for noopener/noreferrer usage patterns =="
rg -n "noopener|noreferrer" --glob='**/*.{ts,tsx,js,jsx}' -S components . || trueRepository: recoupable/chat
Length of output: 6895
Harden external window.open calls against reverse-tabnabbing.
components/ui/TxtFileResult.tsx opens result.txtUrl with window.open(result.txtUrl, "_blank") without noopener,noreferrer.
💡 Proposed fix
- window.open(result.txtUrl, "_blank");
+ window.open(result.txtUrl, "_blank", "noopener,noreferrer");Also found an unprotected window.open in components/VercelChat/dialogs/tasks/TaskRecentRunsSection.tsx ("_blank" without noopener/noreferrer).
🤖 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 `@components/ui/TxtFileResult.tsx` around lines 60 - 62, The window.open calls
in the TxtFileResult component (window.open(result.txtUrl, "_blank")) and in
TaskRecentRunsSection that open links with "_blank" are vulnerable to
reverse-tabnabbing; change each call to include noopener and noreferrer and null
out the opener as a fallback—e.g., replace window.open(url, "_blank") with:
const w = window.open(url, "_blank", "noopener,noreferrer"); if (w) w.opener =
null;—update the calls in the TxtFileResult component (result.txtUrl) and the
TaskRecentRunsSection window.open usage accordingly.
Comment on lines
+30
to
+33
| return { | ||
| id: json.id, | ||
| uri: json.url, | ||
| }; |
Contributor
There was a problem hiding this comment.
Return mapping doesn’t match the upload response contract.
At Line 31, id is read from json.id, but this migration’s API contract returns url (not id). That makes UploadFileResponse.id unreliable at runtime.
💡 Proposed fix
export type UploadFileResponse = {
- id: string;
uri: string;
};
@@
- return {
- id: json.id,
- uri: json.url,
- };
+ if (typeof json?.url !== "string") {
+ throw new Error("Upload failed");
+ }
+ return { uri: json.url };🤖 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 `@lib/files/uploadFile.tsx` around lines 30 - 33, The returned object in
lib/files/uploadFile.tsx maps id to json.id but the API now returns url (not
id); update the mapping in the function that constructs the UploadFileResponse
so UploadFileResponse.id is populated from json.url (or otherwise derive a
stable id from json.url) and keep uri as json.url; ensure references to
UploadFileResponse.id use that json.url value instead of json.id.
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
1 participant
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.
Summary
Cuts over
usePureFileAttachmentsandlib/arweave/uploadFile.tsxto the new dedicatedPOST /api/uploadon api, removes the local/api/uploadroute, and allows*.supabase.coinnext.config.mjsimages.remotePatternsso Next/Image can render the new public CDN URLs.The api-side endpoint returns the same
{ success, fileName, fileType, fileSize, url }shape, so chat callers needed nothing more than a base-URL swap.Depends on api PR landing on
testfirst: recoupable/api#540.Test plan
test: drop a PNG into chat input — attachment uploads successfully and renders inlinelib/arweave/uploadFile.tsx) succeedsSummary by cubic
Repointed file uploads to the recoup API
POST /api/upload, removed Arweave, and fixed the TXT result card to usetxtUrlso generated files render correctly. Uploads now send a Privy Bearer token, and Supabase public URLs render via Next/Image.${getClientApiBaseUrl()}/api/uploadwithAuthorization: Bearer <accessToken>inusePureFileAttachmentsandlib/files/uploadFile.tsx; fetch tokens viausePrivy().getAccessToken()inuseUser,useOrgSettings,useArtistSetting, anduseSaveKnowledgeEdit.app/api/upload/route.ts; movelib/arweave/uploadFile.tsx→lib/files/uploadFile.tsx; improve upload error handling (safe JSON parse; checkres.okanddata.success).lib/arweave/*,lib/ai/generateImage.ts,lib/txtGeneration.ts,lib/generateAndProcessImage.ts, andlib/email/generateTxtFileEmail.ts; droparweavefrompackage.json; updateTxtFileResulttotxtUrl; trim Arweave wording from the system prompt.https://*.supabase.co/storage/v1/object/public/**innext.config.mjsimages; keeparweave.netfor legacy content.recoupable/api#540ontest(auth-required endpoint).Written for commit 23e9e23. Summary will update on new commits.
Review in cubic
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Removed Features
Documentation