From 652621356dff8ab48056b67df591599f24b92312 Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:11:04 -0400 Subject: [PATCH 01/12] Apply password verification to Clips comments --- templates/clips/actions/add-comment.ts | 4 +- .../actions/comment-writing-access.test.ts | 164 ++++++++++++++++-- .../clips/actions/delete-comment.test.ts | 47 +++++ templates/clips/actions/delete-comment.ts | 8 +- templates/clips/actions/list-comments.test.ts | 28 +++ templates/clips/actions/list-comments.ts | 4 +- templates/clips/actions/react-to-comment.ts | 8 +- templates/clips/actions/reply-to-comment.ts | 23 +-- templates/clips/actions/resolve-comment.ts | 8 +- templates/clips/actions/update-comment.ts | 8 +- .../components/player/comments-panel.test.tsx | 54 ++++++ .../app/components/player/comments-panel.tsx | 14 ++ templates/clips/app/routes/share.$shareId.tsx | 3 + ...wers-retain-comment-access-while-protec.md | 6 + .../clips/server/lib/comment-access-token.ts | 18 ++ templates/clips/server/lib/comment-access.ts | 28 ++- .../routes/api/public-recording.get.test.ts | 96 +++++++++- .../server/routes/api/public-recording.get.ts | 13 ++ templates/clips/shared/comment-access.ts | 2 + 19 files changed, 489 insertions(+), 47 deletions(-) create mode 100644 templates/clips/changelog/2026-10-23-password-unlocked-viewers-retain-comment-access-while-protec.md create mode 100644 templates/clips/server/lib/comment-access-token.ts diff --git a/templates/clips/actions/add-comment.ts b/templates/clips/actions/add-comment.ts index 400ae3b3bdf..7100ccb0121 100644 --- a/templates/clips/actions/add-comment.ts +++ b/templates/clips/actions/add-comment.ts @@ -56,8 +56,8 @@ export default defineAction({ .optional() .describe("Organization members mentioned in the comment"), }), - run: async (args) => { - await assertCommentAccess(args.recordingId); + run: async (args, ctx) => { + await assertCommentAccess(args.recordingId, "write", ctx?.requestHeaders); const authorEmail = getRequestUserEmail(); if (!authorEmail) { diff --git a/templates/clips/actions/comment-writing-access.test.ts b/templates/clips/actions/comment-writing-access.test.ts index 6c4bf51ca37..79706a79730 100644 --- a/templates/clips/actions/comment-writing-access.test.ts +++ b/templates/clips/actions/comment-writing-access.test.ts @@ -1,3 +1,8 @@ +import { createHash } from "node:crypto"; + +import type { ActionRunContext } from "@agent-native/core/action"; +import { signScopedAgentAccessToken } from "@agent-native/core/server"; +import { signShortLivedToken } from "@agent-native/core/server/short-lived-token"; import { ForbiddenError } from "@agent-native/core/sharing"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -96,43 +101,88 @@ import replyToComment from "./reply-to-comment"; import resolveComment from "./resolve-comment"; import updateComment from "./update-comment"; +const protectedRecording = { + enableComments: true, + password: "fake-stored-password", + sharePasswordVersion: "initial", +}; +function proofContext(kind?: string): ActionRunContext { + const scope = createHash("sha256") + .update( + JSON.stringify(["recording-1", protectedRecording.password, "initial"]), + ) + .digest("hex"); + let resourceId = `clips-comment:recording-1:${scope}`; + if (kind === "media") resourceId = "recording-1"; + + if (kind === "wrong-record") + resourceId = `clips-comment:recording-2:${scope}`; + if (kind === "rotated") resourceId = "clips-comment:recording-1:old-version"; + let token = signShortLivedToken({ + resourceId, + ttlSeconds: kind === "expired" ? -1 : 600, + }); + if (kind === "agent") + token = signScopedAgentAccessToken({ + resourceKind: "clip-agent-context", + resourceId: "recording-1", + }); + if (kind === "malformed") token = "malformed"; + if (kind === "tampered") token = token.slice(0, -5) + "AAAAA"; + return { + requestHeaders: new Headers(kind ? { "x-clips-comment-token": token } : {}), + }; +} + const writers = [ { name: "new comment", - run: () => - addComment.run({ - recordingId: "recording-1", - content: "New text", - videoTimestampMs: 0, - }), + run: (ctx?: ActionRunContext) => + addComment.run( + { + recordingId: "recording-1", + content: "New text", + videoTimestampMs: 0, + }, + ctx, + ), }, { name: "reply through add-comment", - run: () => - addComment.run({ - recordingId: "recording-1", - content: "Reply text", - videoTimestampMs: 12542, - parentId: "comment-1", - threadId: "thread-1", - }), + run: (ctx?: ActionRunContext) => + addComment.run( + { + recordingId: "recording-1", + content: "Reply text", + videoTimestampMs: 12542, + parentId: "comment-1", + threadId: "thread-1", + }, + ctx, + ), }, { name: "reply-to-comment", - run: () => - replyToComment.run({ commentId: "comment-1", content: "Reply text" }), + run: (ctx?: ActionRunContext) => + replyToComment.run( + { commentId: "comment-1", content: "Reply text" }, + ctx, + ), }, { name: "edit", - run: () => updateComment.run({ id: "comment-1", content: "Updated text" }), + run: (ctx?: ActionRunContext) => + updateComment.run({ id: "comment-1", content: "Updated text" }, ctx), }, { name: "reaction", - run: () => reactToComment.run({ commentId: "comment-1", emoji: "👍" }), + run: (ctx?: ActionRunContext) => + reactToComment.run({ commentId: "comment-1", emoji: "👍" }, ctx), }, { name: "resolve", - run: () => resolveComment.run({ id: "comment-1", resolved: true }), + run: (ctx?: ActionRunContext) => + resolveComment.run({ id: "comment-1", resolved: true }, ctx), }, ]; @@ -151,6 +201,82 @@ beforeEach(() => { }); describe.each(writers)("$name comment writing access", ({ run }) => { + it.each([ + undefined, + "malformed", + "media", + "agent", + "wrong-record", + "expired", + "rotated", + "tampered", + ])( + "refuses password-protected writes with %s proof before side effects", + async (kind) => { + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: protectedRecording, + }); + await expect(run(proofContext(kind))).rejects.toBeInstanceOf( + ForbiddenError, + ); + expect(state.inserted).toEqual([]); + expect(mockUpdate).not.toHaveBeenCalled(); + expect(mockNotify).not.toHaveBeenCalled(); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }, + ); + + it("accepts a purpose-scoped password grant", async () => { + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: protectedRecording, + }); + await expect(run(proofContext("valid"))).resolves.toBeDefined(); + expect(state.inserted.length + mockUpdate.mock.calls.length).toBe(1); + }); + + it.each([ + { password: "fake-new-stored-password" }, + { sharePasswordVersion: "fake-next-version" }, + ])("invalidates proof after password changes: %j", async (change) => { + const ctx = proofContext("valid"); + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: { ...protectedRecording, ...change }, + }); + await expect(run(ctx)).rejects.toBeInstanceOf(ForbiddenError); + expect(state.inserted).toEqual([]); + expect(mockUpdate).not.toHaveBeenCalled(); + }); + + it("lets the owner write without password proof", async () => { + mockAssertAccess.mockResolvedValue({ + role: "owner", + resource: protectedRecording, + }); + await expect(run()).resolves.toBeDefined(); + }); + + it.each([ + { enableComments: false }, + { trashedAt: "2026-01-01T00:00:00Z" }, + { expiresAt: "2000-01-01T00:00:00Z" }, + ])( + "keeps lifecycle restrictions with a valid grant: %j", + async (overrides) => { + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: { ...protectedRecording, ...overrides }, + }); + await expect(run(proofContext("valid"))).rejects.toBeInstanceOf( + ForbiddenError, + ); + expect(state.inserted).toEqual([]); + expect(mockUpdate).not.toHaveBeenCalled(); + }, + ); + it.each(["viewer", "owner"])( "refuses disabled comments for a %s before side effects", async (role) => { diff --git a/templates/clips/actions/delete-comment.test.ts b/templates/clips/actions/delete-comment.test.ts index 9fc221fce97..4df339dea25 100644 --- a/templates/clips/actions/delete-comment.test.ts +++ b/templates/clips/actions/delete-comment.test.ts @@ -1,3 +1,6 @@ +import { createHash } from "node:crypto"; + +import { signShortLivedToken } from "@agent-native/core/server/short-lived-token"; import { ForbiddenError } from "@agent-native/core/sharing"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -141,6 +144,23 @@ beforeEach(() => { }); describe("delete-comment", () => { + it("refuses a password-protected author's deletion without proof", async () => { + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: { + enableComments: true, + password: "fake-password", + sharePasswordVersion: "initial", + }, + }); + const original = [...state.rows]; + await expect(action.run({ id: "comment-1" })).rejects.toBeInstanceOf( + ForbiddenError, + ); + expect(state.rows).toEqual(original); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + it.each([ { role: "viewer", email: "author@example.com" }, { role: "editor", email: "editor@example.com" }, @@ -252,3 +272,30 @@ describe("delete-comment", () => { ]); }); }); + +it("deletes the unlocked author's comment with purpose-scoped proof", async () => { + const password = "fake-stored-password"; + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: { + enableComments: true, + password, + sharePasswordVersion: "initial", + }, + }); + const scope = createHash("sha256") + .update(JSON.stringify(["recording-1", password, "initial"])) + .digest("hex"); + const token = signShortLivedToken({ + resourceId: `clips-comment:recording-1:${scope}`, + ttlSeconds: 600, + }); + await expect( + action.run( + { id: "comment-1" }, + { requestHeaders: new Headers({ "x-clips-comment-token": token }) }, + ), + ).resolves.toMatchObject({ + deletedCommentIds: ["comment-1", "comment-1-reply", "comment-1-reply-2"], + }); +}); diff --git a/templates/clips/actions/delete-comment.ts b/templates/clips/actions/delete-comment.ts index 10bcba12dc6..d3ec524590c 100644 --- a/templates/clips/actions/delete-comment.ts +++ b/templates/clips/actions/delete-comment.ts @@ -15,7 +15,7 @@ export default defineAction({ schema: z.object({ id: z.string().describe("Comment ID"), }), - run: async (args) => { + run: async (args, ctx) => { const db = getDb(); const [existing] = await db .select() @@ -27,7 +27,11 @@ export default defineAction({ const userEmail = getRequestUserEmail(); const isAuthor = sameOwnerEmail(existing.authorEmail, userEmail); - await assertCommentAccess(existing.recordingId); + await assertCommentAccess( + existing.recordingId, + "write", + ctx?.requestHeaders, + ); if (!isAuthor) { try { diff --git a/templates/clips/actions/list-comments.test.ts b/templates/clips/actions/list-comments.test.ts index 0116fc7924a..c4dc70e61d5 100644 --- a/templates/clips/actions/list-comments.test.ts +++ b/templates/clips/actions/list-comments.test.ts @@ -1,3 +1,6 @@ +import { createHash } from "node:crypto"; + +import { signShortLivedToken } from "@agent-native/core/server/short-lived-token"; import { ForbiddenError } from "@agent-native/core/sharing"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -112,3 +115,28 @@ describe("list-comments recording policy", () => { }, ); }); + +it("reads unlocked password comments with the same purpose-scoped proof", async () => { + const password = "fake-stored-password"; + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: { + enableComments: true, + password, + sharePasswordVersion: "initial", + }, + }); + const scope = createHash("sha256") + .update(JSON.stringify(["recording-1", password, "initial"])) + .digest("hex"); + const token = signShortLivedToken({ + resourceId: `clips-comment:recording-1:${scope}`, + ttlSeconds: 600, + }); + await expect( + listComments.run( + { recordingId: "recording-1" }, + { requestHeaders: new Headers({ "x-clips-comment-token": token }) }, + ), + ).resolves.toMatchObject({ comments: [{ id: "comment-1" }] }); +}); diff --git a/templates/clips/actions/list-comments.ts b/templates/clips/actions/list-comments.ts index c822ec5c762..3ec3749c5ec 100644 --- a/templates/clips/actions/list-comments.ts +++ b/templates/clips/actions/list-comments.ts @@ -14,8 +14,8 @@ export default defineAction({ recordingId: z.string().describe("Recording ID"), }), http: { method: "GET" }, - run: async (args) => { - await assertCommentAccess(args.recordingId, "read"); + run: async (args, ctx) => { + await assertCommentAccess(args.recordingId, "read", ctx?.requestHeaders); const db = getDb(); const rows = await db diff --git a/templates/clips/actions/react-to-comment.ts b/templates/clips/actions/react-to-comment.ts index f29f92c605f..a8d445912e1 100644 --- a/templates/clips/actions/react-to-comment.ts +++ b/templates/clips/actions/react-to-comment.ts @@ -17,7 +17,7 @@ export default defineAction({ commentId: z.string().describe("Comment ID"), emoji: z.string().min(1).describe("Emoji character (e.g. 👍, ❤️, 🔥)"), }), - run: async (args) => { + run: async (args, ctx) => { const db = getDb(); const viewerEmail = getRequestUserEmail(); if (!viewerEmail) { @@ -33,7 +33,11 @@ export default defineAction({ if (!existing) throw new Error(`Comment not found: ${args.commentId}`); if (attempt === 0) { - await assertCommentAccess(existing.recordingId); + await assertCommentAccess( + existing.recordingId, + "write", + ctx?.requestHeaders, + ); } const previousJson = existing.emojiReactionsJson; diff --git a/templates/clips/actions/reply-to-comment.ts b/templates/clips/actions/reply-to-comment.ts index 6d685d6b142..aa89952c731 100644 --- a/templates/clips/actions/reply-to-comment.ts +++ b/templates/clips/actions/reply-to-comment.ts @@ -25,7 +25,7 @@ export default defineAction({ .optional() .describe("Organization members mentioned in the reply"), }), - run: async (args) => { + run: async (args, ctx) => { const db = getDb(); const [parent] = await db .select() @@ -34,15 +34,18 @@ export default defineAction({ .limit(1); if (!parent) throw new Error(`Comment not found: ${args.commentId}`); - const result = await addComment.run({ - recordingId: parent.recordingId, - threadId: parent.threadId, - parentId: parent.id, - videoTimestampMs: parent.videoTimestampMs, - content: args.content, - authorName: args.authorName, - mentions: args.mentions, - }); + const result = await addComment.run( + { + recordingId: parent.recordingId, + threadId: parent.threadId, + parentId: parent.id, + videoTimestampMs: parent.videoTimestampMs, + content: args.content, + authorName: args.authorName, + mentions: args.mentions, + }, + ctx, + ); return { ...result, parentId: parent.id }; }, diff --git a/templates/clips/actions/resolve-comment.ts b/templates/clips/actions/resolve-comment.ts index 1c2e4ad24d7..15eafcf301a 100644 --- a/templates/clips/actions/resolve-comment.ts +++ b/templates/clips/actions/resolve-comment.ts @@ -25,7 +25,7 @@ export default defineAction({ .optional() .describe("Explicit resolved value. Omit to toggle."), }), - run: async (args) => { + run: async (args, ctx) => { const db = getDb(); const [existing] = await db .select() @@ -39,7 +39,11 @@ export default defineAction({ throw new Error("Sign in required to resolve comments."); } - await assertCommentAccess(existing.recordingId); + await assertCommentAccess( + existing.recordingId, + "write", + ctx?.requestHeaders, + ); if (!sameOwnerEmail(existing.authorEmail, userEmail)) { try { diff --git a/templates/clips/actions/update-comment.ts b/templates/clips/actions/update-comment.ts index 4b1949ef0da..55fbb9021f6 100644 --- a/templates/clips/actions/update-comment.ts +++ b/templates/clips/actions/update-comment.ts @@ -37,7 +37,7 @@ export default defineAction({ .optional() .describe("Organization members mentioned in the updated comment"), }), - run: async (args) => { + run: async (args, ctx) => { const userEmail = getRequestUserEmail(); if (!userEmail) { throw new Error("Sign in required to edit comments."); @@ -51,7 +51,11 @@ export default defineAction({ .limit(1); if (!existing) throw new Error(`Comment not found: ${args.id}`); - await assertCommentAccess(existing.recordingId); + await assertCommentAccess( + existing.recordingId, + "write", + ctx?.requestHeaders, + ); if (!sameOwnerEmail(existing.authorEmail, userEmail)) { throw new ForbiddenError( diff --git a/templates/clips/app/components/player/comments-panel.test.tsx b/templates/clips/app/components/player/comments-panel.test.tsx index 7e15a60d8a5..299293a9910 100644 --- a/templates/clips/app/components/player/comments-panel.test.tsx +++ b/templates/clips/app/components/player/comments-panel.test.tsx @@ -26,6 +26,7 @@ const actionMocks = vi.hoisted(() => ({ updateComment: vi.fn(), otherMutation: vi.fn(), mutationOptions: new Map(), + requests: vi.fn(), })); vi.mock("@agent-native/core/client/hooks", () => ({ @@ -35,6 +36,11 @@ vi.mock("@agent-native/core/client/hooks", () => ({ actionMocks.mutationOptions.set(name, options); return { mutate: (vars: any) => { + const headers = + typeof options?.headers === "function" + ? options.headers() + : options?.headers; + actionMocks.requests(name, vars, headers); void options?.onMutate?.(vars); return ( name === "add-comment" @@ -196,11 +202,13 @@ describe("CommentsPanel reply composer", () => { comments: Comment[] = [rootComment], presentation: "default" | "share" | "inline" = "share", enableComments = true, + commentAccessToken?: string, ) { act(() => { root.render( { vi.clearAllMocks(); }); + it("supplies current password proof to every comment mutation after rotation", () => { + for (const token of ["fake-first-grant", "fake-rotated-grant", undefined]) { + renderPanel("viewer@example.com", [rootComment], "share", true, token); + for (const name of [ + "add-comment", + "react-to-comment", + "delete-comment", + "update-comment", + ]) { + const options = actionMocks.mutationOptions.get(name); + const headers = + typeof options?.headers === "function" + ? options.headers() + : options?.headers; + expect(headers?.["x-clips-comment-token"]).toBe(token); + } + } + }); + + it("sends rotated proof with a reaction without putting it in action arguments", () => { + for (const token of ["fake-first-grant", "fake-rotated-grant"]) { + renderPanel( + "viewer@example.com", + [ + { + ...rootComment, + emojiReactionsJson: JSON.stringify({ "👍": ["other@example.com"] }), + }, + ], + "share", + true, + token, + ); + const reaction = container.querySelector( + "button[aria-pressed]", + ); + expect(reaction).not.toBeNull(); + act(() => reaction?.click()); + expect(actionMocks.requests).toHaveBeenLastCalledWith( + "react-to-comment", + { commentId: "comment-1", emoji: "👍" }, + { "x-clips-comment-token": token }, + ); + } + }); + it("renders comment URLs as safe external links without including punctuation", () => { const absoluteUrl = container.querySelector( 'a[href="https://example.com/docs?item=1"]', diff --git a/templates/clips/app/components/player/comments-panel.tsx b/templates/clips/app/components/player/comments-panel.tsx index 77a77362479..0180616d1dd 100644 --- a/templates/clips/app/components/player/comments-panel.tsx +++ b/templates/clips/app/components/player/comments-panel.tsx @@ -45,6 +45,7 @@ import { } from "@/components/ui/popover"; import { cn } from "@/lib/utils"; +import { COMMENT_ACCESS_TOKEN_HEADER } from "../../../shared/comment-access"; import { displayCommentMentions, mentionsForCommentText, @@ -183,6 +184,7 @@ function updateReactionUsers( export interface CommentsPanelProps { recordingId: string; + commentAccessToken?: string; comments: Comment[]; currentMs: number; getCurrentMs?: () => number; @@ -404,6 +406,9 @@ export function CommentsPanel(props: CommentsPanelProps) { }; const addComment = useActionMutation("add-comment", { + headers: props.commentAccessToken + ? { [COMMENT_ACCESS_TOKEN_HEADER]: props.commentAccessToken } + : undefined, onMutate: async (vars: any) => { await queryClient.cancelQueries({ queryKey }); const tempId = makeTempId(); @@ -440,6 +445,9 @@ export function CommentsPanel(props: CommentsPanelProps) { }); const reactToComment = useActionMutation("react-to-comment", { + headers: props.commentAccessToken + ? { [COMMENT_ACCESS_TOKEN_HEADER]: props.commentAccessToken } + : undefined, onMutate: async (vars: any) => { await queryClient.cancelQueries({ queryKey }); const currentUser = currentUserEmail; @@ -513,6 +521,9 @@ export function CommentsPanel(props: CommentsPanelProps) { }); const remove = useActionMutation("delete-comment", { + headers: props.commentAccessToken + ? { [COMMENT_ACCESS_TOKEN_HEADER]: props.commentAccessToken } + : undefined, onMutate: async (vars: any) => { await queryClient.cancelQueries({ queryKey }); const current = visibleCommentsRef.current; @@ -548,6 +559,9 @@ export function CommentsPanel(props: CommentsPanelProps) { }); const updateComment = useActionMutation("update-comment", { + headers: props.commentAccessToken + ? { [COMMENT_ACCESS_TOKEN_HEADER]: props.commentAccessToken } + : undefined, onMutate: async (vars: any) => { await queryClient.cancelQueries({ queryKey }); const updatedAt = new Date().toISOString(); diff --git a/templates/clips/app/routes/share.$shareId.tsx b/templates/clips/app/routes/share.$shareId.tsx index 12275ae1d75..5ffc8e2e82f 100644 --- a/templates/clips/app/routes/share.$shareId.tsx +++ b/templates/clips/app/routes/share.$shareId.tsx @@ -2116,6 +2116,9 @@ export default function ShareRoute() { /> ) : ( { const access = await assertAccess("recording", recordingId, "viewer"); const recording = access.resource as { enableComments: boolean; password?: string | null; + sharePasswordVersion?: string | null; expiresAt?: string | null; trashedAt?: string | null; }; @@ -37,8 +44,7 @@ export async function assertCommentAccess( } if ( operation === "read" - ? (Boolean(recording.password) && access.role !== "owner") || - !canReadRecordingComments({ + ? !canReadRecordingComments({ enableComments: recording.enableComments, viewerIsOwner: access.role === "owner", }) @@ -46,4 +52,20 @@ export async function assertCommentAccess( ) { throw new ForbiddenError(); } + if (recording.password && access.role !== "owner") { + const token = requestHeaders?.get(COMMENT_ACCESS_TOKEN_HEADER); + if ( + !token || + !verifyShortLivedToken( + token, + getCommentAccessTokenResourceId( + recordingId, + recording.password, + recording.sharePasswordVersion, + ), + ).ok + ) { + throw new ForbiddenError(); + } + } } diff --git a/templates/clips/server/routes/api/public-recording.get.test.ts b/templates/clips/server/routes/api/public-recording.get.test.ts index de64be9d593..0afa331ebac 100644 --- a/templates/clips/server/routes/api/public-recording.get.test.ts +++ b/templates/clips/server/routes/api/public-recording.get.test.ts @@ -316,6 +316,67 @@ describe("/api/public-recording route", () => { }, ); + it.each([ + { mode: "password", expected: "comment-token" }, + { mode: "agent", expected: null }, + { mode: "owner", expected: null }, + { mode: "unprotected", expected: null }, + { mode: "missing", expected: undefined }, + { mode: "wrong", expected: undefined }, + ])( + "issues comment proof only after password verification: $mode", + async ({ mode, expected }) => { + const rec = makeRecording({ + enableComments: true, + sharePasswordVersion: "initial", + ...(mode === "unprotected" ? { password: null } : {}), + }); + mockGetDb.mockReturnValue( + createDbWithSelectResults([[rec], [], [], [], []]), + ); + mockSignShortLivedToken + .mockReset() + .mockImplementation(({ resourceId }) => + resourceId.startsWith("clips-comment:") + ? "comment-token" + : "media-token", + ); + if (mode === "agent") { + mockGetQuery.mockReturnValue({ + id: "rec-1", + agent_access: "agent-read-token", + }); + mockVerifyScopedAgentAccessToken.mockReturnValue({ ok: true }); + } + if (mode === "owner") { + mockGetSession.mockResolvedValue({ email: "owner@example.com" }); + mockResolveAccess.mockResolvedValue({ role: "owner", resource: rec }); + } + if (mode === "missing") mockGetQuery.mockReturnValue({ id: "rec-1" }); + if (mode === "wrong") mockVerifySharePassword.mockReturnValue(false); + const result = (await handler({ setCookies: [] } as any)) as any; + expect(result.recording?.commentAccessToken).toBe(expected); + const grants = mockSignShortLivedToken.mock.calls.filter(([claims]) => + claims.resourceId.startsWith("clips-comment:"), + ); + if (mode === "password") { + expect(grants).toEqual([ + [ + { + resourceId: + "clips-comment:rec-1:fb0b690c4db3c30649e97252a8748e8e6e6a7c00b0f01d68d7beffa54343f494", + ttlSeconds: 600, + }, + ], + ]); + expect(mockVerifySharePassword).toHaveBeenCalledWith( + "open-sesame", + "encrypted-password", + ); + } else expect(grants).toEqual([]); + }, + ); + it("sets a protected media cookie and long fallback token after password unlock", async () => { const event = { setCookies: [] as unknown[] }; mockGetDb.mockReturnValue( @@ -357,9 +418,22 @@ describe("/api/public-recording route", () => { }); it("reuses a valid protected-media token across public-share polling", async () => { + mockSignShortLivedToken + .mockReset() + .mockImplementation(({ resourceId }) => + resourceId.startsWith("clips-comment:") + ? "comment-token" + : "media-token", + ); const firstEvent = { setCookies: [] as unknown[] }; mockGetDb.mockReturnValue( - createDbWithSelectResults([[makeRecording()], [], [], [], []]), + createDbWithSelectResults([ + [makeRecording({ enableComments: true })], + [], + [], + [], + [], + ]), ); const firstResult = (await handler(firstEvent as any)) as any; @@ -372,7 +446,13 @@ describe("/api/public-recording route", () => { ); mockVerifyShortLivedToken.mockReturnValue({ ok: true }); mockGetDb.mockReturnValue( - createDbWithSelectResults([[makeRecording()], [], [], [], []]), + createDbWithSelectResults([ + [makeRecording({ enableComments: true })], + [], + [], + [], + [], + ]), ); const secondResult = (await handler(secondEvent as any)) as any; @@ -381,11 +461,21 @@ describe("/api/public-recording route", () => { firstResult.recording.videoUrl, ); expect(secondResult.recording.mediaAccessToken).toBe(firstToken); + expect(secondResult.recording.commentAccessToken).toBe("comment-token"); + expect( + mockSignShortLivedToken.mock.calls.filter(([claims]) => + claims.resourceId.startsWith("clips-comment:"), + ), + ).toHaveLength(2); expect(mockVerifyShortLivedToken).toHaveBeenCalledWith( firstToken, "rec-1:password-scoped", ); - expect(mockSignShortLivedToken).toHaveBeenCalledTimes(1); + expect( + mockSignShortLivedToken.mock.calls.filter( + ([claims]) => claims.resourceId === "rec-1:password-scoped", + ), + ).toHaveLength(1); }); it("does not inspect a cached media token until the password gate succeeds", async () => { diff --git a/templates/clips/server/routes/api/public-recording.get.ts b/templates/clips/server/routes/api/public-recording.get.ts index 556710af9ec..84753f27096 100644 --- a/templates/clips/server/routes/api/public-recording.get.ts +++ b/templates/clips/server/routes/api/public-recording.get.ts @@ -36,6 +36,7 @@ import { import { resolveTranscriptPresentation } from "../../../shared/transcript-status.js"; import { getDb, schema } from "../../db/index.js"; import { countRecordingAgentViews } from "../../lib/agent-views.js"; +import { getCommentAccessTokenResourceId } from "../../lib/comment-access-token.js"; import { isMediaVerificationPending } from "../../lib/media-verification-state.js"; import { isHeldForRedaction, @@ -314,6 +315,7 @@ export default defineEventHandler(async (event) => { } let protectedMediaToken: string | null = null; + let commentAccessToken: string | null = null; if (rec.password && !viewerIsOwner) { if (!tokenAllowsAgentAccess) { if (!password) { @@ -335,6 +337,16 @@ export default defineEventHandler(async (event) => { setResponseStatus(event, 401); return { error: "Password required", passwordRequired: true }; } + if (rec.enableComments) { + commentAccessToken = signShortLivedToken({ + resourceId: getCommentAccessTokenResourceId( + recordingId, + rec.password, + rec.sharePasswordVersion, + ), + ttlSeconds: 600, + }); + } } protectedMediaToken = setProtectedMediaAccessCookie( event, @@ -524,6 +536,7 @@ export default defineEventHandler(async (event) => { : rec.editsJson, videoUrl: playbackVideoUrl, mediaAccessToken: protectedMediaToken, + commentAccessToken, videoFormat: rec.videoFormat, videoSizeBytes: rec.videoSizeBytes ?? null, mediaUpdatedAt: rec.mediaUpdatedAt, diff --git a/templates/clips/shared/comment-access.ts b/templates/clips/shared/comment-access.ts index d7696a740d1..e4bdb2bb41b 100644 --- a/templates/clips/shared/comment-access.ts +++ b/templates/clips/shared/comment-access.ts @@ -1,3 +1,5 @@ +export const COMMENT_ACCESS_TOKEN_HEADER = "x-clips-comment-token"; + export function canReadRecordingComments(input: { enableComments: boolean; viewerIsOwner: boolean; From 6dfeb4d89f9126da0625e34811caec538a5720ef Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:14:14 -0400 Subject: [PATCH 02/12] Refresh comment access while a shared clip stays open --- .../hooks/use-comment-access-refresh.test.tsx | 46 +++++++++++++++++++ .../app/hooks/use-comment-access-refresh.ts | 13 ++++++ templates/clips/app/routes/share.$shareId.tsx | 2 + 3 files changed, 61 insertions(+) create mode 100644 templates/clips/app/hooks/use-comment-access-refresh.test.tsx create mode 100644 templates/clips/app/hooks/use-comment-access-refresh.ts diff --git a/templates/clips/app/hooks/use-comment-access-refresh.test.tsx b/templates/clips/app/hooks/use-comment-access-refresh.test.tsx new file mode 100644 index 00000000000..378db76009a --- /dev/null +++ b/templates/clips/app/hooks/use-comment-access-refresh.test.tsx @@ -0,0 +1,46 @@ +// @vitest-environment happy-dom +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, expect, it, vi } from "vitest"; + +import { useCommentAccessRefresh } from "./use-comment-access-refresh"; + +afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); +}); +it("refreshes password proof before expiry, rotates it, and stops when no longer unlocked", async () => { + vi.useFakeTimers(); + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + const refresh = vi.fn(); + const container = document.createElement("div"); + const root = createRoot(container); + function Probe({ token }: { token?: string }) { + useCommentAccessRefresh(token, refresh); + return null; + } + try { + act(() => root.render()); + await act(async () => { + await vi.advanceTimersByTimeAsync(299_999); + }); + expect(refresh).not.toHaveBeenCalled(); + await act(async () => { + await vi.advanceTimersByTimeAsync(1); + }); + expect(refresh).toHaveBeenCalledTimes(1); + act(() => root.render()); + await act(async () => { + await vi.advanceTimersByTimeAsync(300_000); + }); + expect(refresh).toHaveBeenCalledTimes(2); + act(() => root.render()); + await act(async () => { + await vi.advanceTimersByTimeAsync(600_000); + }); + expect(refresh).toHaveBeenCalledTimes(2); + } finally { + act(() => root.unmount()); + } + expect(vi.getTimerCount()).toBe(0); +}); diff --git a/templates/clips/app/hooks/use-comment-access-refresh.ts b/templates/clips/app/hooks/use-comment-access-refresh.ts new file mode 100644 index 00000000000..38a6b60116c --- /dev/null +++ b/templates/clips/app/hooks/use-comment-access-refresh.ts @@ -0,0 +1,13 @@ +import { useEffect } from "react"; + +export function useCommentAccessRefresh( + token: string | null | undefined, + refresh: () => Promise, +): void { + useEffect(() => { + if (!token) return; + // Finished shares stop media polling, but password comment proof expires in ten minutes. + const timer = setInterval(() => void refresh(), 5 * 60_000); + return () => clearInterval(timer); + }, [token, refresh]); +} diff --git a/templates/clips/app/routes/share.$shareId.tsx b/templates/clips/app/routes/share.$shareId.tsx index 5ffc8e2e82f..c28436e9009 100644 --- a/templates/clips/app/routes/share.$shareId.tsx +++ b/templates/clips/app/routes/share.$shareId.tsx @@ -118,6 +118,7 @@ import { TooltipContent, TooltipTrigger, } from "@/components/ui/tooltip"; +import { useCommentAccessRefresh } from "@/hooks/use-comment-access-refresh"; import { usePlayerShortcuts } from "@/hooks/use-player-shortcuts"; import { useSonnerLifecycleToast } from "@/hooks/use-sonner-lifecycle-toast"; import { useViewTracking } from "@/hooks/use-view-tracking"; @@ -833,6 +834,7 @@ export default function ShareRoute() { }); const recording = dataQ.data?.data?.recording; + useCommentAccessRefresh(recording?.commentAccessToken, dataQ.refetch); const hostedFailureCode = (recording as any)?.failureCode; const localizedHostedFailure = hostedFailureCode === "global_storage_cap" From 27bc31010a71c96261bc920fd8dce3fb54ecc7ed Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:17:34 -0400 Subject: [PATCH 03/12] Renew comment access when a shared clip resumes --- .../actions/comment-writing-access.test.ts | 35 +++++++++ .../components/player/comments-panel.test.tsx | 75 +++++++++++++++---- .../hooks/use-comment-access-refresh.test.tsx | 29 +++++++ .../app/hooks/use-comment-access-refresh.ts | 14 +++- 4 files changed, 135 insertions(+), 18 deletions(-) diff --git a/templates/clips/actions/comment-writing-access.test.ts b/templates/clips/actions/comment-writing-access.test.ts index 79706a79730..f10cd0ae188 100644 --- a/templates/clips/actions/comment-writing-access.test.ts +++ b/templates/clips/actions/comment-writing-access.test.ts @@ -357,3 +357,38 @@ it("replies retain the parent thread, organization, frame, and return contract", }), ]); }); + +it.each([undefined, null, "legacy:fake"])( + "binds legacy comment proof to the stored password at version %s", + async (sharePasswordVersion) => { + const resource = { ...protectedRecording, sharePasswordVersion }; + mockAssertAccess.mockResolvedValue({ role: "viewer", resource }); + const scope = createHash("sha256") + .update( + JSON.stringify([ + "recording-1", + resource.password, + sharePasswordVersion ?? null, + ]), + ) + .digest("hex"); + const token = signShortLivedToken({ + resourceId: `clips-comment:recording-1:${scope}`, + ttlSeconds: 600, + }); + const ctx = { + requestHeaders: new Headers({ "x-clips-comment-token": token }), + }; + const args = { + recordingId: "recording-1", + content: "Fake comment", + videoTimestampMs: 0, + }; + await expect(addComment.run(args, ctx)).resolves.toBeDefined(); + resource.password = "fake-rotated-password"; + await expect(addComment.run(args, ctx)).rejects.toBeInstanceOf( + ForbiddenError, + ); + expect(state.inserted).toHaveLength(1); + }, +); diff --git a/templates/clips/app/components/player/comments-panel.test.tsx b/templates/clips/app/components/player/comments-panel.test.tsx index 299293a9910..372a22c8153 100644 --- a/templates/clips/app/components/player/comments-panel.test.tsx +++ b/templates/clips/app/components/player/comments-panel.test.tsx @@ -13,6 +13,7 @@ import { vi, } from "vitest"; +import { useCommentAccessRefresh } from "../../hooks/use-comment-access-refresh"; import { collectCommentSubtreeIds, type Comment, @@ -264,20 +265,60 @@ describe("CommentsPanel reply composer", () => { } }); - it("sends rotated proof with a reaction without putting it in action arguments", () => { - for (const token of ["fake-first-grant", "fake-rotated-grant"]) { - renderPanel( - "viewer@example.com", - [ - { - ...rootComment, - emojiReactionsJson: JSON.stringify({ "👍": ["other@example.com"] }), - }, - ], - "share", - true, - token, + it.each(["active", "resumed"])( + "renews expired proof before a %s viewer submits", + async (mode) => { + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + }); + const refresh = vi.fn(); + function UnlockedShare() { + const [token, setToken] = React.useState("fake-initial-proof"); + const renew = React.useCallback(async () => { + refresh(); + setToken(`fake-renewed-proof-${refresh.mock.calls.length}`); + }, []); + useCommentAccessRefresh(token, renew); + return ( + + ); + } + act(() => + root.render( + + + , + ), ); + if (mode === "active") { + await act(async () => { + await vi.advanceTimersByTimeAsync(660_000); + }); + } else { + vi.setSystemTime(Date.now() + 660_000); + await act(async () => { + window.dispatchEvent(new Event("focus")); + }); + } + expect(refresh).toHaveBeenCalledTimes(mode === "active" ? 2 : 1); const reaction = container.querySelector( "button[aria-pressed]", ); @@ -286,10 +327,12 @@ describe("CommentsPanel reply composer", () => { expect(actionMocks.requests).toHaveBeenLastCalledWith( "react-to-comment", { commentId: "comment-1", emoji: "👍" }, - { "x-clips-comment-token": token }, + { + "x-clips-comment-token": `fake-renewed-proof-${mode === "active" ? 2 : 1}`, + }, ); - } - }); + }, + ); it("renders comment URLs as safe external links without including punctuation", () => { const absoluteUrl = container.querySelector( diff --git a/templates/clips/app/hooks/use-comment-access-refresh.test.tsx b/templates/clips/app/hooks/use-comment-access-refresh.test.tsx index 378db76009a..6226f263e6d 100644 --- a/templates/clips/app/hooks/use-comment-access-refresh.test.tsx +++ b/templates/clips/app/hooks/use-comment-access-refresh.test.tsx @@ -44,3 +44,32 @@ it("refreshes password proof before expiry, rotates it, and stops when no longer } expect(vi.getTimerCount()).toBe(0); }); + +it("refreshes when an unlocked tab becomes visible after suspension", async () => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + const refresh = vi.fn(async () => {}); + const container = document.createElement("div"); + const root = createRoot(container); + function Probe() { + useCommentAccessRefresh("fake-proof", refresh); + return null; + } + try { + act(() => root.render()); + Object.defineProperty(document, "visibilityState", { + value: "visible", + configurable: true, + }); + await act(async () => { + document.dispatchEvent(new Event("visibilitychange")); + }); + expect(refresh).toHaveBeenCalledTimes(1); + act(() => root.unmount()); + window.dispatchEvent(new Event("focus")); + document.dispatchEvent(new Event("visibilitychange")); + expect(refresh).toHaveBeenCalledTimes(1); + } finally { + act(() => root.unmount()); + delete (document as any).visibilityState; + } +}); diff --git a/templates/clips/app/hooks/use-comment-access-refresh.ts b/templates/clips/app/hooks/use-comment-access-refresh.ts index 38a6b60116c..ae32dc02ae4 100644 --- a/templates/clips/app/hooks/use-comment-access-refresh.ts +++ b/templates/clips/app/hooks/use-comment-access-refresh.ts @@ -6,8 +6,18 @@ export function useCommentAccessRefresh( ): void { useEffect(() => { if (!token) return; + const renew = () => void refresh(); + const renewWhenVisible = () => { + if (document.visibilityState === "visible") renew(); + }; // Finished shares stop media polling, but password comment proof expires in ten minutes. - const timer = setInterval(() => void refresh(), 5 * 60_000); - return () => clearInterval(timer); + const timer = setInterval(renew, 5 * 60_000); + window.addEventListener("focus", renew); + document.addEventListener("visibilitychange", renewWhenVisible); + return () => { + clearInterval(timer); + window.removeEventListener("focus", renew); + document.removeEventListener("visibilitychange", renewWhenVisible); + }; }, [token, refresh]); } From 581c765fa71a17661083ae7cf9a9fc8ec54bc688 Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:24:39 -0400 Subject: [PATCH 04/12] Coordinate comment writes with password access renewal --- .../components/player/comments-panel.test.tsx | 141 ++++++++++++++---- .../app/components/player/comments-panel.tsx | 37 +++-- templates/clips/app/routes/share.$shareId.tsx | 19 ++- 3 files changed, 159 insertions(+), 38 deletions(-) diff --git a/templates/clips/app/components/player/comments-panel.test.tsx b/templates/clips/app/components/player/comments-panel.test.tsx index 372a22c8153..a29d4b75d12 100644 --- a/templates/clips/app/components/player/comments-panel.test.tsx +++ b/templates/clips/app/components/player/comments-panel.test.tsx @@ -28,33 +28,40 @@ const actionMocks = vi.hoisted(() => ({ otherMutation: vi.fn(), mutationOptions: new Map(), requests: vi.fn(), + realMutations: false, })); -vi.mock("@agent-native/core/client/hooks", () => ({ - cn: (...classes: Array) => - classes.filter(Boolean).join(" "), - useActionMutation: (name: string, options?: any) => { - actionMocks.mutationOptions.set(name, options); - return { - mutate: (vars: any) => { - const headers = - typeof options?.headers === "function" - ? options.headers() - : options?.headers; - actionMocks.requests(name, vars, headers); - void options?.onMutate?.(vars); - return ( - name === "add-comment" - ? actionMocks.addComment - : name === "update-comment" - ? actionMocks.updateComment - : actionMocks.otherMutation - )(vars); - }, - }; - }, - useAvatarUrl: () => null, -})); +vi.mock("@agent-native/core/client/hooks", async (importOriginal) => { + const actual = + await importOriginal(); + return { + cn: (...classes: Array) => + classes.filter(Boolean).join(" "), + useActionMutation: (name: string, options?: any) => { + if (actionMocks.realMutations) + return actual.useActionMutation(name as any, options); + actionMocks.mutationOptions.set(name, options); + return { + mutate: (vars: any) => { + const headers = + typeof options?.headers === "function" + ? options.headers() + : options?.headers; + actionMocks.requests(name, vars, headers); + void options?.onMutate?.(vars); + return ( + name === "add-comment" + ? actionMocks.addComment + : name === "update-comment" + ? actionMocks.updateComment + : actionMocks.otherMutation + )(vars); + }, + }; + }, + useAvatarUrl: () => null, + }; +}); vi.mock("@agent-native/core/client/i18n", () => ({ useT: () => (key: string) => key, @@ -265,6 +272,90 @@ describe("CommentsPanel reply composer", () => { } }); + it("waits for pending password renewal before cancelling queries or sending a mutation", async () => { + act(() => root.render(null)); + actionMocks.realMutations = true; + onTestFinished(() => { + actionMocks.realMutations = false; + }); + let completeRenewal!: (value: { token: string }) => void; + const deferred = new Promise<{ token: string }>((resolve) => { + completeRenewal = resolve; + }); + const cancel = vi.spyOn(queryClient, "cancelQueries"); + const fetch = vi.fn( + async () => + new Response(JSON.stringify({ id: "comment-1" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + vi.stubGlobal("fetch", fetch); + const renew = () => + queryClient + .fetchQuery({ + queryKey: ["recording", "recording-1"], + queryFn: () => deferred, + }) + .catch(() => null); + function PendingShare() { + useCommentAccessRefresh("fake-expired-proof", renew); + return ( + (await renew())?.token} + /> + ); + } + act(() => + root.render( + + + , + ), + ); + await act(async () => { + window.dispatchEvent(new Event("focus")); + }); + const reaction = container.querySelector( + "button[aria-pressed]", + ); + expect(reaction).not.toBeNull(); + await act(async () => { + reaction?.click(); + }); + expect(cancel).not.toHaveBeenCalled(); + expect(fetch).not.toHaveBeenCalled(); + await act(async () => { + completeRenewal({ token: "fake-fresh-proof" }); + }); + expect(cancel).toHaveBeenCalled(); + expect(fetch).toHaveBeenCalledTimes(1); + const init = fetch.mock.calls[0][1] as RequestInit; + expect(new Headers(init.headers).get("x-clips-comment-token")).toBe( + "fake-fresh-proof", + ); + expect(JSON.parse(String(init.body))).toEqual({ + commentId: "comment-1", + emoji: "👍", + }); + }); + it.each(["active", "resumed"])( "renews expired proof before a %s viewer submits", async (mode) => { diff --git a/templates/clips/app/components/player/comments-panel.tsx b/templates/clips/app/components/player/comments-panel.tsx index 0180616d1dd..00f10a442f3 100644 --- a/templates/clips/app/components/player/comments-panel.tsx +++ b/templates/clips/app/components/player/comments-panel.tsx @@ -185,6 +185,7 @@ function updateReactionUsers( export interface CommentsPanelProps { recordingId: string; commentAccessToken?: string; + getCommentAccessToken?: () => Promise; comments: Comment[]; currentMs: number; getCurrentMs?: () => number; @@ -217,6 +218,14 @@ export function CommentsPanel(props: CommentsPanelProps) { applyComments = defaultLens.applyComments, presentation = "default", } = props; + const commentAccessTokenRef = useRef(props.commentAccessToken); + useEffect(() => { + commentAccessTokenRef.current = props.commentAccessToken; + }, [props.commentAccessToken]); + const commentHeaders = () => + commentAccessTokenRef.current + ? { [COMMENT_ACCESS_TOKEN_HEADER]: commentAccessTokenRef.current } + : undefined; const isSignedIn = !!currentUserEmail; const isSharePresentation = presentation === "share"; const isInlinePresentation = presentation === "inline"; @@ -406,10 +415,11 @@ export function CommentsPanel(props: CommentsPanelProps) { }; const addComment = useActionMutation("add-comment", { - headers: props.commentAccessToken - ? { [COMMENT_ACCESS_TOKEN_HEADER]: props.commentAccessToken } - : undefined, + headers: commentHeaders, onMutate: async (vars: any) => { + if (props.getCommentAccessToken) { + commentAccessTokenRef.current = await props.getCommentAccessToken(); + } await queryClient.cancelQueries({ queryKey }); const tempId = makeTempId(); const now = new Date().toISOString(); @@ -445,10 +455,11 @@ export function CommentsPanel(props: CommentsPanelProps) { }); const reactToComment = useActionMutation("react-to-comment", { - headers: props.commentAccessToken - ? { [COMMENT_ACCESS_TOKEN_HEADER]: props.commentAccessToken } - : undefined, + headers: commentHeaders, onMutate: async (vars: any) => { + if (props.getCommentAccessToken) { + commentAccessTokenRef.current = await props.getCommentAccessToken(); + } await queryClient.cancelQueries({ queryKey }); const currentUser = currentUserEmail; if (!currentUser) { @@ -521,10 +532,11 @@ export function CommentsPanel(props: CommentsPanelProps) { }); const remove = useActionMutation("delete-comment", { - headers: props.commentAccessToken - ? { [COMMENT_ACCESS_TOKEN_HEADER]: props.commentAccessToken } - : undefined, + headers: commentHeaders, onMutate: async (vars: any) => { + if (props.getCommentAccessToken) { + commentAccessTokenRef.current = await props.getCommentAccessToken(); + } await queryClient.cancelQueries({ queryKey }); const current = visibleCommentsRef.current; const target = current.find((comment) => comment.id === vars.id); @@ -559,10 +571,11 @@ export function CommentsPanel(props: CommentsPanelProps) { }); const updateComment = useActionMutation("update-comment", { - headers: props.commentAccessToken - ? { [COMMENT_ACCESS_TOKEN_HEADER]: props.commentAccessToken } - : undefined, + headers: commentHeaders, onMutate: async (vars: any) => { + if (props.getCommentAccessToken) { + commentAccessTokenRef.current = await props.getCommentAccessToken(); + } await queryClient.cancelQueries({ queryKey }); const updatedAt = new Date().toISOString(); const current = visibleCommentsRef.current.find( diff --git a/templates/clips/app/routes/share.$shareId.tsx b/templates/clips/app/routes/share.$shareId.tsx index c28436e9009..ddd22d79149 100644 --- a/templates/clips/app/routes/share.$shareId.tsx +++ b/templates/clips/app/routes/share.$shareId.tsx @@ -834,7 +834,19 @@ export default function ShareRoute() { }); const recording = dataQ.data?.data?.recording; - useCommentAccessRefresh(recording?.commentAccessToken, dataQ.refetch); + const refreshCommentAccess = useCallback( + () => dataQ.refetch({ cancelRefetch: false }), + [dataQ.refetch], + ); + const getCommentAccessToken = useCallback(async () => { + const result = await refreshCommentAccess(); + if (result.error) throw result.error; + if (!result.data?.ok) { + throw new Error(result.data?.data?.error ?? String(result.data?.status)); + } + return result.data.data.recording?.commentAccessToken; + }, [refreshCommentAccess]); + useCommentAccessRefresh(recording?.commentAccessToken, refreshCommentAccess); const hostedFailureCode = (recording as any)?.failureCode; const localizedHostedFailure = hostedFailureCode === "global_storage_cap" @@ -2118,6 +2130,11 @@ export default function ShareRoute() { /> ) : ( Date: Fri, 23 Oct 2026 02:25:35 -0400 Subject: [PATCH 05/12] Align comment renewal types with optional access proof --- templates/clips/app/components/player/comments-panel.test.tsx | 2 +- templates/clips/app/components/player/comments-panel.tsx | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/templates/clips/app/components/player/comments-panel.test.tsx b/templates/clips/app/components/player/comments-panel.test.tsx index a29d4b75d12..c14d3288378 100644 --- a/templates/clips/app/components/player/comments-panel.test.tsx +++ b/templates/clips/app/components/player/comments-panel.test.tsx @@ -283,7 +283,7 @@ describe("CommentsPanel reply composer", () => { completeRenewal = resolve; }); const cancel = vi.spyOn(queryClient, "cancelQueries"); - const fetch = vi.fn( + const fetch = vi.fn( async () => new Response(JSON.stringify({ id: "comment-1" }), { status: 200, diff --git a/templates/clips/app/components/player/comments-panel.tsx b/templates/clips/app/components/player/comments-panel.tsx index 00f10a442f3..7bf36058e79 100644 --- a/templates/clips/app/components/player/comments-panel.tsx +++ b/templates/clips/app/components/player/comments-panel.tsx @@ -218,7 +218,9 @@ export function CommentsPanel(props: CommentsPanelProps) { applyComments = defaultLens.applyComments, presentation = "default", } = props; - const commentAccessTokenRef = useRef(props.commentAccessToken); + const commentAccessTokenRef = useRef( + props.commentAccessToken, + ); useEffect(() => { commentAccessTokenRef.current = props.commentAccessToken; }, [props.commentAccessToken]); From 7cfeb74cd2f9d021c1876f0767ec6345bb82146d Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:28:46 -0400 Subject: [PATCH 06/12] Carry comment access through the fullscreen composer --- .../components/player/comments-panel.test.tsx | 54 +++++++++++++++++++ .../player/timestamped-comment-button.tsx | 23 +++++++- templates/clips/app/routes/share.$shareId.tsx | 8 +++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/templates/clips/app/components/player/comments-panel.test.tsx b/templates/clips/app/components/player/comments-panel.test.tsx index c14d3288378..38fb1fceed0 100644 --- a/templates/clips/app/components/player/comments-panel.test.tsx +++ b/templates/clips/app/components/player/comments-panel.test.tsx @@ -272,6 +272,60 @@ describe("CommentsPanel reply composer", () => { } }); + it("refreshes proof before a fullscreen comment sends", async () => { + act(() => root.render(null)); + actionMocks.realMutations = true; + onTestFinished(() => { + actionMocks.realMutations = false; + }); + let completeRenewal!: (token: string) => void; + const renewal = new Promise((resolve) => { + completeRenewal = resolve; + }); + const fetch = vi.fn( + async () => + new Response(JSON.stringify({ id: "comment-1" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + vi.stubGlobal("fetch", fetch); + act(() => + root.render( + + renewal} + /> + , + ), + ); + const send = + container.querySelector("button[aria-label]"); + expect(send).not.toBeNull(); + await act(async () => { + send?.click(); + }); + expect(fetch).not.toHaveBeenCalled(); + await act(async () => { + completeRenewal("fake-fresh-proof"); + }); + expect(fetch).toHaveBeenCalledTimes(1); + const init = fetch.mock.calls[0][1] as RequestInit; + expect(new Headers(init.headers).get("x-clips-comment-token")).toBe( + "fake-fresh-proof", + ); + expect(JSON.parse(String(init.body))).toEqual({ + recordingId: "recording-1", + content: "Fake fullscreen comment", + videoTimestampMs: 12000, + }); + }); + it("waits for pending password renewal before cancelling queries or sending a mutation", async () => { act(() => root.render(null)); actionMocks.realMutations = true; diff --git a/templates/clips/app/components/player/timestamped-comment-button.tsx b/templates/clips/app/components/player/timestamped-comment-button.tsx index 61447c33e62..f8f2bf8a96d 100644 --- a/templates/clips/app/components/player/timestamped-comment-button.tsx +++ b/templates/clips/app/components/player/timestamped-comment-button.tsx @@ -7,6 +7,7 @@ import { Button } from "@/components/ui/button"; import { Textarea } from "@/components/ui/textarea"; import { cn } from "@/lib/utils"; +import { COMMENT_ACCESS_TOKEN_HEADER } from "../../../shared/comment-access"; import { useAutoResizeCommentTextarea } from "./comment-composer"; import { msToClock } from "./scrubber"; @@ -40,6 +41,8 @@ export function TimestampedCommentButton({ } interface TimestampedCommentBarProps { + commentAccessToken?: string; + getCommentAccessToken?: () => Promise; recordingId: string; atMs: number; onClose: () => void; @@ -50,6 +53,8 @@ interface TimestampedCommentBarProps { } export function TimestampedCommentBar({ + commentAccessToken, + getCommentAccessToken, recordingId, atMs, onClose, @@ -65,7 +70,23 @@ export function TimestampedCommentBar({ const textareaRef = useRef(null); useAutoResizeCommentTextarea(textareaRef, draft, 1); - const addComment = useActionMutation("add-comment"); + const commentAccessTokenRef = useRef( + commentAccessToken, + ); + useEffect(() => { + commentAccessTokenRef.current = commentAccessToken; + }, [commentAccessToken]); + const addComment = useActionMutation("add-comment", { + headers: () => + commentAccessTokenRef.current + ? { [COMMENT_ACCESS_TOKEN_HEADER]: commentAccessTokenRef.current } + : undefined, + onMutate: async () => { + if (getCommentAccessToken) { + commentAccessTokenRef.current = await getCommentAccessToken(); + } + }, + }); useEffect(() => { requestAnimationFrame(() => textareaRef.current?.focus()); diff --git a/templates/clips/app/routes/share.$shareId.tsx b/templates/clips/app/routes/share.$shareId.tsx index ddd22d79149..c673478ef40 100644 --- a/templates/clips/app/routes/share.$shareId.tsx +++ b/templates/clips/app/routes/share.$shareId.tsx @@ -1860,6 +1860,14 @@ export default function ShareRoute() { ? (() => { const composer = ( Date: Fri, 23 Oct 2026 04:07:59 -0400 Subject: [PATCH 07/12] Keep comment renewal feedback responsive --- .../components/player/comments-panel.test.tsx | 679 +++++++++++++++++- .../app/components/player/comments-panel.tsx | 270 +++++-- .../player/timestamped-comment-button.tsx | 28 +- .../hooks/use-comment-access-refresh.test.tsx | 95 ++- .../app/hooks/use-comment-access-refresh.ts | 69 +- templates/clips/app/routes/share.$shareId.tsx | 16 +- 6 files changed, 1058 insertions(+), 99 deletions(-) diff --git a/templates/clips/app/components/player/comments-panel.test.tsx b/templates/clips/app/components/player/comments-panel.test.tsx index 38fb1fceed0..de991a2a476 100644 --- a/templates/clips/app/components/player/comments-panel.test.tsx +++ b/templates/clips/app/components/player/comments-panel.test.tsx @@ -1,6 +1,10 @@ // @vitest-environment happy-dom -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + QueryClient, + QueryClientProvider, + useQuery, +} from "@tanstack/react-query"; import React, { act } from "react"; import { createRoot, type Root } from "react-dom/client"; import { @@ -13,7 +17,10 @@ import { vi, } from "vitest"; -import { useCommentAccessRefresh } from "../../hooks/use-comment-access-refresh"; +import { + useCommentAccessRefresh, + useCommentAccessToken, +} from "../../hooks/use-comment-access-refresh"; import { collectCommentSubtreeIds, type Comment, @@ -200,6 +207,51 @@ async function openMenu(trigger: HTMLButtonElement | null) { }); } +const fakeShareKey = ["public-recording", "fake-share"]; + +function fakeShare(commentAccessToken: string, comments: Comment[]) { + return { + ok: true, + status: 200, + data: { recording: { commentAccessToken }, comments }, + }; +} + +function ProofShare({ + shareFetch, + getCurrentMs, +}: { + shareFetch: (context: { + signal: AbortSignal; + }) => Promise>; + getCurrentMs?: () => number; +}) { + const shareQ = useQuery({ queryKey: fakeShareKey, queryFn: shareFetch }); + const getCommentAccessToken = useCommentAccessToken(shareQ, fakeShareKey); + const share = shareQ.data?.data; + if (!share) return null; + return ( + data?.data?.comments} + applyComments={(data: any, next) => + data ? { ...data, data: { ...data.data, comments: next } } : data + } + commentAccessToken={share.recording.commentAccessToken} + getCommentAccessToken={getCommentAccessToken} + presentation="share" + /> + ); +} + describe("CommentsPanel reply composer", () => { let container: HTMLDivElement; let root: Root; @@ -253,6 +305,25 @@ describe("CommentsPanel reply composer", () => { vi.clearAllMocks(); }); + async function renderProofShare( + shareFetch: Parameters[0]["shareFetch"], + getCurrentMs?: () => number, + ) { + act(() => root.render(null)); + actionMocks.realMutations = true; + onTestFinished(() => { + actionMocks.realMutations = false; + }); + await act(async () => { + root.render( + + + , + ); + }); + await act(async () => new Promise((resolve) => setTimeout(resolve, 0))); + } + it("supplies current password proof to every comment mutation after rotation", () => { for (const token of ["fake-first-grant", "fake-rotated-grant", undefined]) { renderPanel("viewer@example.com", [rootComment], "share", true, token); @@ -410,6 +481,610 @@ describe("CommentsPanel reply composer", () => { }); }); + it.each(["renewed", "rejected"] as const)( + "shows a comment at once while stale proof renews, then sends only %s proof", + async (outcome) => { + act(() => root.render(null)); + actionMocks.realMutations = true; + onTestFinished(() => { + actionMocks.realMutations = false; + }); + const shareKey = ["public-recording", "fake-share"]; + let settleRenewal!: () => void; + const shareFetches = [ + async () => ({ token: "fake-stale-proof", comments: [rootComment] }), + () => + new Promise<{ token: string; comments: Comment[] }>( + (resolve, reject) => { + settleRenewal = () => + outcome === "renewed" + ? resolve({ + token: "fake-renewed-proof", + comments: [rootComment], + }) + : reject(new Error("fake renewal failure")); + }, + ), + ]; + const fetch = vi.fn( + async () => + new Response( + JSON.stringify({ id: "comment-2", threadId: "comment-2" }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ), + ); + vi.stubGlobal("fetch", fetch); + function StaleShare() { + const shareQ = useQuery({ + queryKey: shareKey, + queryFn: () => shareFetches.shift()!(), + }); + if (!shareQ.data) return null; + return ( + data?.comments} + applyComments={(data: any, next) => + data ? { ...data, comments: next } : data + } + commentAccessToken={shareQ.data.token} + getCommentAccessToken={async () => { + const result = await shareQ.refetch({ cancelRefetch: false }); + if (result.error) throw result.error; + return result.data?.token; + }} + presentation="share" + /> + ); + } + await act(async () => { + root.render( + + + , + ); + }); + await act(async () => new Promise((resolve) => setTimeout(resolve, 0))); + const composer = container.querySelector( + 'textarea[placeholder="commentsPanel.leaveComment"]', + ); + expect(composer).not.toBeNull(); + act(() => setTextareaValue(composer!, "Fake stale-proof comment")); + await act(async () => { + composer?.dispatchEvent( + new KeyboardEvent("keydown", { + bubbles: true, + ctrlKey: true, + key: "Enter", + }), + ); + }); + const feed = () => + Array.from(container.querySelectorAll("li")) + .map((item) => item.textContent) + .join(" "); + expect(feed()).toContain("Fake stale-proof comment"); + expect(fetch).not.toHaveBeenCalled(); + + await act(async () => { + settleRenewal(); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + if (outcome === "rejected") { + expect(feed()).not.toContain("Fake stale-proof comment"); + expect(fetch).not.toHaveBeenCalled(); + return; + } + expect(feed()).toContain("Fake stale-proof comment"); + expect(fetch).toHaveBeenCalledTimes(1); + const init = fetch.mock.calls[0][1] as RequestInit; + expect(new Headers(init.headers).get("x-clips-comment-token")).toBe( + "fake-renewed-proof", + ); + }, + ); + + it("sends every comment write on a fresh grant at once and renews only after a refusal", async () => { + const ownComment: Comment = { + ...rootComment, + authorEmail: "viewer@example.com", + emojiReactionsJson: JSON.stringify({ "👍": ["other@example.com"] }), + }; + const shareFetch = vi.fn(async () => + fakeShare(`fake-proof-${shareFetch.mock.calls.length}`, [ownComment]), + ); + const responses: Array<(response: Response) => void> = []; + const fetch = vi.fn( + () => new Promise((resolve) => responses.push(resolve)), + ); + vi.stubGlobal("fetch", fetch); + const flush = () => + act(async () => new Promise((resolve) => setTimeout(resolve, 0))); + await renderProofShare(shareFetch); + const feed = () => + Array.from(container.querySelectorAll("li")) + .map((item) => item.textContent) + .join(" "); + const sent = (index: number) => { + const init = fetch.mock.calls[index]?.[1] as RequestInit | undefined; + return { + token: new Headers(init?.headers).get("x-clips-comment-token"), + body: JSON.parse(String(init?.body)), + }; + }; + const composer = () => + container.querySelector( + 'textarea[placeholder="commentsPanel.leaveComment"]', + ); + const submitComment = async (content: string) => { + act(() => setTextareaValue(composer()!, content)); + await act(async () => { + composer()?.dispatchEvent( + new KeyboardEvent("keydown", { + bubbles: true, + ctrlKey: true, + key: "Enter", + }), + ); + }); + }; + const chooseMenuItem = async (label: string) => { + await openMenu( + container.querySelector('[aria-haspopup="menu"]'), + ); + const item = Array.from( + document.body.querySelectorAll('[role="menuitem"]'), + ).find((candidate) => candidate.textContent?.trim() === label); + expect(item).toBeDefined(); + await act(async () => { + item?.click(); + await Promise.resolve(); + }); + }; + + await submitComment("Fake fresh-proof comment"); + expect(feed()).toContain("Fake fresh-proof comment"); + expect(sent(0)).toMatchObject({ + token: "fake-proof-1", + body: { content: "Fake fresh-proof comment" }, + }); + + await act(async () => { + container + .querySelector("button[aria-pressed]") + ?.click(); + }); + expect(feed()).toContain("👍 2"); + expect(sent(1)).toMatchObject({ + token: "fake-proof-1", + body: { commentId: "comment-1", emoji: "👍" }, + }); + + await chooseMenuItem("commentsPanel.editComment"); + act(() => + setTextareaValue( + container.querySelector( + 'textarea[aria-label="commentsPanel.editComment"]', + )!, + "Fake fresh-proof edit", + ), + ); + await act(async () => { + Array.from(container.querySelectorAll("button")) + .find((button) => button.textContent?.trim() === "common.save") + ?.click(); + }); + expect(feed()).toContain("Fake fresh-proof edit"); + expect(sent(2)).toMatchObject({ + token: "fake-proof-1", + body: { id: "comment-1", content: "Fake fresh-proof edit" }, + }); + + await chooseMenuItem("commentsPanel.delete"); + expect(feed()).not.toContain("Fake fresh-proof edit"); + expect(sent(3)).toMatchObject({ + token: "fake-proof-1", + body: { id: "comment-1" }, + }); + expect(shareFetch).toHaveBeenCalledTimes(1); + + await act(async () => { + responses[0]( + new Response(JSON.stringify({ error: "Forbidden" }), { + status: 403, + headers: { "Content-Type": "application/json" }, + }), + ); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + await submitComment("Fake retried comment"); + await flush(); + expect(shareFetch).toHaveBeenCalledTimes(2); + expect(sent(4)).toMatchObject({ + token: "fake-proof-2", + body: { content: "Fake retried comment" }, + }); + }); + + it("joins a proof renewal already in flight instead of cancelling it", async () => { + let finishRenewal!: () => void; + let renewalAborted = false; + const shareFetch = vi.fn(async ({ signal }: { signal: AbortSignal }) => { + const proof = `fake-proof-${shareFetch.mock.calls.length}`; + if (proof !== "fake-proof-1") { + signal.addEventListener("abort", () => { + renewalAborted = true; + }); + await new Promise((resolve) => { + finishRenewal = resolve; + }); + } + return fakeShare(proof, [rootComment]); + }); + const fetch = vi.fn( + async () => + new Response( + JSON.stringify({ id: "comment-2", threadId: "comment-2" }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ), + ); + vi.stubGlobal("fetch", fetch); + await renderProofShare(shareFetch); + await act(async () => { + window.dispatchEvent(new Event("focus")); + }); + expect(shareFetch).toHaveBeenCalledTimes(2); + + const composer = container.querySelector( + 'textarea[placeholder="commentsPanel.leaveComment"]', + ); + act(() => setTextareaValue(composer!, "Fake comment during renewal")); + await act(async () => { + container + .querySelector( + 'button[aria-label="commentsPanel.commentButton"]', + ) + ?.click(); + }); + expect(container.querySelectorAll("li")).toHaveLength(2); + expect(fetch).not.toHaveBeenCalled(); + + await act(async () => { + finishRenewal(); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + expect(renewalAborted).toBe(false); + expect(shareFetch).toHaveBeenCalledTimes(2); + expect(fetch).toHaveBeenCalledTimes(1); + const init = fetch.mock.calls[0][1] as RequestInit; + expect(new Headers(init.headers).get("x-clips-comment-token")).toBe( + "fake-proof-2", + ); + }); + + it.each(["comment", "reply"] as const)( + "returns a failed %s to its composer with feedback when stale proof cannot renew", + async (kind) => { + vi.useFakeTimers({ toFake: ["Date"] }); + onTestFinished(() => { + vi.useRealTimers(); + }); + let playheadMs = 34_000; + let settleRenewal!: (renewed: boolean) => void; + const shareFetch = vi.fn(async () => { + const proof = `fake-proof-${shareFetch.mock.calls.length}`; + if (proof !== "fake-proof-1") { + await new Promise((resolve, reject) => { + settleRenewal = (renewed) => + renewed ? resolve() : reject(new Error("fake renewal failure")); + }); + } + return fakeShare(proof, [rootComment]); + }); + const fetch = vi.fn( + async () => + new Response( + JSON.stringify({ id: "comment-2", threadId: "thread-1" }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ), + ); + vi.stubGlobal("fetch", fetch); + await renderProofShare(shareFetch, () => playheadMs); + if (kind === "reply") { + await act(async () => { + Array.from(container.querySelectorAll("button")) + .find( + (button) => button.textContent?.trim() === "commentsPanel.reply", + ) + ?.click(); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + } + const label = + kind === "reply" + ? "commentsPanel.writeReply" + : "commentsPanel.leaveComment"; + const composer = () => + container.querySelector( + `textarea[placeholder="${label}"]`, + ); + const send = async () => { + await act(async () => { + container + .querySelector( + `button[aria-label="${ + kind === "reply" + ? "commentsPanel.writeReply" + : "commentsPanel.commentButton" + }"]`, + ) + ?.click(); + }); + }; + act(() => { + setTextareaValue(composer()!, "Hi @"); + composer()!.setSelectionRange(4, 4); + composer()!.dispatchEvent( + new KeyboardEvent("keyup", { bubbles: true, key: "@" }), + ); + }); + await act(async () => { + document.body + .querySelector('button[role="option"]') + ?.click(); + await Promise.resolve(); + }); + expect(composer()?.value).toBe("Hi @Member "); + playheadMs = 50_000; + vi.setSystemTime(Date.now() + 300_000); + + await send(); + expect(container.querySelectorAll("li")).toHaveLength(2); + expect(composer()?.value ?? "").toBe(""); + expect(shareFetch).toHaveBeenCalledTimes(2); + await act(async () => { + settleRenewal(false); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + expect(container.querySelectorAll("li")).toHaveLength(1); + expect(fetch).not.toHaveBeenCalled(); + expect(composer()?.value).toBe("Hi @Member "); + expect(container.querySelector('[role="alert"]')?.textContent).toBe( + "notificationsRoute.sendFailed", + ); + + await send(); + await act(async () => { + settleRenewal(true); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + expect(container.querySelector('[role="alert"]')).toBeNull(); + expect(fetch).toHaveBeenCalledTimes(1); + const init = fetch.mock.calls[0][1] as RequestInit; + expect(new Headers(init.headers).get("x-clips-comment-token")).toBe( + "fake-proof-3", + ); + expect(JSON.parse(String(init.body))).toMatchObject({ + content: "Hi @Member", + mentions: [{ email: "member@example.com", name: "Member" }], + ...(kind === "reply" + ? { + videoTimestampMs: 12_000, + threadId: "thread-1", + parentId: "comment-1", + } + : { videoTimestampMs: 34_000 }), + }); + }, + ); + + it("keeps newer typing when a send fails behind it", async () => { + vi.useFakeTimers({ toFake: ["Date"] }); + onTestFinished(() => { + vi.useRealTimers(); + }); + let failRenewal!: () => void; + const shareFetch = vi.fn(async () => { + const proof = `fake-proof-${shareFetch.mock.calls.length}`; + if (proof !== "fake-proof-1") { + await new Promise((_resolve, reject) => { + failRenewal = () => reject(new Error("fake renewal failure")); + }); + } + return fakeShare(proof, [rootComment]); + }); + const fetch = vi.fn(); + vi.stubGlobal("fetch", fetch); + await renderProofShare(shareFetch); + const composer = container.querySelector( + 'textarea[placeholder="commentsPanel.leaveComment"]', + ); + act(() => setTextareaValue(composer!, "Fake failed comment")); + vi.setSystemTime(Date.now() + 300_000); + await act(async () => { + container + .querySelector( + 'button[aria-label="commentsPanel.commentButton"]', + ) + ?.click(); + }); + act(() => setTextareaValue(composer!, "Fake newer draft")); + await act(async () => { + failRenewal(); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + expect(composer?.value).toBe("Fake newer draft"); + expect(container.querySelector('[role="alert"]')?.textContent).toBe( + "notificationsRoute.sendFailed", + ); + expect(fetch).not.toHaveBeenCalled(); + }); + + it.each([ + { otherReply: "kept open", restored: false }, + { otherReply: "cancelled", restored: true }, + ])( + "restores a failed reply only while no other comment's reply composer is open ($otherReply)", + async ({ otherReply, restored }) => { + vi.useFakeTimers({ toFake: ["Date"] }); + onTestFinished(() => { + vi.useRealTimers(); + }); + const otherComment: Comment = { + ...rootComment, + id: "comment-2", + threadId: "thread-2", + content: "Fake second thread", + videoTimestampMs: 20_000, + }; + let failRenewal!: () => void; + const shareFetch = vi.fn(async () => { + const proof = `fake-proof-${shareFetch.mock.calls.length}`; + if (proof !== "fake-proof-1") { + await new Promise((_resolve, reject) => { + failRenewal = () => reject(new Error("fake renewal failure")); + }); + } + return fakeShare(proof, [rootComment, otherComment]); + }); + const fetch = vi.fn(); + vi.stubGlobal("fetch", fetch); + await renderProofShare(shareFetch); + const openReply = async (content: string) => { + const card = Array.from(container.querySelectorAll("li")).find((item) => + item.firstElementChild?.textContent?.includes(content), + )?.firstElementChild; + await act(async () => { + Array.from(card?.querySelectorAll("button") ?? []) + .find( + (button) => button.textContent?.trim() === "commentsPanel.reply", + ) + ?.click(); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + }; + const replyComposer = () => + container.querySelector( + 'textarea[placeholder="commentsPanel.writeReply"]', + ); + const replyingTo = () => + replyComposer()?.closest("li")?.firstElementChild?.textContent; + + await openReply("Please take a look"); + act(() => setTextareaValue(replyComposer()!, "Fake failed reply")); + vi.setSystemTime(Date.now() + 300_000); + await act(async () => { + container + .querySelector( + 'button[aria-label="commentsPanel.writeReply"]', + ) + ?.click(); + }); + expect(container.textContent).toContain("Fake failed reply"); + expect(replyComposer()).toBeNull(); + + await openReply("Fake second thread"); + expect(replyingTo()).toContain("Fake second thread"); + if (otherReply === "cancelled") { + act(() => { + replyComposer()?.dispatchEvent( + new KeyboardEvent("keydown", { bubbles: true, key: "Escape" }), + ); + }); + expect(replyComposer()).toBeNull(); + } + await act(async () => { + failRenewal(); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + expect(fetch).not.toHaveBeenCalled(); + expect(replyingTo()).toContain( + restored ? "Please take a look" : "Fake second thread", + ); + expect(replyComposer()?.value).toBe(restored ? "Fake failed reply" : ""); + expect(container.querySelector('[role="alert"]')?.textContent).toBe( + "notificationsRoute.sendFailed", + ); + }, + ); + + it("keeps a fullscreen draft open with feedback when proof cannot renew, then sends it on retry", async () => { + act(() => root.render(null)); + actionMocks.realMutations = true; + onTestFinished(() => { + actionMocks.realMutations = false; + }); + const fetch = vi.fn( + async () => + new Response(JSON.stringify({ id: "comment-2" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + vi.stubGlobal("fetch", fetch); + const getCommentAccessToken = vi + .fn<(refusedToken?: string | null) => Promise>() + .mockRejectedValueOnce(new Error("fake password changed")) + .mockResolvedValueOnce("fake-renewed-proof"); + const onAdded = vi.fn(); + const onClose = vi.fn(); + const onDraftChange = vi.fn(); + act(() => + root.render( + + + , + ), + ); + const send = + container.querySelector("button[aria-label]"); + await act(async () => { + send?.click(); + }); + expect(container.querySelector('[role="alert"]')?.textContent).toBe( + "notificationsRoute.sendFailed", + ); + expect(fetch).not.toHaveBeenCalled(); + expect(onAdded).not.toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); + expect(onDraftChange).not.toHaveBeenCalled(); + expect(container.querySelector("textarea")?.value).toBe( + "Fake fullscreen comment", + ); + expect(send?.disabled).toBe(false); + + await act(async () => { + send?.click(); + }); + expect(getCommentAccessToken).toHaveBeenLastCalledWith("fake-stale-proof"); + expect(fetch).toHaveBeenCalledTimes(1); + const init = fetch.mock.calls[0][1] as RequestInit; + expect(new Headers(init.headers).get("x-clips-comment-token")).toBe( + "fake-renewed-proof", + ); + expect(container.querySelector('[role="alert"]')).toBeNull(); + expect(onDraftChange).toHaveBeenCalledWith(""); + expect(onAdded).toHaveBeenCalledTimes(1); + expect(onClose).toHaveBeenCalledTimes(1); + }); + it.each(["active", "resumed"])( "renews expired proof before a %s viewer submits", async (mode) => { diff --git a/templates/clips/app/components/player/comments-panel.tsx b/templates/clips/app/components/player/comments-panel.tsx index 7bf36058e79..01df3ee20ff 100644 --- a/templates/clips/app/components/player/comments-panel.tsx +++ b/templates/clips/app/components/player/comments-panel.tsx @@ -104,6 +104,13 @@ type EditState = { authoritativeToken?: number; }; +type SentDraft = { + value: string; + mentions: MentionEntry[]; + atMs: number; + replyTo: Comment | null; +}; + const defaultLens: CommentsLens = { selectComments: (data) => (data as { comments?: Comment[] } | undefined)?.comments, @@ -185,7 +192,9 @@ function updateReactionUsers( export interface CommentsPanelProps { recordingId: string; commentAccessToken?: string; - getCommentAccessToken?: () => Promise; + getCommentAccessToken?: ( + refusedToken?: string | null, + ) => Promise; comments: Comment[]; currentMs: number; getCurrentMs?: () => number; @@ -224,15 +233,22 @@ export function CommentsPanel(props: CommentsPanelProps) { useEffect(() => { commentAccessTokenRef.current = props.commentAccessToken; }, [props.commentAccessToken]); + const refusedCommentAccessTokenRef = useRef( + undefined, + ); const commentHeaders = () => commentAccessTokenRef.current ? { [COMMENT_ACCESS_TOKEN_HEADER]: commentAccessTokenRef.current } : undefined; + const refuseCommentAccess = () => { + refusedCommentAccessTokenRef.current = commentAccessTokenRef.current; + }; const isSignedIn = !!currentUserEmail; const isSharePresentation = presentation === "share"; const isInlinePresentation = presentation === "inline"; const isConversationPresentation = isSharePresentation || isInlinePresentation; + const t = useT(); const formatters = useFormatters(); const [draft, setDraft] = useState(""); // A comment belongs to the frame on screen when typing starts, not when it @@ -252,6 +268,21 @@ export function CommentsPanel(props: CommentsPanelProps) { const [editingId, setEditingId] = useState(null); const [editDraft, setEditDraft] = useState(""); const [editMentions, setEditMentions] = useState([]); + // Composers clear on send, so a failed add needs what was sent to put back. + const sentDraftsRef = useRef(new WeakMap()); + const composerDraftsRef = useRef({ + comment: draft, + reply: replyDraft, + replyTo, + }); + useEffect(() => { + composerDraftsRef.current = { comment: draft, reply: replyDraft, replyTo }; + }, [draft, replyDraft, replyTo]); + const [sendFailed, setSendFailed] = useState<"comment" | "reply" | null>( + null, + ); + const clearSendFailed = (composer: "comment" | "reply") => + setSendFailed((failed) => (failed === composer ? null : failed)); const replyComposerRef = useRef(null); const { data: mentionMembers = [] } = useMentionMembers( recordingId, @@ -280,13 +311,33 @@ export function CommentsPanel(props: CommentsPanelProps) { }); }; + // A renewal refetches this query, so queries are cancelled only after it + // settles, and its response can replace the optimistic change: `show` runs + // again afterward and must be idempotent. + const prepareCommentWrite = async (show: () => void, undo: () => void) => { + if (props.getCommentAccessToken) { + const proof = props.getCommentAccessToken( + refusedCommentAccessTokenRef.current, + ); + show(); + try { + commentAccessTokenRef.current = await proof; + } catch (error) { + undo(); + throw error; + } + } + await queryClient.cancelQueries({ queryKey }); + show(); + }; + const reconcileReactionMutation = ( - ctx: CommentsMutationContext, + ctx: CommentsMutationContext | undefined, serverUsers?: string[], succeeded = false, ) => { if ( - !ctx.reactionKey || + !ctx?.reactionKey || !ctx.operationToken || !ctx.commentId || !ctx.emoji @@ -339,7 +390,7 @@ export function CommentsPanel(props: CommentsPanelProps) { }; const reconcileEditMutation = ( - ctx: CommentsMutationContext, + ctx: CommentsMutationContext | undefined, data?: { id?: string; content?: string; @@ -348,7 +399,7 @@ export function CommentsPanel(props: CommentsPanelProps) { }, succeeded = false, ) => { - if (!ctx.commentId || !ctx.operationToken) return; + if (!ctx?.commentId || !ctx.operationToken) return; const state = editStatesRef.current.get(ctx.commentId); if (!state) return; @@ -416,13 +467,32 @@ export function CommentsPanel(props: CommentsPanelProps) { } }; + // Text typed, or a reply opened on another comment, since the send wins over + // the failed draft. + const restoreSentDraft = (vars: object) => { + const sent = sentDraftsRef.current.get(vars); + if (!sent) return; + const composer = sent.replyTo ? "reply" : "comment"; + setSendFailed(composer); + const live = composerDraftsRef.current; + const replyingElsewhere = + sent.replyTo && live.replyTo && live.replyTo.id !== sent.replyTo.id; + if (live[composer].trim() || replyingElsewhere) return; + live[composer] = sent.value; + if (sent.replyTo) { + setReplyTo(sent.replyTo); + setReplyDraft(sent.value); + setReplyMentions(sent.mentions); + } else { + setDraft(sent.value); + setDraftAtMs(sent.atMs); + setDraftMentions(sent.mentions); + } + }; + const addComment = useActionMutation("add-comment", { headers: commentHeaders, onMutate: async (vars: any) => { - if (props.getCommentAccessToken) { - commentAccessTokenRef.current = await props.getCommentAccessToken(); - } - await queryClient.cancelQueries({ queryKey }); const tempId = makeTempId(); const now = new Date().toISOString(); const optimistic: Comment = { @@ -438,11 +508,22 @@ export function CommentsPanel(props: CommentsPanelProps) { createdAt: now, updatedAt: now, }; - patchComments((list) => [...list, optimistic]); - return { type: "add", tempId } satisfies CommentsMutationContext; + const context = { type: "add", tempId } satisfies CommentsMutationContext; + await prepareCommentWrite( + () => + patchComments((list) => + list.some((comment) => comment.id === tempId) + ? list + : [...list, optimistic], + ), + () => rollbackComments(context), + ); + return context; }, - onError: (_err, _vars, ctx: any) => { + onError: (_err, vars: any, ctx: any) => { + refuseCommentAccess(); rollbackComments(ctx); + restoreSentDraft(vars); }, onSuccess: (data: any, _vars, ctx: any) => { if (!ctx?.tempId || !data?.id) return; @@ -459,12 +540,12 @@ export function CommentsPanel(props: CommentsPanelProps) { const reactToComment = useActionMutation("react-to-comment", { headers: commentHeaders, onMutate: async (vars: any) => { - if (props.getCommentAccessToken) { - commentAccessTokenRef.current = await props.getCommentAccessToken(); - } - await queryClient.cancelQueries({ queryKey }); const currentUser = currentUserEmail; if (!currentUser) { + await prepareCommentWrite( + () => {}, + () => {}, + ); return { type: "reaction", commentId: vars.commentId, @@ -496,29 +577,35 @@ export function CommentsPanel(props: CommentsPanelProps) { })(); state.latestToken = operationToken; state.pending.set(operationToken, { optimisticUsers }); - patchComments((commentList) => - commentList.map((comment) => - comment.id === vars.commentId - ? { - ...comment, - emojiReactionsJson: updateReactionUsers( - comment.emojiReactionsJson, - vars.emoji, - optimisticUsers, - ), - } - : comment, - ), - ); - return { + const context = { type: "reaction", commentId: vars.commentId, emoji: vars.emoji, reactionKey, operationToken, } satisfies CommentsMutationContext; + await prepareCommentWrite( + () => + patchComments((commentList) => + commentList.map((comment) => + comment.id === vars.commentId + ? { + ...comment, + emojiReactionsJson: updateReactionUsers( + comment.emojiReactionsJson, + vars.emoji, + optimisticUsers, + ), + } + : comment, + ), + ), + () => reconcileReactionMutation(context, undefined, false), + ); + return context; }, onError: (_err, _vars, ctx: any) => { + refuseCommentAccess(); reconcileReactionMutation(ctx, undefined, false); }, onSuccess: (data: any, _vars: any, ctx: any) => { @@ -536,25 +623,25 @@ export function CommentsPanel(props: CommentsPanelProps) { const remove = useActionMutation("delete-comment", { headers: commentHeaders, onMutate: async (vars: any) => { - if (props.getCommentAccessToken) { - commentAccessTokenRef.current = await props.getCommentAccessToken(); - } - await queryClient.cancelQueries({ queryKey }); const current = visibleCommentsRef.current; const target = current.find((comment) => comment.id === vars.id); const removedIds = target ? collectCommentSubtreeIds(current, target.id) : new Set(); const removed = current.filter((comment) => removedIds.has(comment.id)); - patchComments((list) => { - return list.filter((comment) => !removedIds.has(comment.id)); - }); + const withoutRemoved = (list: Comment[]) => + list.filter((comment) => !removedIds.has(comment.id)); + await prepareCommentWrite( + () => patchComments(withoutRemoved), + () => patchComments((list) => [...withoutRemoved(list), ...removed]), + ); return { type: "remove", removed, } satisfies CommentsMutationContext; }, onError: (_err, _vars, ctx: any) => { + refuseCommentAccess(); rollbackComments(ctx); }, onSuccess: (data: any, _vars: any, ctx: any) => { @@ -575,10 +662,6 @@ export function CommentsPanel(props: CommentsPanelProps) { const updateComment = useActionMutation("update-comment", { headers: commentHeaders, onMutate: async (vars: any) => { - if (props.getCommentAccessToken) { - commentAccessTokenRef.current = await props.getCommentAccessToken(); - } - await queryClient.cancelQueries({ queryKey }); const updatedAt = new Date().toISOString(); const current = visibleCommentsRef.current.find( (comment) => comment.id === vars.id, @@ -607,27 +690,33 @@ export function CommentsPanel(props: CommentsPanelProps) { updatedAt, }); } - patchComments((list) => - list.map((comment) => - comment.id === vars.id - ? { - ...comment, - content: vars.content, - ...(vars.mentions === undefined - ? {} - : { mentions: displayCommentMentions(vars.mentions) }), - updatedAt, - } - : comment, - ), - ); - return { + const context = { type: "update", commentId: vars.id, operationToken, } satisfies CommentsMutationContext; + await prepareCommentWrite( + () => + patchComments((list) => + list.map((comment) => + comment.id === vars.id + ? { + ...comment, + content: vars.content, + ...(vars.mentions === undefined + ? {} + : { mentions: displayCommentMentions(vars.mentions) }), + updatedAt, + } + : comment, + ), + ), + () => reconcileEditMutation(context, undefined, false), + ); + return context; }, onError: (_err, vars: any, ctx: any) => { + refuseCommentAccess(); reconcileEditMutation(ctx, undefined, false); setEditingId(vars.id); setEditDraft(vars.content); @@ -691,14 +780,22 @@ export function CommentsPanel(props: CommentsPanelProps) { ...mentionArgs(value, draftMentions), ...(currentUserName ? { authorName: currentUserName } : {}), }; + sentDraftsRef.current.set(vars, { + value, + mentions: target ? replyMentions : draftMentions, + atMs: vars.videoTimestampMs, + replyTo: target, + }); if (target) { setReplyDraft(""); setReplyMentions([]); setReplyTo(null); + clearSendFailed("reply"); } else { setDraft(""); setDraftAtMs(null); setDraftMentions([]); + clearSendFailed("comment"); } addComment.mutate(vars); } @@ -752,27 +849,43 @@ export function CommentsPanel(props: CommentsPanelProps) { if (!value.trim()) setDraftAtMs(null); else if (draftAtMs === null) setDraftAtMs(getCurrentMs?.() ?? currentMs); setDraft(value); + clearSendFailed("comment"); } + const sendFailedAlert = ( +

+ {t("notificationsRoute.sendFailed")} +

+ ); + const composer = ( - - setDraftMentions((current) => upsertMention(current, mention)) - } - members={mentionMembers} - onSubmit={() => submitDraft(draft, null)} - onUnauthenticated={onUnauthenticated} - /> + <> + + setDraftMentions((current) => upsertMention(current, mention)) + } + members={mentionMembers} + onSubmit={() => submitDraft(draft, null)} + onUnauthenticated={onUnauthenticated} + /> + {sendFailed === "comment" ? sendFailedAlert : null} + ); const renderCommentNode = ( @@ -822,7 +935,10 @@ export function CommentsPanel(props: CommentsPanelProps) { textareaRef={replyComposerRef} currentUserEmail={currentUserEmail} currentUserName={currentUserName} - onDraftChange={setReplyDraft} + onDraftChange={(value) => { + setReplyDraft(value); + clearSendFailed("reply"); + }} onMentionAdd={(mention) => setReplyMentions((current) => upsertMention(current, mention)) } @@ -831,9 +947,11 @@ export function CommentsPanel(props: CommentsPanelProps) { setReplyDraft(""); setReplyMentions([]); setReplyTo(null); + clearSendFailed("reply"); }} onSubmit={() => submitDraft(replyDraft, replyTo)} /> + {sendFailed === "reply" ? sendFailedAlert : null} ) : null} {node.children.length > 0 ? ( diff --git a/templates/clips/app/components/player/timestamped-comment-button.tsx b/templates/clips/app/components/player/timestamped-comment-button.tsx index f8f2bf8a96d..6ff03353e7a 100644 --- a/templates/clips/app/components/player/timestamped-comment-button.tsx +++ b/templates/clips/app/components/player/timestamped-comment-button.tsx @@ -42,7 +42,9 @@ export function TimestampedCommentButton({ interface TimestampedCommentBarProps { commentAccessToken?: string; - getCommentAccessToken?: () => Promise; + getCommentAccessToken?: ( + refusedToken?: string | null, + ) => Promise; recordingId: string; atMs: number; onClose: () => void; @@ -76,6 +78,11 @@ export function TimestampedCommentBar({ useEffect(() => { commentAccessTokenRef.current = commentAccessToken; }, [commentAccessToken]); + const refusedCommentAccessTokenRef = useRef( + undefined, + ); + // Toasts do not paint over real fullscreen, so failure shows in the bar. + const [failed, setFailed] = useState(false); const addComment = useActionMutation("add-comment", { headers: () => commentAccessTokenRef.current @@ -83,9 +90,15 @@ export function TimestampedCommentBar({ : undefined, onMutate: async () => { if (getCommentAccessToken) { - commentAccessTokenRef.current = await getCommentAccessToken(); + commentAccessTokenRef.current = await getCommentAccessToken( + refusedCommentAccessTokenRef.current, + ); } }, + onError: () => { + refusedCommentAccessTokenRef.current = commentAccessTokenRef.current; + setFailed(true); + }, }); useEffect(() => { @@ -95,6 +108,7 @@ export function TimestampedCommentBar({ const submit = () => { const content = draft.trim(); if (!content) return; + setFailed(false); addComment.mutate( { recordingId, content, videoTimestampMs: atMs }, { @@ -116,7 +130,10 @@ export function TimestampedCommentBar({