Skip to content

fix(mothership): fix mothership file uploads#3640

Merged
Sg312 merged 3 commits intostagingfrom
fix/mothership-files
Mar 17, 2026
Merged

fix(mothership): fix mothership file uploads#3640
Sg312 merged 3 commits intostagingfrom
fix/mothership-files

Conversation

@Sg312
Copy link
Collaborator

@Sg312 Sg312 commented Mar 17, 2026

Summary

Fixes mothership file uploads

Type of Change

  • Bug fix

Testing

How has this been tested? What should reviewers focus on?

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Mar 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 17, 2026 11:11pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 17, 2026

Greptile Summary

This PR fixes two bugs in the mothership file upload flow: (1) corrects the previewUrl serve-context query parameter from ?context=copilot to ?context=mothership so that the authorization layer dispatches to the correct handler, and (2) changes trackChatUpload from always inserting a duplicate workspaceFiles record to first attempting to update the existing record that was created by the presigned-upload step, falling back to an insert only when no record is found.

Key changes:

  • use-chat.ts: ?context=copilot?context=mothership in toDisplayAttachment — necessary for verifyFileAccess to route mothership files through verifyWorkspaceFileAccess instead of verifyCopilotFileAccess.
  • workspace-file-manager.ts: trackChatUpload now issues an UPDATE … RETURNING before falling back to INSERT, preventing duplicate rows for the same S3 key.
  • The UPDATE WHERE clause omits a workspaceId filter, which is a minor gap compared to the rest of the file's patterns.
  • Mutating the record's context to 'mothership' causes verifyWorkspaceFileAccess to miss it in its DB lookup path (which only queries context='workspace'). In cloud environments authorization falls back to object-storage metadata and succeeds; in local-storage environments this fallback may not carry workspaceId, potentially causing authorization failures.

Confidence Score: 3/5

  • Safe to merge for cloud deployments; may cause authorization regressions in local-storage environments.
  • The use-chat.ts fix is clearly correct. The trackChatUpload upsert logic is correct in concept, but the context mutation from 'workspace' to 'mothership' breaks the DB-lookup path inside verifyWorkspaceFileAccess for local storage, and the UPDATE is missing a workspaceId scope guard.
  • apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts — the UPDATE query and authorization implications of the context change.

Important Files Changed

Filename Overview
apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts Adds an upsert pattern to trackChatUpload — updates the existing workspace-context record to mothership instead of always inserting a duplicate. The UPDATE WHERE clause is missing a workspaceId filter, and mutating the context from 'workspace' to 'mothership' breaks the authorization DB-lookup path in local-storage environments.
apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts Single-line fix: corrects the previewUrl context query parameter from ?context=copilot to ?context=mothership so that the file-serve route calls verifyWorkspaceFileAccess (which handles mothership) instead of verifyCopilotFileAccess.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Client uploads file via upload API] --> B[Storage service creates workspaceFiles record with context=workspace]
    B --> C[trackChatUpload is called]
    C --> D{Existing record for key?}
    D -- Yes --> E[UPDATE record: set chatId and context=mothership]
    D -- No --> F[INSERT new record with context=mothership]
    E --> G[Client requests file preview]
    F --> G
    G --> H[serve route receives context=mothership]
    H --> I[verifyWorkspaceFileAccess called]
    I --> J{DB lookup for context=workspace}
    J -- Found --> K[Access granted via DB]
    J -- Not found, record is now context=mothership --> L{Cloud storage metadata lookup}
    L -- Has workspaceId --> M[Access granted via metadata]
    L -- No workspaceId local storage --> N[Access DENIED - 404]
Loading

Last reviewed commit: "Fix"

@Sg312 Sg312 merged commit cdd0f75 into staging Mar 17, 2026
11 checks passed
waleedlatif1 added a commit that referenced this pull request Mar 18, 2026
* fix(mothership): fix mothership file uploads (#3640)

* Fix files

* Fix

* Fix

* fix(workspace): prevent stale placeholder data from corrupting workflow registry on switch

* feat(csp): allow chat UI to be embedded in iframes (#3643)

* feat(csp): allow chat UI to be embedded in iframes

Mirror the existing form embed CSP pattern for chat pages: add
getChatEmbedCSPPolicy() with frame-ancestors *, configure /chat/:path*
headers in next.config.ts without X-Frame-Options, and early-return in
proxy.ts so chat routes skip the strict runtime CSP.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* refactor(csp): extract shared getEmbedCSPPolicy helper

Deduplicate getChatEmbedCSPPolicy and getFormEmbedCSPPolicy into a
shared private helper to prevent future divergence.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* fix(logs): add durable execution diagnostics foundation (#3564)

* fix(logs): persist execution diagnostics markers

Store last-started and last-completed block markers with finalization metadata so later read surfaces can explain how a run ended without reconstructing executor state.

* fix(executor): preserve durable diagnostics ordering

Await only the persistence needed to keep diagnostics durable before terminal completion while keeping callback failures from changing execution behavior.

* fix(logs): preserve fallback diagnostics semantics

Keep successful fallback output and accumulated cost intact while tightening progress-write draining and deduplicating trace span counting for diagnostics helpers.

* fix(api): restore async execute route test mock

Add the missing AuthType export to the hybrid auth mock so the async execution route test exercises the 202 queueing path instead of crashing with a 500 in CI.

* fix(executor): align async block error handling

* fix(logs): tighten marker ordering scope

Allow same-millisecond marker writes to replace prior markers and drop the unused diagnostics read helper so this PR stays focused on persistence rather than unread foundation code.

* fix(logs): remove unused finalization type guard

Drop the unused  helper so this PR only ships the persistence-side status types it actually uses.

* fix(executor): await subflow diagnostics callbacks

Ensure empty-subflow and subflow-error lifecycle callbacks participate in progress-write draining before terminal finalization while still swallowing callback failures.

---------

Co-authored-by: test <test@example.com>
Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>

* feat(admin): add user search by email and ID, remove table border

- Replace Load Users button with a live search input; query fires on any input
- Email search uses listUsers with contains operator
- User ID search (UUID format) uses admin.getUser directly for exact lookup
- Remove outer border on user table that rendered white in dark mode
- Reset pagination to page 0 on new search

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(admin): replace live search with explicit search button

- Split searchInput (controlled input) from searchQuery (committed value)
  so the hook only fires on Search click or Enter, not every keystroke
- Gate table render on searchQuery.length > 0 to prevent stale results
  showing after input is cleared

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Siddharth Ganesan <33737564+Sg312@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: PlaneInABottle <y.mirza.altay@gmail.com>
Co-authored-by: test <test@example.com>
Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>
@waleedlatif1 waleedlatif1 deleted the fix/mothership-files branch March 18, 2026 10:36
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.

1 participant