Let clip viewers reshare public/org clips via copy-link - #2812
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Here's a visual recap of what changed: Open the full interactive recap |
|
@builderio-bot look at latest PR feedback and fix anything you agree with. Be skeptical. Reply to every comment (directly on the comment thread of each comment) if you fixed it or not and why. then check back every 2 minutes on a loop and see if any new feedback posted, until at least 20 minutes go by without any new feedback posted we want to address, including making sure CI passes too and no merge conflicts (make sure code is mergeable) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@SajalChaplot can you share a screenshot of the change? |
|
@steve8708 I couldn't figure out the deploy preview url for clips. What is it named? I can capture the screenshot there. |
|
are you using Builder or a local dev env? would suggest making sure you do one of those so you aren't dependent on deployment for UI testing |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
I encountered an error while processing your request. Please try again. |
|
Added the screenshot. It is viewer inside organization. Was using local environment with claude, Builder app just keeps crashing my mac. Idk why. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Builder reviewed your changes and found 1 potential issue 🟡
Review Details
Code Review Summary
This incremental review covers the latest cross-route sharing changes. The prior /share/:shareId versus /r/:recordingId restriction gap is addressed, and the dialog still avoids loading access-list data for viewer-only mode while preserving editor read-only and owner/admin management behavior.
Key Findings
🟡 MEDIUM: /r/:recordingId now marks every viewer as viewerReshareOnly and renders the Share dialog unconditionally, including viewers with explicit access to private recordings. In the non-public Link tab, this mode still mints an agent-readable bearer link, allowing a private viewer to forward the clip despite the /share/:shareId path intentionally hiding Share for private viewers.
Risk classification: High, because this PR changes sharing and media-access controls. Browser verification was attempted but blocked by an unrelated stale Better Auth database pool that prevents sign-in/sign-up; the dev server otherwise starts successfully.
🧪 Browser testing: Will run after this review (PR touches UI code).
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Builder reviewed your changes and found 1 potential issue 🔴
Review Details
Code Review Summary
This incremental review verifies that the previously reported private-viewer route condition was updated to restrict viewerReshareOnly to public/org recordings. However, the viewer-only flag is not propagated into LinkTab, so the core restriction is incomplete for organization-visible viewers.
Key Findings
🔴 HIGH: An org viewer who opens the newly available Share dialog can still trigger create-recording-agent-link and see the agent-sharing controls. The flag currently only disables the access-list query and Invite tab; LinkTab still treats the initial org visibility as loaded and runs its non-public agent-link effect. This mints a bearer URL beyond the intended copy-link-only capability.
The editor/admin paths and native download gating otherwise remain consistent. Risk classification: High because this concerns sharing authorization and bearer-link creation. Browser testing was attempted, but authentication was blocked by a sandbox-wide Neon/Postgres pool failure; static source analysis confirmed the finding.
🧪 Browser testing: Will run after this review (PR touches UI code).
| @@ -265,21 +285,23 @@ function ShareRecordingContent({ | |||
| /> | |||
There was a problem hiding this comment.
🔴 Propagate viewer-only restrictions into LinkTab
viewerReshareOnly prevents the shares query and hides Invite, but it is not passed to LinkTab. For an org-visible viewer, initialVisibility makes the link state loaded, so LinkTab still runs create-recording-agent-link and renders the agent-sharing controls, allowing a bearer URL instead of only the intended human copy link. Pass the restriction through and suppress the agent-link mutation/controls (and other non-copy-only extras) in viewer-only mode.
Additional Info
The LinkTab effect invokes the mutation whenever visibility is non-public; org visibility therefore follows that path even though ShareRecordingContent is in viewerReshareOnly mode.
There was a problem hiding this comment.
Agreed, real gap — viewerReshareOnly only stopped list-resource-shares and the Invite tab, but LinkTab's own effect independently calls create-recording-agent-link whenever !isPublic, which includes org-visibility reshare viewers, minting a scoped bearer-token URL for them.
Fixed in 827f365: threaded viewerReshareOnly down into LinkTab, added it to the effect's early-return guard so the agent link is never minted in that mode, and hid the whole "share with agents" block (!isPublic && !viewerReshareOnly) since there's no bearer-token-minting capability to offer a bare-link-only viewer anyway.
77a2349 to
0ae03ee
Compare
|
@builderio-bot look at latest PR feedback and fix anything you agree with. Be skeptical. Reply to every comment (directly on the comment thread of each comment) if you fixed it or not and why. then check back every 2 minutes on a loop and see if any new feedback posted, until at least 20 minutes go by without any new feedback posted we want to address, including making sure CI passes too and no merge conflicts (make sure code is mergeable) |
A plain viewer who reached a clip through public/org visibility never saw the Share button on /share/:shareId (gated on viewerCanEdit), even though the copy-link field inside the dialog already worked for any role. Show the Share button for viewers too when the clip's visibility is public or org, and restrict their dialog to the Link/Embed tabs so they can forward the clip without seeing admin-only invite/visibility controls.
…invite view, gate download by enableDownloads
main's role model (post-rebase) added a "commenter" role between viewer and editor. viewerReshareOnly here still only matched "viewer", so a commenter landing on /r/:recordingId would trigger the full list-resource-shares call (leaking every collaborator's email) instead of getting the same reshare-only treatment a plain viewer gets. Mirrors the equivalent fix already applied to canReshareLink in share.$shareId.tsx.
0b842ca to
c260b19
Compare
|
There was a problem with your request, please try again later. Error id: |
This comment has been minimized.
This comment has been minimized.
|
@builderio-bot look at latest PR feedback and fix anything you agree with. Be skeptical. Reply to every comment (directly on the comment thread of each comment) if you fixed it or not and why. then check back every 2 minutes on a loop and see if any new feedback posted, until at least 20 minutes go by without any new feedback posted we want to address, including making sure CI passes too and no merge conflicts (make sure code is mergeable) Goal for this PR was to allow non-clip owners within an organization to have an option to share clip to agent. |
|
20 minutes have passed with no new feedback, all CI checks green, no merge conflicts, SummaryNothing new arrived since I last resolved conflicts — no new review comments to fix, and CI stayed green for the full 20-minute window. I also verified against the stated PR goal ("let non-clip-owners within an org share a clip to an agent"): the current code gates the visibility dropdown and the Invite/People list behind ⠀ |
|
@SajalChaplot something i noticed in your screenshots - when theres not two tabs at the top we do'nt have to show one big "link" tab, lets just hide those tabs at the top, otherewise looks like a confusing button |
A reshare-only viewer on a non-public clip only ever sees the Link tab (no Invite, no Embed), so the tab list rendered a single "Link" trigger that looked like a confusing button rather than an actual tab switcher. Hide the tab bar entirely when there's nothing to switch between.
9ee52fd to
1e6535c
Compare
|
Fixed the tab UI problem before merging. |
3 participants
Summary
public/orgvisibility never saw the Share button on/share/:shareId— it was gated onviewerCanEdit(owner/admin/editor only), even though the dialog's copy-link field already worked for any role once opened.recording.visibilityispublicororg(a viewer on a narrowly-sharedprivateclip still sees nothing, preserving the owner's tighter intent).Why
Goal: when another user views a clip the creator shared with visibility set to public or organization, the receiver should be able to reshare that clip (to humans or agents) via a simple copy-link, without gaining any admin-level sharing controls.
Viewer UI for public link
Viewer UI for org link
No backend/action changes were needed —
list-resource-sharesandcreate-recording-agent-linkalready permit any viewer; only the Share button's visibility and the dialog's tab set needed adjusting.Test plan
oxfmton both touched filesviewerCanEdit/canManagepatterns (exact conditions added, no new state)/share/:shareIdwas blocked by an unrelated, pre-existing gate in a fresh local dev DB (no storage provider configured → clip never reachesstatus: "ready", which early-returns before the Share button code for any role) — reviewers with areadyclip and a second account should confirm the Share button now appears for aviewerrole on public/org clips and that the dialog shows Link+Embed only.