From 509895a3c6130cf1e95c1c52df8b54ed57f537b1 Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Thu, 22 Oct 2026 13:36:44 -0400 Subject: [PATCH 1/7] Respect recording Trash and comment settings --- templates/clips/actions/add-comment.test.ts | 8 +- templates/clips/actions/add-comment.ts | 13 +- .../actions/comment-writing-access.test.ts | 223 ++++++++++++++++++ .../clips/actions/delete-comment.test.ts | 51 ++-- .../actions/get-recording-player-data.test.ts | 20 ++ .../actions/get-recording-player-data.ts | 10 + templates/clips/actions/react-to-comment.ts | 4 +- templates/clips/actions/reply-to-comment.ts | 69 +----- templates/clips/actions/resolve-comment.ts | 16 +- .../clips/actions/update-comment.test.ts | 14 +- templates/clips/actions/update-comment.ts | 18 +- ...-discussion-settings-apply-consistently.md | 6 + .../server/lib/comment-writing-access.ts | 25 ++ .../clips/server/lib/recording-page-access.ts | 7 + .../routes/api/public-recording.get.test.ts | 53 +++++ .../server/routes/api/public-recording.get.ts | 9 + .../api/thumbnail/[recordingId].get.test.ts | 41 ++++ .../routes/api/thumbnail/[recordingId].get.ts | 15 +- .../api/video/[recordingId].get.test.ts | 48 ++++ .../routes/api/video/[recordingId].get.ts | 14 +- 20 files changed, 518 insertions(+), 146 deletions(-) create mode 100644 templates/clips/actions/comment-writing-access.test.ts create mode 100644 templates/clips/changelog/2026-10-22-recording-sharing-and-discussion-settings-apply-consistently.md create mode 100644 templates/clips/server/lib/comment-writing-access.ts diff --git a/templates/clips/actions/add-comment.test.ts b/templates/clips/actions/add-comment.test.ts index 55bb5fda333..ee161b2aa64 100644 --- a/templates/clips/actions/add-comment.test.ts +++ b/templates/clips/actions/add-comment.test.ts @@ -22,7 +22,8 @@ vi.mock("@agent-native/core/server/request-context", () => ({ getRequestUserName: () => undefined, })); -vi.mock("@agent-native/core/sharing", () => ({ +vi.mock("@agent-native/core/sharing", async (importOriginal) => ({ + ...(await importOriginal()), assertAccess: (...args: unknown[]) => mockAssertAccess(...args), })); @@ -50,7 +51,10 @@ describe("add-comment access", () => { beforeEach(() => { vi.clearAllMocks(); mockGetRequestUserEmail.mockReturnValue("viewer@example.com"); - mockAssertAccess.mockResolvedValue({ role: "commenter", resource: {} }); + mockAssertAccess.mockResolvedValue({ + role: "commenter", + resource: { enableComments: true }, + }); mockDb.select.mockReturnValue({ from: vi.fn(() => ({ where: vi.fn(() => ({ diff --git a/templates/clips/actions/add-comment.ts b/templates/clips/actions/add-comment.ts index 95ef6141ed2..cedd2775b2a 100644 --- a/templates/clips/actions/add-comment.ts +++ b/templates/clips/actions/add-comment.ts @@ -4,14 +4,13 @@ import { getRequestUserEmail, getRequestUserName, } from "@agent-native/core/server/request-context"; -import { assertAccess, ForbiddenError } from "@agent-native/core/sharing"; import { and, eq } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; import { notifyRecordingComment } from "../server/lib/activity-notifications.js"; import { resolveCommentMentions } from "../server/lib/comment-mentions.js"; -import { isRecordingExpiredForViewer } from "../server/lib/recording-page-access.js"; +import { assertCommentWritingAccess } from "../server/lib/comment-writing-access.js"; import { nanoid } from "../server/lib/recordings.js"; const mentionSchema = z.object({ @@ -58,15 +57,7 @@ export default defineAction({ .describe("Organization members mentioned in the comment"), }), run: async (args) => { - const access = await assertAccess("recording", args.recordingId, "viewer"); - if ( - isRecordingExpiredForViewer({ - expiresAt: (access.resource as { expiresAt?: string }).expiresAt, - viewerIsOwner: access.role === "owner", - }) - ) { - throw new ForbiddenError("Recording has expired"); - } + await assertCommentWritingAccess(args.recordingId); 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 new file mode 100644 index 00000000000..b0139f60daf --- /dev/null +++ b/templates/clips/actions/comment-writing-access.test.ts @@ -0,0 +1,223 @@ +import { ForbiddenError } from "@agent-native/core/sharing"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const state = vi.hoisted(() => ({ + comment: { + id: "comment-1", + recordingId: "recording-1", + organizationId: "org-1", + threadId: "thread-1", + parentId: null, + authorEmail: "author@example.com", + content: "Original text", + videoTimestampMs: 12542, + mentionsJson: null, + emojiReactionsJson: "{}", + resolved: false, + }, + inserted: [] as Record[], +})); +const mockAssertAccess = vi.hoisted(() => vi.fn()); +const mockWriteAppState = vi.hoisted(() => vi.fn()); +const mockNotify = vi.hoisted(() => + vi.fn<(...args: unknown[]) => Promise>(async () => null), +); +const mockUpdate = vi.hoisted(() => vi.fn()); + +vi.mock("@agent-native/core/sharing", async (importOriginal) => ({ + ...(await importOriginal()), + assertAccess: (...args: unknown[]) => mockAssertAccess(...args), +})); +vi.mock("@agent-native/core/application-state", () => ({ + writeAppState: (...args: unknown[]) => mockWriteAppState(...args), +})); +vi.mock("@agent-native/core/server/request-context", () => ({ + getRequestUserEmail: () => "author@example.com", + getRequestUserName: () => "Author", +})); +vi.mock("../server/lib/activity-notifications.js", () => ({ + notifyRecordingComment: (...args: unknown[]) => mockNotify(...args), + notifyRecordingReaction: (...args: unknown[]) => mockNotify(...args), +})); +vi.mock("../server/lib/comment-mentions.js", () => ({ + resolveCommentMentions: async () => [], +})); +vi.mock("../server/lib/recordings.js", () => ({ + nanoid: () => "new-comment", + sameOwnerEmail: (left: string, right: string) => left === right, +})); +vi.mock("../server/db/index.js", () => { + const recordings = { + id: "recording.id", + organizationId: "recording.organizationId", + }; + const recordingComments = { + id: "comment.id", + recordingId: "comment.recordingId", + organizationId: "comment.organizationId", + threadId: "comment.threadId", + content: "comment.content", + emojiReactionsJson: "comment.emojiReactionsJson", + }; + return { + schema: { recordings, recordingComments }, + getDb: () => ({ + select: () => ({ + from: (table: unknown) => ({ + where: () => ({ + limit: async () => + table === recordings + ? [{ organizationId: "org-1" }] + : [{ ...state.comment }], + }), + }), + }), + insert: () => ({ + values: async (row: Record) => { + state.inserted.push(row); + }, + }), + update: () => ({ + set: (patch: Record) => ({ + where: () => { + mockUpdate(patch); + Object.assign(state.comment, patch); + return { returning: async () => [{ id: state.comment.id }] }; + }, + }), + }), + }), + }; +}); + +import addComment from "./add-comment"; +import reactToComment from "./react-to-comment"; +import replyToComment from "./reply-to-comment"; +import resolveComment from "./resolve-comment"; +import updateComment from "./update-comment"; + +const writers = [ + { + name: "new comment", + run: () => + addComment.run({ + recordingId: "recording-1", + content: "New text", + videoTimestampMs: 0, + }), + }, + { + name: "reply through add-comment", + run: () => + addComment.run({ + recordingId: "recording-1", + content: "Reply text", + videoTimestampMs: 12542, + parentId: "comment-1", + threadId: "thread-1", + }), + }, + { + name: "reply-to-comment", + run: () => + replyToComment.run({ commentId: "comment-1", content: "Reply text" }), + }, + { + name: "edit", + run: () => updateComment.run({ id: "comment-1", content: "Updated text" }), + }, + { + name: "reaction", + run: () => reactToComment.run({ commentId: "comment-1", emoji: "๐Ÿ‘" }), + }, + { + name: "resolve", + run: () => resolveComment.run({ id: "comment-1", resolved: true }), + }, +]; + +beforeEach(() => { + vi.clearAllMocks(); + state.inserted = []; + Object.assign(state.comment, { + content: "Original text", + emojiReactionsJson: "{}", + resolved: false, + }); + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: { enableComments: true }, + }); +}); + +describe.each(writers)("$name comment writing access", ({ run }) => { + it.each(["viewer", "owner"])( + "refuses disabled comments for a %s before side effects", + async (role) => { + mockAssertAccess.mockResolvedValue({ + role, + resource: { enableComments: false }, + }); + await expect(run()).rejects.toBeInstanceOf(ForbiddenError); + expect(state.inserted).toEqual([]); + expect(mockUpdate).not.toHaveBeenCalled(); + expect(mockNotify).not.toHaveBeenCalled(); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }, + ); + + it("still allows writing when comments are enabled", async () => { + await expect(run()).resolves.toBeDefined(); + expect(mockWriteAppState).toHaveBeenCalled(); + expect(state.inserted.length + mockUpdate.mock.calls.length).toBe(1); + }); + + it("refuses an expired recording for a viewer", async () => { + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: { enableComments: true, expiresAt: "2000-01-01T00:00:00.000Z" }, + }); + await expect(run()).rejects.toBeInstanceOf(ForbiddenError); + expect(state.inserted).toEqual([]); + expect(mockUpdate).not.toHaveBeenCalled(); + }); + + it("preserves the owner's expiry exemption when comments are enabled", async () => { + mockAssertAccess.mockResolvedValue({ + role: "owner", + resource: { enableComments: true, expiresAt: "2000-01-01T00:00:00.000Z" }, + }); + await expect(run()).resolves.toBeDefined(); + }); + + it("preserves recording access denial", async () => { + mockAssertAccess.mockRejectedValue(new ForbiddenError()); + await expect(run()).rejects.toBeInstanceOf(ForbiddenError); + expect(state.inserted).toEqual([]); + expect(mockUpdate).not.toHaveBeenCalled(); + }); +}); + +it("replies retain the parent thread, organization, frame, and return contract", async () => { + const result = await replyToComment.run({ + commentId: "comment-1", + content: "Reply text", + }); + expect(result).toMatchObject({ + id: "new-comment", + threadId: "thread-1", + parentId: "comment-1", + notified: null, + }); + expect(state.inserted).toEqual([ + expect.objectContaining({ + recordingId: "recording-1", + organizationId: "org-1", + threadId: "thread-1", + parentId: "comment-1", + content: "Reply text", + videoTimestampMs: 12542, + authorEmail: "author@example.com", + }), + ]); +}); diff --git a/templates/clips/actions/delete-comment.test.ts b/templates/clips/actions/delete-comment.test.ts index 20ada3b57a1..bc5c0289c0f 100644 --- a/templates/clips/actions/delete-comment.test.ts +++ b/templates/clips/actions/delete-comment.test.ts @@ -15,14 +15,9 @@ const mockGetUserEmail = vi.hoisted(() => vi.fn<() => string | null>(() => "author@example.com"), ); const mockWriteAppState = vi.hoisted(() => vi.fn()); -const { MockForbiddenError } = vi.hoisted(() => { - class MockForbiddenError extends Error {} - return { MockForbiddenError }; -}); - -vi.mock("@agent-native/core/sharing", () => ({ +vi.mock("@agent-native/core/sharing", async (importOriginal) => ({ + ...(await importOriginal()), assertAccess: (...args: unknown[]) => mockAssertAccess(...args), - ForbiddenError: MockForbiddenError, })); vi.mock("@agent-native/core/server/request-context", () => ({ @@ -37,7 +32,8 @@ vi.mock("../server/lib/recording-page-access.js", () => ({ isRecordingExpiredForViewer: () => false, })); -vi.mock("drizzle-orm", () => ({ +vi.mock("drizzle-orm", async (importOriginal) => ({ + ...(await importOriginal()), and: (...conditions: unknown[]) => ({ type: "and", conditions }), eq: (column: unknown, value: unknown) => ({ type: "eq", @@ -145,19 +141,30 @@ beforeEach(() => { }); describe("delete-comment", () => { - it("deletes a comment and every nested descendant", async () => { - const result = await action.run(action.schema.parse({ id: "comment-1" })); + it.each([true, false])( + "deletes a comment and descendants with enableComments=%s", + async (enableComments) => { + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: { enableComments }, + }); + const result = await action.run(action.schema.parse({ id: "comment-1" })); - expect(result).toEqual({ - id: "comment-1", - deletedCommentIds: ["comment-1", "comment-1-reply", "comment-1-reply-2"], - }); - expect(state.rows.map((row) => row.id)).toEqual([ - "comment-2", - "comment-1-cross-record-reply", - ]); - expect(mockWriteAppState).toHaveBeenCalledWith("refresh-signal", { - ts: expect.any(Number), - }); - }); + expect(result).toEqual({ + id: "comment-1", + deletedCommentIds: [ + "comment-1", + "comment-1-reply", + "comment-1-reply-2", + ], + }); + expect(state.rows.map((row) => row.id)).toEqual([ + "comment-2", + "comment-1-cross-record-reply", + ]); + expect(mockWriteAppState).toHaveBeenCalledWith("refresh-signal", { + ts: expect.any(Number), + }); + }, + ); }); diff --git a/templates/clips/actions/get-recording-player-data.test.ts b/templates/clips/actions/get-recording-player-data.test.ts index 2fe22dd0aa0..d46ae971196 100644 --- a/templates/clips/actions/get-recording-player-data.test.ts +++ b/templates/clips/actions/get-recording-player-data.test.ts @@ -232,6 +232,26 @@ describe("get-recording-player-data direct public access", () => { mockShareLimit.mockResolvedValue([]); }); + it.each(["admin", "editor", "viewer"] as const)( + "withholds a trashed recording's direct player payload from %s callers", + async (role) => { + mockResolveAccess.mockResolvedValue({ + role, + resource: { + id: "rec-1", + visibility: "org", + password: null, + expiresAt: null, + trashedAt: "2026-10-22T12:00:00.000Z", + }, + }); + await expect(action.run({ recordingId: "rec-1" })).rejects.toThrow( + "No access to recording rec-1", + ); + expect(mockDb.select).not.toHaveBeenCalled(); + }, + ); + it.each(["admin", "editor", "viewer"] as const)( "requires an explicit recording share for %s callers", async (role) => { diff --git a/templates/clips/actions/get-recording-player-data.ts b/templates/clips/actions/get-recording-player-data.ts index 11fdebf7102..127681edb16 100644 --- a/templates/clips/actions/get-recording-player-data.ts +++ b/templates/clips/actions/get-recording-player-data.ts @@ -64,6 +64,7 @@ import { resolvePlayerVideoUrl } from "../server/lib/player-video-url.js"; import { canOpenDirectRecordingPage, isRecordingExpiredForViewer, + isRecordingTrashedForViewer, } from "../server/lib/recording-page-access.js"; import { hasExplicitRecordingShare } from "../server/lib/recording-share-grant.js"; import { @@ -161,6 +162,15 @@ export default defineAction({ const db = getDb(); const rec: any = access.resource; + if ( + isRecordingTrashedForViewer({ + trashedAt: rec.trashedAt, + viewerIsOwner: access.role === "owner", + }) + ) { + throw new ForbiddenError(`No access to recording ${args.recordingId}`); + } + if ( isRecordingExpiredForViewer({ expiresAt: rec.expiresAt, diff --git a/templates/clips/actions/react-to-comment.ts b/templates/clips/actions/react-to-comment.ts index 4647465904a..c03dec1d32c 100644 --- a/templates/clips/actions/react-to-comment.ts +++ b/templates/clips/actions/react-to-comment.ts @@ -1,12 +1,12 @@ import { defineAction } from "@agent-native/core/action"; import { writeAppState } from "@agent-native/core/application-state"; import { getRequestUserEmail } from "@agent-native/core/server/request-context"; -import { assertAccess } from "@agent-native/core/sharing"; import { and, eq } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; import { notifyRecordingReaction } from "../server/lib/activity-notifications.js"; +import { assertCommentWritingAccess } from "../server/lib/comment-writing-access.js"; const MAX_CAS_ATTEMPTS = 3; @@ -33,7 +33,7 @@ export default defineAction({ if (!existing) throw new Error(`Comment not found: ${args.commentId}`); if (attempt === 0) { - await assertAccess("recording", existing.recordingId, "viewer"); + await assertCommentWritingAccess(existing.recordingId); } const previousJson = existing.emojiReactionsJson; diff --git a/templates/clips/actions/reply-to-comment.ts b/templates/clips/actions/reply-to-comment.ts index 375b611fadb..6d685d6b142 100644 --- a/templates/clips/actions/reply-to-comment.ts +++ b/templates/clips/actions/reply-to-comment.ts @@ -1,18 +1,9 @@ import { defineAction } from "@agent-native/core/action"; -import { writeAppState } from "@agent-native/core/application-state"; -import { - getRequestUserEmail, - getRequestUserName, -} from "@agent-native/core/server/request-context"; -import { assertAccess, ForbiddenError } from "@agent-native/core/sharing"; import { eq } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; -import { notifyRecordingComment } from "../server/lib/activity-notifications.js"; -import { resolveCommentMentions } from "../server/lib/comment-mentions.js"; -import { isRecordingExpiredForViewer } from "../server/lib/recording-page-access.js"; -import { nanoid } from "../server/lib/recordings.js"; +import addComment from "./add-comment.js"; const mentionSchema = z.object({ email: z.string().email(), @@ -43,66 +34,16 @@ export default defineAction({ .limit(1); if (!parent) throw new Error(`Comment not found: ${args.commentId}`); - const access = await assertAccess( - "recording", - parent.recordingId, - "viewer", - ); - if ( - isRecordingExpiredForViewer({ - expiresAt: (access.resource as { expiresAt?: string }).expiresAt, - viewerIsOwner: access.role === "owner", - }) - ) { - throw new ForbiddenError("Recording has expired"); - } - - const authorEmail = getRequestUserEmail(); - if (!authorEmail) { - throw new Error("Sign in required to reply to comments."); - } - const authorName = - getRequestUserName()?.trim() || args.authorName?.trim() || null; - const mentions = await resolveCommentMentions( - args.mentions, - parent.organizationId, - ); - - const id = nanoid(); - const now = new Date().toISOString(); - - await db.insert(schema.recordingComments).values({ - id, + const result = await addComment.run({ recordingId: parent.recordingId, - organizationId: parent.organizationId, threadId: parent.threadId, parentId: parent.id, - authorEmail, - authorName, - content: args.content, - mentionsJson: mentions.length > 0 ? JSON.stringify(mentions) : null, videoTimestampMs: parent.videoTimestampMs, - createdAt: now, - updatedAt: now, - }); - - const notified = await notifyRecordingComment({ - recordingId: parent.recordingId, - threadId: parent.threadId, - authorEmail, - authorName: authorName ?? undefined, content: args.content, - mentions, - videoTimestampMs: parent.videoTimestampMs, - isReply: true, + authorName: args.authorName, + mentions: args.mentions, }); - await writeAppState("refresh-signal", { ts: Date.now() }); - - console.log( - `Replied to comment ${args.commentId} (thread: ${parent.threadId})`, - ); - - return { id, threadId: parent.threadId, parentId: parent.id, notified }; + return { ...result, parentId: parent.id }; }, }); diff --git a/templates/clips/actions/resolve-comment.ts b/templates/clips/actions/resolve-comment.ts index bea8fb34ffc..2593c2ea0a6 100644 --- a/templates/clips/actions/resolve-comment.ts +++ b/templates/clips/actions/resolve-comment.ts @@ -6,7 +6,7 @@ import { eq } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; -import { isRecordingExpiredForViewer } from "../server/lib/recording-page-access.js"; +import { assertCommentWritingAccess } from "../server/lib/comment-writing-access.js"; import { sameOwnerEmail } from "../server/lib/recordings.js"; const cliBoolean = z.preprocess((value) => { @@ -39,19 +39,7 @@ export default defineAction({ throw new Error("Sign in required to resolve comments."); } - const access = await assertAccess( - "recording", - existing.recordingId, - "viewer", - ); - if ( - isRecordingExpiredForViewer({ - expiresAt: (access.resource as { expiresAt?: string }).expiresAt, - viewerIsOwner: access.role === "owner", - }) - ) { - throw new ForbiddenError("Recording has expired"); - } + await assertCommentWritingAccess(existing.recordingId); if (!sameOwnerEmail(existing.authorEmail, userEmail)) { try { diff --git a/templates/clips/actions/update-comment.test.ts b/templates/clips/actions/update-comment.test.ts index d20a485a79a..30830a90a75 100644 --- a/templates/clips/actions/update-comment.test.ts +++ b/templates/clips/actions/update-comment.test.ts @@ -20,14 +20,9 @@ const mockGetUserEmail = vi.hoisted(() => ); const mockWriteAppState = vi.hoisted(() => vi.fn()); const mockIsOrgMember = vi.hoisted(() => vi.fn()); -const { MockForbiddenError } = vi.hoisted(() => { - class MockForbiddenError extends Error {} - return { MockForbiddenError }; -}); - -vi.mock("@agent-native/core/sharing", () => ({ +vi.mock("@agent-native/core/sharing", async (importOriginal) => ({ + ...(await importOriginal()), assertAccess: (...args: unknown[]) => mockAssertAccess(...args), - ForbiddenError: MockForbiddenError, })); vi.mock("@agent-native/core/server/request-context", () => ({ @@ -126,7 +121,10 @@ async function run(args: { id: string; content: string }) { beforeEach(() => { vi.resetAllMocks(); - mockAssertAccess.mockResolvedValue({ role: "viewer", resource: {} }); + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: { enableComments: true }, + }); state.conflict = false; state.rows = [ { diff --git a/templates/clips/actions/update-comment.ts b/templates/clips/actions/update-comment.ts index 5f07bd95824..d4dc8ff4337 100644 --- a/templates/clips/actions/update-comment.ts +++ b/templates/clips/actions/update-comment.ts @@ -1,13 +1,13 @@ import { defineAction } from "@agent-native/core/action"; import { writeAppState } from "@agent-native/core/application-state"; import { getRequestUserEmail } from "@agent-native/core/server/request-context"; -import { assertAccess, ForbiddenError } from "@agent-native/core/sharing"; +import { ForbiddenError } from "@agent-native/core/sharing"; import { and, eq } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; import { resolveCommentMentions } from "../server/lib/comment-mentions.js"; -import { isRecordingExpiredForViewer } from "../server/lib/recording-page-access.js"; +import { assertCommentWritingAccess } from "../server/lib/comment-writing-access.js"; import { sameOwnerEmail } from "../server/lib/recordings.js"; import { displayCommentMentions, @@ -51,19 +51,7 @@ export default defineAction({ .limit(1); if (!existing) throw new Error(`Comment not found: ${args.id}`); - const access = await assertAccess( - "recording", - existing.recordingId, - "viewer", - ); - if ( - isRecordingExpiredForViewer({ - expiresAt: (access.resource as { expiresAt?: string }).expiresAt, - viewerIsOwner: access.role === "owner", - }) - ) { - throw new ForbiddenError("Recording has expired"); - } + await assertCommentWritingAccess(existing.recordingId); if (!sameOwnerEmail(existing.authorEmail, userEmail)) { throw new ForbiddenError( diff --git a/templates/clips/changelog/2026-10-22-recording-sharing-and-discussion-settings-apply-consistently.md b/templates/clips/changelog/2026-10-22-recording-sharing-and-discussion-settings-apply-consistently.md new file mode 100644 index 00000000000..1161229fefa --- /dev/null +++ b/templates/clips/changelog/2026-10-22-recording-sharing-and-discussion-settings-apply-consistently.md @@ -0,0 +1,6 @@ +--- +type: fixed +date: 2026-10-22 +--- + +Recording sharing and discussion settings apply consistently. diff --git a/templates/clips/server/lib/comment-writing-access.ts b/templates/clips/server/lib/comment-writing-access.ts new file mode 100644 index 00000000000..4da99d299f7 --- /dev/null +++ b/templates/clips/server/lib/comment-writing-access.ts @@ -0,0 +1,25 @@ +import { assertAccess, ForbiddenError } from "@agent-native/core/sharing"; + +import { isRecordingExpiredForViewer } from "./recording-page-access.js"; + +export async function assertCommentWritingAccess( + recordingId: string, +): Promise { + const access = await assertAccess("recording", recordingId, "viewer"); + const recording = access.resource as { + enableComments: boolean; + expiresAt?: string | null; + }; + + if ( + isRecordingExpiredForViewer({ + expiresAt: recording.expiresAt, + viewerIsOwner: access.role === "owner", + }) + ) { + throw new ForbiddenError("Recording has expired"); + } + if (!recording.enableComments) { + throw new ForbiddenError(); + } +} diff --git a/templates/clips/server/lib/recording-page-access.ts b/templates/clips/server/lib/recording-page-access.ts index 6ad984c313b..485b3f5cbe4 100644 --- a/templates/clips/server/lib/recording-page-access.ts +++ b/templates/clips/server/lib/recording-page-access.ts @@ -23,6 +23,13 @@ export function isRecordingExpiredForViewer(input: { return !input.viewerIsOwner && isRecordingExpired(input.expiresAt, input.now); } +export function isRecordingTrashedForViewer(input: { + trashedAt: string | null | undefined; + viewerIsOwner: boolean; +}): boolean { + return Boolean(input.trashedAt) && !input.viewerIsOwner; +} + /** * Decide whether an authenticated request may use the editor/player route. * Public visibility is intentionally a share-link concern; a direct `/r/*` 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 372224cd0a2..acde9ec0177 100644 --- a/templates/clips/server/routes/api/public-recording.get.test.ts +++ b/templates/clips/server/routes/api/public-recording.get.test.ts @@ -226,6 +226,59 @@ describe("/api/public-recording route", () => { mockHasExplicitRecordingShare.mockResolvedValue(false); }); + it.each([null, "viewer", "editor", "admin"])( + "withholds trashed share data from %s callers, including existing agent tokens", + async (role) => { + const event = { setCookies: [] as unknown[] }; + const recording = makeRecording({ + trashedAt: "2026-10-22T12:00:00.000Z", + }); + mockGetDb.mockReturnValue( + createDbWithSelectResults([[recording], [], [], [], []]), + ); + mockGetQuery.mockReturnValue({ + id: "rec-1", + agent_access: "existing-token", + }); + mockVerifyScopedAgentAccessToken.mockReturnValue({ ok: true }); + if (role) { + mockGetSession.mockResolvedValue({ + email: "viewer@example.com", + orgId: "org-1", + }); + mockResolveAccess.mockResolvedValue({ role, resource: recording }); + } + + expect(await handler(event as any)).toEqual({ error: "Not found" }); + expect(mockSetResponseStatus).toHaveBeenCalledWith(event, 404); + expect(mockSignShortLivedToken).not.toHaveBeenCalled(); + expect(mockSignScopedAgentAccessToken).not.toHaveBeenCalled(); + expect(mockResolvePlayerVideoUrl).not.toHaveBeenCalled(); + }, + ); + + it.each([ + { role: "owner", trashedAt: "2026-10-22T12:00:00.000Z" }, + { role: "viewer", trashedAt: null }, + ])( + "keeps owner Trash previews and restored shares available: $role", + async ({ role, trashedAt }) => { + const recording = makeRecording({ trashedAt }); + mockGetDb.mockReturnValue( + createDbWithSelectResults([[recording], [], [], [], []]), + ); + mockGetSession.mockResolvedValue({ + email: `${role}@example.com`, + orgId: "org-1", + }); + mockResolveAccess.mockResolvedValue({ role, resource: recording }); + + expect(await handler({ setCookies: [] } as any)).toMatchObject({ + recording: { id: "rec-1" }, + }); + }, + ); + it("sets a protected media cookie and long fallback token after password unlock", async () => { const event = { setCookies: [] as unknown[] }; mockGetDb.mockReturnValue( diff --git a/templates/clips/server/routes/api/public-recording.get.ts b/templates/clips/server/routes/api/public-recording.get.ts index feb7858d189..270c3681c8f 100644 --- a/templates/clips/server/routes/api/public-recording.get.ts +++ b/templates/clips/server/routes/api/public-recording.get.ts @@ -46,6 +46,7 @@ import { resolvePlayerVideoUrl } from "../../lib/player-video-url.js"; import { canOpenDirectRecordingPage, isRecordingExpiredForViewer, + isRecordingTrashedForViewer, type RecordingPageAccessRole, } from "../../lib/recording-page-access.js"; import { hasExplicitRecordingShare } from "../../lib/recording-share-grant.js"; @@ -252,6 +253,14 @@ export default defineEventHandler(async (event) => { : null; const viewerIsOwner = viewerAccess?.role === "owner"; + if ( + isRecordingTrashedForViewer({ trashedAt: rec.trashedAt, viewerIsOwner }) + ) { + setResponseHeader(event, "Cache-Control", "private, max-age=0, no-store"); + setResponseStatus(event, 404); + return { error: "Not found" }; + } + let viewerIsOrgMember = false; if (session?.email && rec.visibility === "org" && rec.organizationId) { try { diff --git a/templates/clips/server/routes/api/thumbnail/[recordingId].get.test.ts b/templates/clips/server/routes/api/thumbnail/[recordingId].get.test.ts index 594d515b265..883c19ec301 100644 --- a/templates/clips/server/routes/api/thumbnail/[recordingId].get.test.ts +++ b/templates/clips/server/routes/api/thumbnail/[recordingId].get.test.ts @@ -202,6 +202,47 @@ describe("/api/thumbnail/:recordingId route", () => { mockSignShortLivedToken.mockReturnValue("renewed-token"); }); + it.each([null, "viewer", "editor", "admin"])( + "does not serve trashed thumbnails to %s callers", + async (role) => { + const recording = makeRow({ trashedAt: "2026-10-22T12:00:00.000Z" }); + mockGetDb.mockReturnValue(createDbWithRow(recording)); + if (role) + mockResolveAccess.mockResolvedValue({ role, resource: recording }); + vi.mocked(fetch).mockResolvedValue( + new Response("thumbnail", { + headers: { "content-type": "image/jpeg" }, + }), + ); + const event = makeEvent(); + + expect(await handler(event as any)).toEqual({ error: "Not found" }); + expect(event.status).toBe(404); + expect(fetch).not.toHaveBeenCalled(); + expect(mockFetchS3ObjectByUrl).not.toHaveBeenCalled(); + }, + ); + + it.each([ + { role: "owner", trashedAt: "2026-10-22T12:00:00.000Z" }, + { role: "viewer", trashedAt: null }, + ])( + "serves owner Trash previews and restored thumbnails: $role", + async ({ role, trashedAt }) => { + const recording = makeRow({ trashedAt }); + mockGetDb.mockReturnValue(createDbWithSelectSequence([])); + mockResolveAccess.mockResolvedValue({ role, resource: recording }); + vi.mocked(fetch).mockResolvedValue( + new Response("thumbnail", { + headers: { "content-type": "image/jpeg" }, + }), + ); + const result = await handler(makeEvent() as any); + expect(result).toBeInstanceOf(Response); + expect(await (result as Response).text()).toBe("thumbnail"); + }, + ); + it("proxies a public provider thumbnail", async () => { mockGetDb.mockReturnValue(createDbWithRow(makeRow())); vi.mocked(fetch).mockResolvedValue( diff --git a/templates/clips/server/routes/api/thumbnail/[recordingId].get.ts b/templates/clips/server/routes/api/thumbnail/[recordingId].get.ts index 7e88a709e05..961c6c45da3 100644 --- a/templates/clips/server/routes/api/thumbnail/[recordingId].get.ts +++ b/templates/clips/server/routes/api/thumbnail/[recordingId].get.ts @@ -34,7 +34,10 @@ import { isHeldForRedaction, REDACTION_HOLD_MESSAGE, } from "../../../lib/pending-redactions.js"; -import { isRecordingExpiredForViewer } from "../../../lib/recording-page-access.js"; +import { + isRecordingExpiredForViewer, + isRecordingTrashedForViewer, +} from "../../../lib/recording-page-access.js"; import { getOrganizationRoleForEmail } from "../../../lib/recordings.js"; import { fetchS3ObjectByUrl, @@ -403,11 +406,15 @@ async function loadRecording(recordingId: string, event: H3Event) { gate = row; } - if (!clipId) return { recording: gate, role, gateId } as const; - - if (gate.trashedAt && role !== "owner") { + if ( + isRecordingTrashedForViewer({ + trashedAt: gate.trashedAt, + viewerIsOwner: role === "owner", + }) + ) { return { error: "Not found", status: 404 } as const; } + if (!clipId) return { recording: gate, role, gateId } as const; const [footage] = await getDb() .select({ thumbnailUrl: schema.recordings.thumbnailUrl, diff --git a/templates/clips/server/routes/api/video/[recordingId].get.test.ts b/templates/clips/server/routes/api/video/[recordingId].get.test.ts index 68ee01acb11..72367206e17 100644 --- a/templates/clips/server/routes/api/video/[recordingId].get.test.ts +++ b/templates/clips/server/routes/api/video/[recordingId].get.test.ts @@ -205,6 +205,54 @@ describe("/api/video/:recordingId route", () => { mockGetDb.mockReturnValue(createDbWithSelectResult([])); }); + it.each(["viewer", "editor", "admin"])( + "does not serve a trashed recording to a %s with an existing media token", + async (role) => { + mockResolveAccess.mockResolvedValue({ + role, + resource: { + visibility: "public", + password: null, + trashedAt: "2026-10-22T12:00:00.000Z", + videoUrl: "https://cdn.example.com/clip.mp4", + }, + }); + mockVerifyShortLivedToken.mockReturnValue({ ok: true }); + vi.mocked(fetch).mockResolvedValue(new Response("video")); + const event = makeEvent(); + event.query = { t: "existing-token" }; + + expect(await handler(event as any)).toEqual({ error: "Not found" }); + expect(event.status).toBe(404); + expect(fetch).not.toHaveBeenCalled(); + expect(mockFetchS3ObjectByUrl).not.toHaveBeenCalled(); + }, + ); + + it.each([ + { role: "owner", trashedAt: "2026-10-22T12:00:00.000Z" }, + { role: "viewer", trashedAt: null }, + ])( + "serves owner Trash previews and restored videos: $role", + async ({ role, trashedAt }) => { + mockResolveAccess.mockResolvedValue({ + role, + resource: { + visibility: "public", + password: null, + trashedAt, + videoUrl: "https://cdn.example.com/clip.mp4", + }, + }); + vi.mocked(fetch).mockResolvedValue( + new Response("video", { headers: { "content-type": "video/mp4" } }), + ); + const result = await handler(makeEvent() as any); + expect(result).toBeInstanceOf(Response); + expect(await (result as Response).text()).toBe("video"); + }, + ); + it("returns a controlled media fetch error when upstream fetch throws", async () => { vi.mocked(fetch).mockRejectedValue(new Error("fetch failed")); diff --git a/templates/clips/server/routes/api/video/[recordingId].get.ts b/templates/clips/server/routes/api/video/[recordingId].get.ts index 05dcf11f3a2..94718d344ea 100644 --- a/templates/clips/server/routes/api/video/[recordingId].get.ts +++ b/templates/clips/server/routes/api/video/[recordingId].get.ts @@ -78,7 +78,10 @@ import { isHeldForRedaction, REDACTION_HOLD_MESSAGE, } from "../../../lib/pending-redactions.js"; -import { isRecordingExpiredForViewer } from "../../../lib/recording-page-access.js"; +import { + isRecordingExpiredForViewer, + isRecordingTrashedForViewer, +} from "../../../lib/recording-page-access.js"; import { getOrganizationRoleForEmail } from "../../../lib/recordings.js"; import { fetchS3ObjectByUrl } from "../../../lib/s3-upload-provider.js"; import { @@ -377,9 +380,12 @@ export default defineEventHandler(async (event: H3Event) => { } const gate = recRow; - // The owner's context panel lists items for a trashed Clip, so the owner - // can still play them. - if (clipId && gate.trashedAt && role !== "owner") { + if ( + isRecordingTrashedForViewer({ + trashedAt: gate.trashedAt, + viewerIsOwner: role === "owner", + }) + ) { setResponseStatus(event, 404); return { error: "Not found" }; } From b78d1fd6e322cf8d6db1e3a8bd7fafcad14922a7 Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:29:10 -0400 Subject: [PATCH 2/7] Apply recording policies to comment reads --- templates/clips/actions/add-comment.ts | 4 +- .../actions/comment-writing-access.test.ts | 10 ++ .../actions/get-recording-player-data.test.ts | 46 ++++++ .../actions/get-recording-player-data.ts | 18 ++- templates/clips/actions/list-comments.test.ts | 90 +++++++++++ templates/clips/actions/list-comments.ts | 4 +- .../clips/actions/list-notifications.test.ts | 144 ++++++++++++++++++ templates/clips/actions/list-notifications.ts | 25 ++- templates/clips/actions/react-to-comment.ts | 4 +- templates/clips/actions/resolve-comment.ts | 4 +- templates/clips/actions/update-comment.ts | 4 +- templates/clips/actions/view-screen.test.ts | 109 ++++++++++++- templates/clips/actions/view-screen.ts | 17 ++- ...nt-writing-access.ts => comment-access.ts} | 19 ++- 14 files changed, 464 insertions(+), 34 deletions(-) create mode 100644 templates/clips/actions/list-comments.test.ts create mode 100644 templates/clips/actions/list-notifications.test.ts rename templates/clips/server/lib/{comment-writing-access.ts => comment-access.ts} (57%) diff --git a/templates/clips/actions/add-comment.ts b/templates/clips/actions/add-comment.ts index cedd2775b2a..400ae3b3bdf 100644 --- a/templates/clips/actions/add-comment.ts +++ b/templates/clips/actions/add-comment.ts @@ -9,8 +9,8 @@ import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; import { notifyRecordingComment } from "../server/lib/activity-notifications.js"; +import { assertCommentAccess } from "../server/lib/comment-access.js"; import { resolveCommentMentions } from "../server/lib/comment-mentions.js"; -import { assertCommentWritingAccess } from "../server/lib/comment-writing-access.js"; import { nanoid } from "../server/lib/recordings.js"; const mentionSchema = z.object({ @@ -57,7 +57,7 @@ export default defineAction({ .describe("Organization members mentioned in the comment"), }), run: async (args) => { - await assertCommentWritingAccess(args.recordingId); + await assertCommentAccess(args.recordingId); 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 b0139f60daf..6c4bf51ca37 100644 --- a/templates/clips/actions/comment-writing-access.test.ts +++ b/templates/clips/actions/comment-writing-access.test.ts @@ -182,6 +182,16 @@ describe.each(writers)("$name comment writing access", ({ run }) => { expect(mockUpdate).not.toHaveBeenCalled(); }); + it("refuses a trashed recording for a viewer", async () => { + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: { enableComments: true, trashedAt: "2026-10-22T12:00:00.000Z" }, + }); + await expect(run()).rejects.toBeInstanceOf(ForbiddenError); + expect(state.inserted).toEqual([]); + expect(mockUpdate).not.toHaveBeenCalled(); + }); + it("preserves the owner's expiry exemption when comments are enabled", async () => { mockAssertAccess.mockResolvedValue({ role: "owner", diff --git a/templates/clips/actions/get-recording-player-data.test.ts b/templates/clips/actions/get-recording-player-data.test.ts index d46ae971196..d7d7556fe86 100644 --- a/templates/clips/actions/get-recording-player-data.test.ts +++ b/templates/clips/actions/get-recording-player-data.test.ts @@ -552,6 +552,51 @@ describe("get-recording-player-data view count", () => { expect(result.recording.folderId).toBe("folder-1"); }); + it.each(["owner", "viewer"])( + "omits disabled comments from the %s player payload", + async (role) => { + mockResolveAccess.mockResolvedValue({ + role, + resource: { + id: "rec-1", + ownerEmail: "owner@example.com", + visibility: "org", + enableComments: false, + password: null, + expiresAt: null, + status: "ready", + chaptersJson: "[]", + }, + }); + mockPlayerQuery.build = () => { + let isCommentsQuery = false; + const query: Record = {}; + query.from = (table: unknown) => { + isCommentsQuery = + (table as { videoTimestampMs?: string })?.videoTimestampMs === + "recordingComments.videoTimestampMs"; + return query; + }; + query.where = () => query; + query.limit = async () => []; + query.orderBy = async () => + isCommentsQuery + ? [ + { + id: "comment-1", + authorEmail: "author@example.com", + authorName: "Author", + content: "Saved comment", + }, + ] + : []; + return query; + }; + const result = await action.run({ recordingId: "rec-1" }); + expect(result.comments).toEqual([]); + }, + ); + it("includes a trashed recording's timestamp in the player payload", async () => { const trashedAt = "2026-09-22T12:00:00.000Z"; mockResolveAccess.mockResolvedValue({ @@ -703,6 +748,7 @@ describe("get-recording-player-data for agents reading a YouTube clip", () => { role: "owner", resource: { id: "rec-yt", + enableComments: true, ownerEmail: "owner@example.com", visibility: "private", password: null, diff --git a/templates/clips/actions/get-recording-player-data.ts b/templates/clips/actions/get-recording-player-data.ts index 127681edb16..69a9cf76030 100644 --- a/templates/clips/actions/get-recording-player-data.ts +++ b/templates/clips/actions/get-recording-player-data.ts @@ -251,14 +251,16 @@ export default defineAction({ ? normalizeBuilderCreditsStatus(builderCreditsRaw) : null; - const comments = await db - .select() - .from(schema.recordingComments) - .where(eq(schema.recordingComments.recordingId, args.recordingId)) - .orderBy( - asc(schema.recordingComments.videoTimestampMs), - asc(schema.recordingComments.createdAt), - ); + const comments = rec.enableComments + ? await db + .select() + .from(schema.recordingComments) + .where(eq(schema.recordingComments.recordingId, args.recordingId)) + .orderBy( + asc(schema.recordingComments.videoTimestampMs), + asc(schema.recordingComments.createdAt), + ) + : []; const hydratedComments = await hydrateCommentAuthorNames(comments); const reactions = await db diff --git a/templates/clips/actions/list-comments.test.ts b/templates/clips/actions/list-comments.test.ts new file mode 100644 index 00000000000..e350985a951 --- /dev/null +++ b/templates/clips/actions/list-comments.test.ts @@ -0,0 +1,90 @@ +import { ForbiddenError } from "@agent-native/core/sharing"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mockAssertAccess = vi.hoisted(() => vi.fn()); +const mockSelect = vi.hoisted(() => vi.fn()); +const comment = vi.hoisted(() => ({ + id: "comment-1", + recordingId: "recording-1", + authorEmail: "author@example.com", + authorName: "Author", + content: "A saved comment", + mentionsJson: null, +})); + +vi.mock("@agent-native/core/sharing", async (importOriginal) => ({ + ...(await importOriginal()), + assertAccess: (...args: unknown[]) => mockAssertAccess(...args), +})); +vi.mock("../server/db/index.js", () => ({ + getDb: () => ({ select: mockSelect }), + schema: { recordingComments: {} }, +})); +vi.mock("../server/lib/user-identities.js", () => ({ + hydrateCommentAuthorNames: async (rows: unknown[]) => rows, +})); + +import listComments from "./list-comments"; + +beforeEach(() => { + vi.clearAllMocks(); + mockAssertAccess.mockResolvedValue({ + role: "viewer", + resource: { enableComments: true, trashedAt: null, expiresAt: null }, + }); + mockSelect.mockReturnValue({ + from: () => ({ + where: () => ({ orderBy: async () => [comment] }), + }), + }); +}); + +describe("list-comments recording policy", () => { + it.each([ + { role: "viewer", resource: { enableComments: false } }, + { role: "owner", resource: { enableComments: false } }, + { + role: "viewer", + resource: { enableComments: true, trashedAt: "2026-10-22T12:00:00.000Z" }, + }, + { + role: "editor", + resource: { enableComments: true, trashedAt: "2026-10-22T12:00:00.000Z" }, + }, + { + role: "viewer", + resource: { enableComments: true, expiresAt: "2000-01-01T00:00:00.000Z" }, + }, + ])( + "refuses unavailable comments before querying: $role $resource", + async (access) => { + mockAssertAccess.mockResolvedValue(access); + await expect( + listComments.run({ recordingId: "recording-1" }), + ).rejects.toBeInstanceOf(ForbiddenError); + expect(mockSelect).not.toHaveBeenCalled(); + }, + ); + + it.each([ + { + role: "owner", + resource: { enableComments: true, trashedAt: "2026-10-22T12:00:00.000Z" }, + }, + { + role: "owner", + resource: { enableComments: true, expiresAt: "2000-01-01T00:00:00.000Z" }, + }, + { role: "viewer", resource: { enableComments: true, trashedAt: null } }, + ])( + "keeps owner previews and restored comments available: $role $resource", + async (access) => { + mockAssertAccess.mockResolvedValue(access); + expect( + await listComments.run({ recordingId: "recording-1" }), + ).toMatchObject({ + comments: [{ id: "comment-1", content: "A saved comment" }], + }); + }, + ); +}); diff --git a/templates/clips/actions/list-comments.ts b/templates/clips/actions/list-comments.ts index 995feac95a7..7e6611ae69b 100644 --- a/templates/clips/actions/list-comments.ts +++ b/templates/clips/actions/list-comments.ts @@ -1,9 +1,9 @@ import { defineAction } from "@agent-native/core/action"; -import { assertAccess } from "@agent-native/core/sharing"; import { eq, asc } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; +import { assertCommentAccess } from "../server/lib/comment-access.js"; import { hydrateCommentAuthorNames } from "../server/lib/user-identities.js"; import { displayCommentMentions } from "../shared/comment-mentions.js"; @@ -15,7 +15,7 @@ export default defineAction({ }), http: { method: "GET" }, run: async (args) => { - await assertAccess("recording", args.recordingId, "viewer"); + await assertCommentAccess(args.recordingId); const db = getDb(); const rows = await db diff --git a/templates/clips/actions/list-notifications.test.ts b/templates/clips/actions/list-notifications.test.ts new file mode 100644 index 00000000000..42851e6ce2b --- /dev/null +++ b/templates/clips/actions/list-notifications.test.ts @@ -0,0 +1,144 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const state = vi.hoisted(() => ({ + email: "viewer@example.com", + recording: { + id: "rec-test", + title: "Fake recording", + ownerEmail: "owner@example.com", + password: null as string | null, + expiresAt: null as string | null, + trashedAt: null as string | null, + }, +})); +const tables = vi.hoisted(() => ({ + recordings: { + id: "id", + title: "title", + ownerEmail: "ownerEmail", + password: "password", + expiresAt: "expiresAt", + trashedAt: "trashedAt", + }, + recordingShares: {}, + recordingComments: { + recordingId: "recordingId", + createdAt: "createdAt", + mentionsJson: "mentionsJson", + }, + recordingReactions: { recordingId: "recordingId", createdAt: "createdAt" }, +})); + +vi.mock("@agent-native/core/action", () => ({ + defineAction: (options: unknown) => options, +})); +vi.mock("@agent-native/core/sharing", () => ({ accessFilter: () => true })); +vi.mock("@agent-native/core/user-profile/server", () => ({ + getUserProfiles: async () => new Map(), +})); +vi.mock("../server/lib/recordings.js", () => ({ + getCurrentOwnerEmail: () => state.email, + sameOwnerEmail: (a: string, b: string) => + a?.toLowerCase() === b?.toLowerCase(), +})); +vi.mock("../server/lib/user-identities.js", () => ({ + profileNameFor: () => "Fake author", +})); +vi.mock("drizzle-orm", () => ({ + and: (...conditions: unknown[]) => conditions, + or: (...conditions: unknown[]) => conditions, + desc: vi.fn(), + gte: vi.fn(), + sql: vi.fn(), + inArray: (column: string, values: string[]) => ({ column, values }), +})); +vi.mock("../server/db/index.js", () => ({ + schema: tables, + getDb: () => ({ + select: (projection?: Record) => { + let table: unknown; + const query = { + from: (value: unknown) => { + table = value; + return query; + }, + where: () => query, + orderBy: () => query, + limit: () => query, + then: (resolve: (rows: unknown[]) => unknown) => { + const rows = + table === tables.recordings + ? [ + Object.fromEntries( + Object.entries(projection!).map(([key, column]) => [ + key, + state.recording[column as keyof typeof state.recording], + ]), + ), + ] + : table === tables.recordingComments + ? [ + { + id: "comment-test", + recordingId: "rec-test", + content: "Fake discussion", + authorEmail: "author@example.com", + createdAt: new Date().toISOString(), + mentionsJson: JSON.stringify([ + { email: "viewer@example.com" }, + ]), + }, + ] + : []; + return Promise.resolve(rows).then(resolve); + }, + }; + return query; + }, + }), +})); + +import action from "./list-notifications"; + +describe("list-notifications recording lifecycle", () => { + beforeEach(() => { + state.email = "viewer@example.com"; + state.recording.trashedAt = null; + state.recording.expiresAt = null; + state.recording.password = null; + }); + + it("withholds a trashed recording's mention from a nonowner", async () => { + state.recording.trashedAt = "2026-01-01T00:00:00Z"; + expect(await action.run({ days: 30, limit: 200 })).toEqual({ + items: [], + count: 0, + }); + }); + + it.each([ + { email: "owner@example.com", trashedAt: "2026-01-01T00:00:00Z" }, + { email: "viewer@example.com", trashedAt: null }, + ])( + "preserves owner Trash and restored mentions: %j", + async ({ email, trashedAt }) => { + state.email = email; + state.recording.trashedAt = trashedAt; + const result = await action.run({ days: 30, limit: 200 }); + expect(result.items[0]?.preview).toBe("Fake discussion"); + expect(result.count).toBe(1); + }, + ); + + it.each(["viewer@example.com", "owner@example.com"])( + "preserves expiry filtering for %s", + async (email) => { + state.email = email; + state.recording.expiresAt = "2020-01-01T00:00:00Z"; + expect(await action.run({ days: 30, limit: 200 })).toEqual({ + items: [], + count: 0, + }); + }, + ); +}); diff --git a/templates/clips/actions/list-notifications.ts b/templates/clips/actions/list-notifications.ts index 5f79afbb69b..feb166544f6 100644 --- a/templates/clips/actions/list-notifications.ts +++ b/templates/clips/actions/list-notifications.ts @@ -5,7 +5,10 @@ import { and, desc, gte, inArray, or, sql } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; -import { canReceiveRecordingActivity } from "../server/lib/recording-page-access.js"; +import { + canReceiveRecordingActivity, + isRecordingTrashedForViewer, +} from "../server/lib/recording-page-access.js"; import { getCurrentOwnerEmail, sameOwnerEmail, @@ -32,16 +35,22 @@ export default defineAction({ ownerEmail: schema.recordings.ownerEmail, password: schema.recordings.password, expiresAt: schema.recordings.expiresAt, + trashedAt: schema.recordings.trashedAt, }) .from(schema.recordings) .where(accessFilter(schema.recordings, schema.recordingShares)); - const accessibleRecordings = visibleRecordings.filter((recording) => - canReceiveRecordingActivity({ - ownerEmail: recording.ownerEmail, - recipientEmail: me, - hasPassword: Boolean(recording.password), - expiresAt: recording.expiresAt, - }), + const accessibleRecordings = visibleRecordings.filter( + (recording) => + !isRecordingTrashedForViewer({ + trashedAt: recording.trashedAt, + viewerIsOwner: sameOwnerEmail(recording.ownerEmail, me), + }) && + canReceiveRecordingActivity({ + ownerEmail: recording.ownerEmail, + recipientEmail: me, + hasPassword: Boolean(recording.password), + expiresAt: recording.expiresAt, + }), ); if (accessibleRecordings.length === 0) { return { items: [], count: 0 }; diff --git a/templates/clips/actions/react-to-comment.ts b/templates/clips/actions/react-to-comment.ts index c03dec1d32c..f29f92c605f 100644 --- a/templates/clips/actions/react-to-comment.ts +++ b/templates/clips/actions/react-to-comment.ts @@ -6,7 +6,7 @@ import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; import { notifyRecordingReaction } from "../server/lib/activity-notifications.js"; -import { assertCommentWritingAccess } from "../server/lib/comment-writing-access.js"; +import { assertCommentAccess } from "../server/lib/comment-access.js"; const MAX_CAS_ATTEMPTS = 3; @@ -33,7 +33,7 @@ export default defineAction({ if (!existing) throw new Error(`Comment not found: ${args.commentId}`); if (attempt === 0) { - await assertCommentWritingAccess(existing.recordingId); + await assertCommentAccess(existing.recordingId); } const previousJson = existing.emojiReactionsJson; diff --git a/templates/clips/actions/resolve-comment.ts b/templates/clips/actions/resolve-comment.ts index 2593c2ea0a6..1c2e4ad24d7 100644 --- a/templates/clips/actions/resolve-comment.ts +++ b/templates/clips/actions/resolve-comment.ts @@ -6,7 +6,7 @@ import { eq } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; -import { assertCommentWritingAccess } from "../server/lib/comment-writing-access.js"; +import { assertCommentAccess } from "../server/lib/comment-access.js"; import { sameOwnerEmail } from "../server/lib/recordings.js"; const cliBoolean = z.preprocess((value) => { @@ -39,7 +39,7 @@ export default defineAction({ throw new Error("Sign in required to resolve comments."); } - await assertCommentWritingAccess(existing.recordingId); + await assertCommentAccess(existing.recordingId); if (!sameOwnerEmail(existing.authorEmail, userEmail)) { try { diff --git a/templates/clips/actions/update-comment.ts b/templates/clips/actions/update-comment.ts index d4dc8ff4337..4b1949ef0da 100644 --- a/templates/clips/actions/update-comment.ts +++ b/templates/clips/actions/update-comment.ts @@ -6,8 +6,8 @@ import { and, eq } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; +import { assertCommentAccess } from "../server/lib/comment-access.js"; import { resolveCommentMentions } from "../server/lib/comment-mentions.js"; -import { assertCommentWritingAccess } from "../server/lib/comment-writing-access.js"; import { sameOwnerEmail } from "../server/lib/recordings.js"; import { displayCommentMentions, @@ -51,7 +51,7 @@ export default defineAction({ .limit(1); if (!existing) throw new Error(`Comment not found: ${args.id}`); - await assertCommentWritingAccess(existing.recordingId); + await assertCommentAccess(existing.recordingId); if (!sameOwnerEmail(existing.authorEmail, userEmail)) { throw new ForbiddenError( diff --git a/templates/clips/actions/view-screen.test.ts b/templates/clips/actions/view-screen.test.ts index bf242236ac4..a6deece9a98 100644 --- a/templates/clips/actions/view-screen.test.ts +++ b/templates/clips/actions/view-screen.test.ts @@ -1,5 +1,12 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +const context = vi.hoisted(() => ({ + navigation: { view: "library", recordingId: "rec-test" }, + recording: null as Record | null, + email: "viewer@example.com", + childReads: vi.fn(), +})); + const TABLES = vi.hoisted(() => ({ recordings: { id: "recordings.id", @@ -35,6 +42,29 @@ const mockMeetingsWhere = vi.hoisted(() => const makeRealDb = vi.hoisted(() => () => ({ select: (_projection?: unknown) => ({ from: (table: any) => { + if (context.recording) { + if (table === TABLES.recordings) { + return { where: async () => [context.recording] }; + } + context.childReads(table); + const rows = + table === "recordingComments" + ? [ + { + id: "comment-test", + content: "Fake discussion", + authorEmail: "author@example.com", + }, + ] + : []; + return { + where: () => + Object.assign(Promise.resolve(rows), { + orderBy: async () => rows, + limit: async () => rows, + }), + }; + } if (table && "recordingId" in table) return { where: mockMeetingsWhere }; if (table && "position" in table) { return { where: () => ({ orderBy: async () => [] }) }; @@ -104,11 +134,11 @@ vi.mock("@agent-native/core", () => ({ vi.mock("@agent-native/core/application-state", () => ({ readAppState: async () => null, readAppStateForCurrentTab: async (key: string) => - key === "navigation" ? { view: "library" } : null, + key === "navigation" ? context.navigation : null, })); vi.mock("@agent-native/core/server/request-context", () => ({ - getRequestUserEmail: () => "viewer@example.com", + getRequestUserEmail: () => context.email, })); vi.mock("@agent-native/core/sharing", () => ({ @@ -139,6 +169,9 @@ vi.mock("../server/db/index.js", () => ({ recordings: TABLES.recordings, recordingShares: "recordingShares", recordingViewers: "recordingViewers", + recordingComments: "recordingComments", + recordingTranscripts: { recordingId: "transcripts.recordingId" }, + recordingBrowserDiagnostics: { recordingId: "diagnostics.recordingId" }, }, })); @@ -154,6 +187,12 @@ vi.mock("../server/lib/recordings.js", () => ({ email, }), parseSpaceIds: vi.fn(), + sameOwnerEmail: (a: string, b: string) => + a?.toLowerCase() === b?.toLowerCase(), +})); + +vi.mock("../server/lib/user-identities.js", () => ({ + hydrateCommentAuthorNames: async (rows: unknown[]) => rows, })); vi.mock("../shared/browser-diagnostics.js", () => ({ @@ -169,6 +208,8 @@ import action from "./view-screen"; describe("view-screen library view on a cold start", () => { beforeEach(() => { vi.clearAllMocks(); + context.recording = null; + context.navigation.view = "library"; }); it("builds the meeting-exclusion subquery off a resolved db, not the lazy proxy", async () => { @@ -208,3 +249,67 @@ describe("view-screen library view on a cold start", () => { ); }); }); + +describe("view-screen recording access", () => { + beforeEach(() => { + vi.clearAllMocks(); + context.navigation.view = "recording"; + context.email = "viewer@example.com"; + context.recording = { + id: "rec-test", + title: "Fake recording", + ownerEmail: "owner@example.com", + status: "ready", + enableComments: true, + trashedAt: null, + expiresAt: null, + }; + }); + + it.each([ + { trashedAt: "2026-01-01T00:00:00Z" }, + { expiresAt: "2020-01-01T00:00:00Z" }, + ])("withholds recording context after access ends: %j", async (settings) => { + Object.assign(context.recording!, settings); + const screen = JSON.parse(await action.run({})); + expect(screen.recording).toBeUndefined(); + expect(screen.comments).toBeUndefined(); + expect(screen.transcript).toBeUndefined(); + expect(context.childReads).not.toHaveBeenCalled(); + }); + + it.each(["viewer@example.com", "owner@example.com"])( + "omits disabled comments for %s", + async (email) => { + context.email = email; + context.recording!.enableComments = false; + const screen = JSON.parse(await action.run({})); + expect(screen.recording.id).toBe("rec-test"); + expect(screen.comments).toEqual([]); + expect(context.childReads).not.toHaveBeenCalledWith("recordingComments"); + }, + ); + + it.each([ + { + email: "owner@example.com", + trashedAt: "2026-01-01T00:00:00Z", + expiresAt: null, + }, + { + email: "owner@example.com", + trashedAt: null, + expiresAt: "2020-01-01T00:00:00Z", + }, + { email: "viewer@example.com", trashedAt: null, expiresAt: null }, + ])( + "keeps owner previews and restored viewer context: %j", + async ({ email, ...settings }) => { + context.email = email; + Object.assign(context.recording!, settings); + const screen = JSON.parse(await action.run({})); + expect(screen.recording.id).toBe("rec-test"); + expect(screen.comments[0].content).toBe("Fake discussion"); + }, + ); +}); diff --git a/templates/clips/actions/view-screen.ts b/templates/clips/actions/view-screen.ts index 5323ae10718..ee64ef84101 100644 --- a/templates/clips/actions/view-screen.ts +++ b/templates/clips/actions/view-screen.ts @@ -22,10 +22,15 @@ import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; import { agentRecordingAccessFilter } from "../server/lib/agent-recording-access.js"; import { listingThumbnailUrl } from "../server/lib/player-thumbnail-url.js"; +import { + isRecordingExpiredForViewer, + isRecordingTrashedForViewer, +} from "../server/lib/recording-page-access.js"; import { getActiveOrganizationId, ownerEmailMatches, parseSpaceIds, + sameOwnerEmail, } from "../server/lib/recordings.js"; import { hydrateCommentAuthorNames } from "../server/lib/user-identities.js"; import { getAgentClipReadiness } from "../shared/agent-context.js"; @@ -97,7 +102,15 @@ async function fetchRecording(id: string) { ), ), ); - return row ? mapRecording(row) : null; + if (!row) return null; + const viewerIsOwner = sameOwnerEmail(row.ownerEmail, getRequestUserEmail()); + if ( + isRecordingTrashedForViewer({ trashedAt: row.trashedAt, viewerIsOwner }) || + isRecordingExpiredForViewer({ expiresAt: row.expiresAt, viewerIsOwner }) + ) { + return null; + } + return mapRecording(row); } async function fetchTranscript(recordingId: string) { @@ -501,7 +514,7 @@ async function addRecordingContext( const [transcript, comments, browserDiagnosticsSummary, bugReportSummary] = await Promise.all([ fetchTranscript(recordingId), - fetchComments(recordingId), + recording.enableComments ? fetchComments(recordingId) : [], fetchBrowserDiagnosticsSummary(recordingId), fetchBugReportSummary(recordingId), ]); diff --git a/templates/clips/server/lib/comment-writing-access.ts b/templates/clips/server/lib/comment-access.ts similarity index 57% rename from templates/clips/server/lib/comment-writing-access.ts rename to templates/clips/server/lib/comment-access.ts index 4da99d299f7..cbfc16f7283 100644 --- a/templates/clips/server/lib/comment-writing-access.ts +++ b/templates/clips/server/lib/comment-access.ts @@ -1,16 +1,27 @@ import { assertAccess, ForbiddenError } from "@agent-native/core/sharing"; -import { isRecordingExpiredForViewer } from "./recording-page-access.js"; +import { + isRecordingExpiredForViewer, + isRecordingTrashedForViewer, +} from "./recording-page-access.js"; -export async function assertCommentWritingAccess( - recordingId: string, -): Promise { +export async function assertCommentAccess(recordingId: string): Promise { const access = await assertAccess("recording", recordingId, "viewer"); const recording = access.resource as { enableComments: boolean; expiresAt?: string | null; + trashedAt?: string | null; }; + if ( + isRecordingTrashedForViewer({ + trashedAt: recording.trashedAt, + viewerIsOwner: access.role === "owner", + }) + ) { + throw new ForbiddenError(); + } + if ( isRecordingExpiredForViewer({ expiresAt: recording.expiresAt, From e845de2e1439191d8a667fe5fe495e0d49680e4c Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:49:39 -0400 Subject: [PATCH 3/7] Apply Trash access to comment cleanup --- .../clips/actions/delete-comment.test.ts | 44 ++++++++++++++++--- templates/clips/actions/delete-comment.ts | 13 +++++- .../clips/actions/list-notifications.test.ts | 13 ++++++ 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/templates/clips/actions/delete-comment.test.ts b/templates/clips/actions/delete-comment.test.ts index bc5c0289c0f..f4d2175c05c 100644 --- a/templates/clips/actions/delete-comment.test.ts +++ b/templates/clips/actions/delete-comment.test.ts @@ -1,3 +1,4 @@ +import { ForbiddenError } from "@agent-native/core/sharing"; import { beforeEach, describe, expect, it, vi } from "vitest"; type CommentRow = { @@ -28,10 +29,6 @@ vi.mock("@agent-native/core/application-state", () => ({ writeAppState: (...args: unknown[]) => mockWriteAppState(...args), })); -vi.mock("../server/lib/recording-page-access.js", () => ({ - isRecordingExpiredForViewer: () => false, -})); - vi.mock("drizzle-orm", async (importOriginal) => ({ ...(await importOriginal()), and: (...conditions: unknown[]) => ({ type: "and", conditions }), @@ -141,12 +138,49 @@ beforeEach(() => { }); describe("delete-comment", () => { + it.each([ + { role: "viewer", email: "author@example.com" }, + { role: "editor", email: "editor@example.com" }, + ])( + "refuses deletion from Trash for a nonowner: %j", + async ({ role, email }) => { + mockGetUserEmail.mockReturnValue(email); + mockAssertAccess.mockResolvedValue({ + role, + resource: { enableComments: true, trashedAt: "2026-01-01T00:00:00Z" }, + }); + const originalRows = [...state.rows]; + await expect( + action.run(action.schema.parse({ id: "comment-1" })), + ).rejects.toBeInstanceOf(ForbiddenError); + expect(state.rows).toEqual(originalRows); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }, + ); + + it.each([true, false])( + "preserves owner cleanup in Trash with enableComments=%s", + async (enableComments) => { + mockGetUserEmail.mockReturnValue("owner@example.com"); + mockAssertAccess.mockResolvedValue({ + role: "owner", + resource: { enableComments, trashedAt: "2026-01-01T00:00:00Z" }, + }); + const result = await action.run(action.schema.parse({ id: "comment-1" })); + expect(result.deletedCommentIds).toEqual([ + "comment-1", + "comment-1-reply", + "comment-1-reply-2", + ]); + }, + ); + it.each([true, false])( "deletes a comment and descendants with enableComments=%s", async (enableComments) => { mockAssertAccess.mockResolvedValue({ role: "viewer", - resource: { enableComments }, + resource: { enableComments, trashedAt: null }, }); const result = await action.run(action.schema.parse({ id: "comment-1" })); diff --git a/templates/clips/actions/delete-comment.ts b/templates/clips/actions/delete-comment.ts index ed9e76db563..a28ff6b931b 100644 --- a/templates/clips/actions/delete-comment.ts +++ b/templates/clips/actions/delete-comment.ts @@ -6,7 +6,10 @@ import { and, eq, inArray } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; -import { isRecordingExpiredForViewer } from "../server/lib/recording-page-access.js"; +import { + isRecordingExpiredForViewer, + isRecordingTrashedForViewer, +} from "../server/lib/recording-page-access.js"; export default defineAction({ description: @@ -31,6 +34,14 @@ export default defineAction({ existing.recordingId, "viewer", ); + if ( + isRecordingTrashedForViewer({ + trashedAt: (access.resource as { trashedAt?: string | null }).trashedAt, + viewerIsOwner: access.role === "owner", + }) + ) { + throw new ForbiddenError(); + } if ( isRecordingExpiredForViewer({ expiresAt: (access.resource as { expiresAt?: string }).expiresAt, diff --git a/templates/clips/actions/list-notifications.test.ts b/templates/clips/actions/list-notifications.test.ts index 42851e6ce2b..3fe55f3d29c 100644 --- a/templates/clips/actions/list-notifications.test.ts +++ b/templates/clips/actions/list-notifications.test.ts @@ -9,6 +9,7 @@ const state = vi.hoisted(() => ({ password: null as string | null, expiresAt: null as string | null, trashedAt: null as string | null, + enableComments: true, }, })); const tables = vi.hoisted(() => ({ @@ -106,6 +107,7 @@ describe("list-notifications recording lifecycle", () => { state.recording.trashedAt = null; state.recording.expiresAt = null; state.recording.password = null; + state.recording.enableComments = true; }); it("withholds a trashed recording's mention from a nonowner", async () => { @@ -116,6 +118,17 @@ describe("list-notifications recording lifecycle", () => { }); }); + it.each(["viewer@example.com", "owner@example.com"])( + "preserves historical activity with comments disabled for %s", + async (email) => { + state.email = email; + state.recording.enableComments = false; + const result = await action.run({ days: 30, limit: 200 }); + expect(result.items[0]?.preview).toBe("Fake discussion"); + expect(result.count).toBe(1); + }, + ); + it.each([ { email: "owner@example.com", trashedAt: "2026-01-01T00:00:00Z" }, { email: "viewer@example.com", trashedAt: null }, From 71fb7a9b24c6e6f85dbbfa411457c7e01fb5fd73 Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Sun, 25 Oct 2026 14:58:31 -0400 Subject: [PATCH 4/7] Show unavailable state for missing shared recordings --- .../clips/app/lib/missing-share-retry.test.ts | 48 ----- .../clips/app/lib/missing-share-retry.ts | 20 -- .../app/routes/share-auth-loading.test.ts | 6 - .../clips/app/routes/share.$shareId.test.tsx | 187 ++++++++++++++++++ templates/clips/app/routes/share.$shareId.tsx | 24 +-- 5 files changed, 188 insertions(+), 97 deletions(-) delete mode 100644 templates/clips/app/lib/missing-share-retry.test.ts delete mode 100644 templates/clips/app/lib/missing-share-retry.ts create mode 100644 templates/clips/app/routes/share.$shareId.test.tsx diff --git a/templates/clips/app/lib/missing-share-retry.test.ts b/templates/clips/app/lib/missing-share-retry.test.ts deleted file mode 100644 index 44ee8cf14d5..00000000000 --- a/templates/clips/app/lib/missing-share-retry.test.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { describe, expect, it } from "vitest"; - -import { missingShareRetryDelayMs } from "./missing-share-retry"; - -describe("missingShareRetryDelayMs", () => { - it("backs off while a share the loader could not find is still missing", () => { - const delays = [1, 2, 3, 4, 5, 6, 7].map((updateCount) => - missingShareRetryDelayMs({ - status: 404, - updateCount, - loaderFoundClip: false, - }), - ); - expect(delays).toEqual([250, 500, 1000, 2000, 2000, 2000, 2000]); - }); - - it("gives up after the retry limit", () => { - expect( - missingShareRetryDelayMs({ - status: 404, - updateCount: 8, - loaderFoundClip: false, - }), - ).toBeNull(); - }); - - it("does not retry a 404 once the loader found the clip", () => { - expect( - missingShareRetryDelayMs({ - status: 404, - updateCount: 1, - loaderFoundClip: true, - }), - ).toBeNull(); - }); - - it("does not retry other statuses", () => { - for (const status of [200, 401, 403, 410, undefined]) { - expect( - missingShareRetryDelayMs({ - status, - updateCount: 1, - loaderFoundClip: false, - }), - ).toBeNull(); - } - }); -}); diff --git a/templates/clips/app/lib/missing-share-retry.ts b/templates/clips/app/lib/missing-share-retry.ts deleted file mode 100644 index 76a46eb58c5..00000000000 --- a/templates/clips/app/lib/missing-share-retry.ts +++ /dev/null @@ -1,20 +0,0 @@ -const MISSING_SHARE_RETRY_LIMIT = 8; -const MISSING_SHARE_RETRY_INTERVAL_MS = [250, 500, 1000, 2000] as const; - -// How long to wait before asking the share API again, or null to stop. A 404 -// means "the share row isn't written yet" only while the loader has not found -// the clip. Once it has (loaderFoundClip), the API returns 404 for a viewer -// without access, so retrying only delays the request-access screen. -export function missingShareRetryDelayMs({ - status, - updateCount, - loaderFoundClip, -}: { - status: number | undefined; - updateCount: number; - loaderFoundClip: boolean; -}): number | null { - if (status !== 404 || loaderFoundClip) return null; - if (updateCount >= MISSING_SHARE_RETRY_LIMIT) return null; - return MISSING_SHARE_RETRY_INTERVAL_MS[updateCount - 1] ?? 2000; -} diff --git a/templates/clips/app/routes/share-auth-loading.test.ts b/templates/clips/app/routes/share-auth-loading.test.ts index 3088693d9a8..033ac54b0c0 100644 --- a/templates/clips/app/routes/share-auth-loading.test.ts +++ b/templates/clips/app/routes/share-auth-loading.test.ts @@ -102,12 +102,6 @@ describe("authenticated recording route loading", () => { expect(processingView).not.toContain("ShareCopyRow"); }); - it("keeps transient missing share records loading while retrying locally", () => { - const route = readRoute("share.$shareId.tsx"); - expect(route).toContain("retryingMissingShare ||"); - expect(route).toContain("getQueryState(shareQueryKey)?.dataUpdateCount"); - }); - it("keeps expired share loader data impersonal for CDN caching", () => { const route = readRoute("share.$shareId.tsx"); diff --git a/templates/clips/app/routes/share.$shareId.test.tsx b/templates/clips/app/routes/share.$shareId.test.tsx new file mode 100644 index 00000000000..c68a6674c97 --- /dev/null +++ b/templates/clips/app/routes/share.$shareId.test.tsx @@ -0,0 +1,187 @@ +// @vitest-environment happy-dom + +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { MemoryRouter, Route, Routes } from "react-router"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + loaderData: {} as Record, + sessionStatus: "unauthenticated" as "unauthenticated" | "loading", +})); + +vi.mock("react-router", async (importOriginal) => ({ + ...(await importOriginal()), + useLoaderData: () => mocks.loaderData, +})); + +vi.mock("@agent-native/core/client/hooks", async (importOriginal) => ({ + ...(await importOriginal()), + useSession: () => ({ + session: null, + isLoading: mocks.sessionStatus === "loading", + status: mocks.sessionStatus, + error: null, + retry: () => {}, + }), + useActionMutation: () => ({ isPending: false, mutate: vi.fn() }), +})); + +vi.mock("@agent-native/core/client/i18n", async (importOriginal) => ({ + ...(await importOriginal()), + useT: () => (key: string) => key, +})); + +vi.mock("@agent-native/core/client/analytics", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("@agent-native/core/client/analytics") + >()), + trackEvent: vi.fn(), +})); + +vi.mock("../../server/db", () => ({ getDb: vi.fn(), schema: {} })); + +import ShareRoute from "./share.$shareId"; + +const SHARE_ID = "clip-1"; + +const trashedPublicClip = { + id: SHARE_ID, + title: "Retired demo", + description: "", + ownerInitial: "A", + brandLogoUrl: null, + thumbnailUrl: null, + animatedThumbnailUrl: null, + updatedAt: "2026-10-24T12:00:00.000Z", + visibility: "public", + status: "ready", + hasPassword: false, + archivedAt: null, + trashedAt: "2026-10-24T12:00:00.000Z", + isLoomEmbedBacked: false, +}; + +function loaderData(overrides: Record) { + return { + recording: null, + agentContextUrl: null, + origin: "http://localhost:3000", + shareUrl: null, + ...overrides, + }; +} + +describe("share page when the public recording read answers Not found", () => { + let container: HTMLDivElement; + let root: Root; + let queryClient: QueryClient; + let fetchMock: ReturnType; + + beforeEach(() => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + mocks.sessionStatus = "unauthenticated"; + fetchMock = vi.fn( + async () => + new Response(JSON.stringify({ error: "Not found" }), { + status: 404, + headers: { "content-type": "application/json" }, + }), + ); + vi.stubGlobal("fetch", fetchMock); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + queryClient = new QueryClient(); + }); + + afterEach(() => { + act(() => root.unmount()); + queryClient.clear(); + container.remove(); + vi.unstubAllGlobals(); + }); + + function render() { + act(() => { + root.render( + + + + } /> + + + , + ); + }); + } + + function publicRecordingReads() { + return fetchMock.mock.calls.filter(([url]) => + String(url).includes("/api/public-recording"), + ).length; + } + + function pageState() { + return { + heading: container.querySelector("h1")?.textContent ?? null, + loadingPlaceholder: container.querySelector(".skeleton-shimmer") !== null, + publicRecordingReads: publicRecordingReads(), + }; + } + + async function settle(ms: number) { + await act(() => new Promise((resolve) => setTimeout(resolve, ms))); + } + + it.each([ + ["a trashed clip", { recording: trashedPublicClip }], + ["a missing clip", {}], + ])("shows Clip unavailable at once for %s", async (_label, overrides) => { + mocks.loaderData = loaderData(overrides); + render(); + + expect(pageState()).toMatchObject({ + heading: null, + loadingPlaceholder: true, + }); + + await vi.waitFor(() => expect(publicRecordingReads()).toBe(1)); + await settle(400); + + expect(pageState()).toEqual({ + heading: "sharePage.clipUnavailable", + loadingPlaceholder: false, + publicRecordingReads: 1, + }); + }); + + it("keeps the request-access screen for a private clip", async () => { + mocks.loaderData = loaderData({ accessDeniedStatus: 401 }); + render(); + + await vi.waitFor(() => expect(publicRecordingReads()).toBe(1)); + await settle(400); + + expect(pageState()).toEqual({ + heading: "sharePage.privateClip", + loadingPlaceholder: false, + publicRecordingReads: 1, + }); + }); + + it("waits for the session before treating Not found as final", async () => { + mocks.sessionStatus = "loading"; + mocks.loaderData = loaderData({ recording: trashedPublicClip }); + render(); + + await vi.waitFor(() => expect(publicRecordingReads()).toBe(1)); + await settle(400); + + expect(pageState()).toMatchObject({ + heading: null, + loadingPlaceholder: true, + }); + }); +}); diff --git a/templates/clips/app/routes/share.$shareId.tsx b/templates/clips/app/routes/share.$shareId.tsx index 12275ae1d75..fee48f90946 100644 --- a/templates/clips/app/routes/share.$shareId.tsx +++ b/templates/clips/app/routes/share.$shareId.tsx @@ -122,7 +122,6 @@ import { usePlayerShortcuts } from "@/hooks/use-player-shortcuts"; import { useSonnerLifecycleToast } from "@/hooks/use-sonner-lifecycle-toast"; import { useViewTracking } from "@/hooks/use-view-tracking"; import { withMediaVersion } from "@/lib/media-url"; -import { missingShareRetryDelayMs } from "@/lib/missing-share-retry"; import { parsePlaybackSpeed } from "@/lib/playback-speed"; import { readRecoverableRecordingBackupForServerId, @@ -775,15 +774,6 @@ export default function ShareRoute() { enabled: !!shareId, refetchInterval: (q) => { const payload = (q.state.data as { data?: any } | undefined)?.data; - const status = (q.state.data as { status?: number } | undefined)?.status; - const updateCount = - queryClient.getQueryState(shareQueryKey)?.dataUpdateCount ?? 0; - const retryDelay = missingShareRetryDelayMs({ - status, - updateCount, - loaderFoundClip: Boolean(loaderData.accessDeniedStatus), - }); - if (retryDelay !== null) return retryDelay; const rec = payload?.recording; if (!rec) return false; // Poll while the recording is still being assembled / transcoded so the @@ -1321,19 +1311,7 @@ export default function ShareRoute() { retrySession(); }, [retrySession, sessionStatus, shareNeedsSession]); - const retryingMissingShare = - missingShareRetryDelayMs({ - status: dataQ.data?.status, - updateCount: - queryClient.getQueryState(shareQueryKey)?.dataUpdateCount ?? 0, - loaderFoundClip: Boolean(loaderData.accessDeniedStatus), - }) !== null; - - if ( - dataQ.isLoading || - retryingMissingShare || - (sessionNeedsRetry && shareNeedsSession) - ) { + if (dataQ.isLoading || (sessionNeedsRetry && shareNeedsSession)) { return ( <> {agentDiscovery} From 4669b0d15a01a60380eaf733d49c7972687593d8 Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:51:33 -0400 Subject: [PATCH 5/7] Keep unavailable recordings out of share metadata --- .../actions/get-recording-player-data.test.ts | 4 +- .../clips/actions/list-notifications.test.ts | 22 ++-- templates/clips/actions/view-screen.test.ts | 17 +-- templates/clips/actions/view-screen.ts | 5 +- .../clips/app/routes/share.$shareId.test.tsx | 101 +++++++++++++++++- templates/clips/app/routes/share.$shareId.tsx | 18 ++-- .../routes/api/public-recording.get.test.ts | 10 +- .../api/thumbnail/[recordingId].get.test.ts | 8 +- .../api/video/[recordingId].get.test.ts | 8 +- 9 files changed, 161 insertions(+), 32 deletions(-) diff --git a/templates/clips/actions/get-recording-player-data.test.ts b/templates/clips/actions/get-recording-player-data.test.ts index d7d7556fe86..7b9a4186fef 100644 --- a/templates/clips/actions/get-recording-player-data.test.ts +++ b/templates/clips/actions/get-recording-player-data.test.ts @@ -573,8 +573,8 @@ describe("get-recording-player-data view count", () => { const query: Record = {}; query.from = (table: unknown) => { isCommentsQuery = - (table as { videoTimestampMs?: string })?.videoTimestampMs === - "recordingComments.videoTimestampMs"; + (table as { videoTimestampMs?: string } | undefined) + ?.videoTimestampMs === "recordingComments.videoTimestampMs"; return query; }; query.where = () => query; diff --git a/templates/clips/actions/list-notifications.test.ts b/templates/clips/actions/list-notifications.test.ts index 3fe55f3d29c..add38ea6604 100644 --- a/templates/clips/actions/list-notifications.test.ts +++ b/templates/clips/actions/list-notifications.test.ts @@ -39,8 +39,10 @@ vi.mock("@agent-native/core/user-profile/server", () => ({ })); vi.mock("../server/lib/recordings.js", () => ({ getCurrentOwnerEmail: () => state.email, - sameOwnerEmail: (a: string, b: string) => - a?.toLowerCase() === b?.toLowerCase(), + sameOwnerEmail: ( + a: string | null | undefined, + b: string | null | undefined, + ) => a?.toLowerCase() === b?.toLowerCase(), })); vi.mock("../server/lib/user-identities.js", () => ({ profileNameFor: () => "Fake author", @@ -70,12 +72,16 @@ vi.mock("../server/db/index.js", () => ({ const rows = table === tables.recordings ? [ - Object.fromEntries( - Object.entries(projection!).map(([key, column]) => [ - key, - state.recording[column as keyof typeof state.recording], - ]), - ), + projection + ? Object.fromEntries( + Object.entries(projection).map(([key, column]) => [ + key, + state.recording[ + column as keyof typeof state.recording + ], + ]), + ) + : state.recording, ] : table === tables.recordingComments ? [ diff --git a/templates/clips/actions/view-screen.test.ts b/templates/clips/actions/view-screen.test.ts index a6deece9a98..b00ccdef869 100644 --- a/templates/clips/actions/view-screen.test.ts +++ b/templates/clips/actions/view-screen.test.ts @@ -187,8 +187,10 @@ vi.mock("../server/lib/recordings.js", () => ({ email, }), parseSpaceIds: vi.fn(), - sameOwnerEmail: (a: string, b: string) => - a?.toLowerCase() === b?.toLowerCase(), + sameOwnerEmail: ( + a: string | null | undefined, + b: string | null | undefined, + ) => a?.toLowerCase() === b?.toLowerCase(), })); vi.mock("../server/lib/user-identities.js", () => ({ @@ -251,11 +253,13 @@ describe("view-screen library view on a cold start", () => { }); describe("view-screen recording access", () => { + let recording: Record; + beforeEach(() => { vi.clearAllMocks(); context.navigation.view = "recording"; context.email = "viewer@example.com"; - context.recording = { + recording = { id: "rec-test", title: "Fake recording", ownerEmail: "owner@example.com", @@ -264,13 +268,14 @@ describe("view-screen recording access", () => { trashedAt: null, expiresAt: null, }; + context.recording = recording; }); it.each([ { trashedAt: "2026-01-01T00:00:00Z" }, { expiresAt: "2020-01-01T00:00:00Z" }, ])("withholds recording context after access ends: %j", async (settings) => { - Object.assign(context.recording!, settings); + Object.assign(recording, settings); const screen = JSON.parse(await action.run({})); expect(screen.recording).toBeUndefined(); expect(screen.comments).toBeUndefined(); @@ -282,7 +287,7 @@ describe("view-screen recording access", () => { "omits disabled comments for %s", async (email) => { context.email = email; - context.recording!.enableComments = false; + recording.enableComments = false; const screen = JSON.parse(await action.run({})); expect(screen.recording.id).toBe("rec-test"); expect(screen.comments).toEqual([]); @@ -306,7 +311,7 @@ describe("view-screen recording access", () => { "keeps owner previews and restored viewer context: %j", async ({ email, ...settings }) => { context.email = email; - Object.assign(context.recording!, settings); + Object.assign(recording, settings); const screen = JSON.parse(await action.run({})); expect(screen.recording.id).toBe("rec-test"); expect(screen.comments[0].content).toBe("Fake discussion"); diff --git a/templates/clips/actions/view-screen.ts b/templates/clips/actions/view-screen.ts index ee64ef84101..f415fc0bfed 100644 --- a/templates/clips/actions/view-screen.ts +++ b/templates/clips/actions/view-screen.ts @@ -88,7 +88,7 @@ function mapRecording(r: any) { async function fetchRecording(id: string) { const db = getDb(); - const [row] = await db + const rows = await db .select() .from(schema.recordings) .where( @@ -102,7 +102,8 @@ async function fetchRecording(id: string) { ), ), ); - if (!row) return null; + if (rows.length === 0) return null; + const [row] = rows; const viewerIsOwner = sameOwnerEmail(row.ownerEmail, getRequestUserEmail()); if ( isRecordingTrashedForViewer({ trashedAt: row.trashedAt, viewerIsOwner }) || diff --git a/templates/clips/app/routes/share.$shareId.test.tsx b/templates/clips/app/routes/share.$shareId.test.tsx index c68a6674c97..1d855b9c270 100644 --- a/templates/clips/app/routes/share.$shareId.test.tsx +++ b/templates/clips/app/routes/share.$shareId.test.tsx @@ -9,6 +9,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ loaderData: {} as Record, sessionStatus: "unauthenticated" as "unauthenticated" | "loading", + recordingRows: [] as Record[], })); vi.mock("react-router", async (importOriginal) => ({ @@ -40,9 +41,30 @@ vi.mock("@agent-native/core/client/analytics", async (importOriginal) => ({ trackEvent: vi.fn(), })); -vi.mock("../../server/db", () => ({ getDb: vi.fn(), schema: {} })); +vi.mock("../../server/db", () => ({ + getDb: () => ({ + select: () => ({ + from: () => ({ + where: () => ({ + limit: () => Promise.resolve(mocks.recordingRows), + }), + }), + }), + }), + schema: { recordings: {}, organizationSettings: {} }, +})); + +vi.mock("@agent-native/core/server", () => ({ + getRequestUserEmail: () => null, + signScopedAgentAccessToken: () => "access-request-token", + verifyScopedAgentAccessToken: () => ({ ok: false }), +})); + +vi.mock("@agent-native/core/sharing", () => ({ + resolveAccess: () => Promise.resolve(null), +})); -import ShareRoute from "./share.$shareId"; +import ShareRoute, { loader, meta } from "./share.$shareId"; const SHARE_ID = "clip-1"; @@ -185,3 +207,78 @@ describe("share page when the public recording read answers Not found", () => { }); }); }); + +describe("share page server data for a clip in Trash", () => { + function recordingRow(overrides: Record = {}) { + return { + id: SHARE_ID, + title: "Quarterly roadmap", + description: "Pricing changes for Q4", + thumbnailUrl: "https://media.example.test/clip-1/thumb.jpg", + animatedThumbnailUrl: null, + updatedAt: "2026-10-24T12:00:00.000Z", + sharePasswordVersion: null, + visibility: "public", + status: "ready", + ownerEmail: "owner@example.test", + organizationId: null, + password: null, + expiresAt: null, + archivedAt: null, + trashedAt: null, + sourceAppName: null, + videoUrl: "https://media.example.test/clip-1/video.mp4", + ...overrides, + }; + } + + async function loadShare(row?: Record) { + mocks.recordingRows = row ? [row] : []; + const pageData = await loader({ + params: { shareId: SHARE_ID }, + url: new URL(`https://clips.example.test/share/${SHARE_ID}`), + } as unknown as Parameters[0]); + const tags = meta({ loaderData: pageData } as unknown as Parameters< + typeof meta + >[0]); + return { pageData, tags }; + } + + it.each(["public", "private"])( + "serves a trashed %s clip the same page data and meta tags as a missing one", + async (visibility) => { + const missing = await loadShare(); + const trashed = await loadShare( + recordingRow({ visibility, trashedAt: "2026-10-24T12:00:00.000Z" }), + ); + + expect(trashed).toEqual(missing); + }, + ); + + it("keeps the title and image of a public clip that is not in Trash", async () => { + const { pageData, tags } = await loadShare(recordingRow()); + + expect(pageData).toMatchObject({ + recording: { title: "Quarterly roadmap" }, + }); + expect(tags).toContainEqual({ + property: "og:title", + content: "Quarterly roadmap ยท Clips", + }); + expect(tags).toContainEqual( + expect.objectContaining({ property: "og:image" }), + ); + }); + + it("keeps the request-access page data for a private clip", async () => { + const { pageData } = await loadShare( + recordingRow({ visibility: "private" }), + ); + + expect(pageData).toMatchObject({ + data: { recording: null, accessDeniedStatus: 401 }, + init: { status: 401 }, + }); + }); +}); diff --git a/templates/clips/app/routes/share.$shareId.tsx b/templates/clips/app/routes/share.$shareId.tsx index fee48f90946..611a1c46d99 100644 --- a/templates/clips/app/routes/share.$shareId.tsx +++ b/templates/clips/app/routes/share.$shareId.tsx @@ -140,7 +140,10 @@ import { parseRedactions } from "@/lib/video-redactions"; import { getDb, schema } from "../../server/db"; import { resolvePlayerThumbnailUrl } from "../../server/lib/player-thumbnail-url"; -import { isRecordingExpired } from "../../server/lib/recording-page-access"; +import { + isRecordingExpired, + isRecordingTrashedForViewer, +} from "../../server/lib/recording-page-access"; import { buildAgentApiUrls, buildAgentDiscoveryPayload, @@ -313,7 +316,13 @@ export async function loader({ params, url }: LoaderFunctionArgs) { }).ok : false; - if (isRecordingExpired(rec.expiresAt)) { + if ( + isRecordingExpired(rec.expiresAt) || + isRecordingTrashedForViewer({ + trashedAt: rec.trashedAt, + viewerIsOwner: false, + }) + ) { return shareLoaderData(emptyLoaderData(url), hasAgentAccessToken); } @@ -367,10 +376,7 @@ export async function loader({ params, url }: LoaderFunctionArgs) { isLoomEmbedBacked: isLoomEmbedBackedRecording(rec), }; const canExposeAnonymousAgentContext = - rec.visibility === "public" && - !rec.password && - !rec.archivedAt && - !rec.trashedAt; + rec.visibility === "public" && !rec.password && !rec.archivedAt; return shareLoaderData( { recording, 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 acde9ec0177..f3f1163eba2 100644 --- a/templates/clips/server/routes/api/public-recording.get.test.ts +++ b/templates/clips/server/routes/api/public-recording.get.test.ts @@ -249,7 +249,9 @@ describe("/api/public-recording route", () => { mockResolveAccess.mockResolvedValue({ role, resource: recording }); } - expect(await handler(event as any)).toEqual({ error: "Not found" }); + expect( + await handler(event as unknown as Parameters[0]), + ).toEqual({ error: "Not found" }); expect(mockSetResponseStatus).toHaveBeenCalledWith(event, 404); expect(mockSignShortLivedToken).not.toHaveBeenCalled(); expect(mockSignScopedAgentAccessToken).not.toHaveBeenCalled(); @@ -273,7 +275,11 @@ describe("/api/public-recording route", () => { }); mockResolveAccess.mockResolvedValue({ role, resource: recording }); - expect(await handler({ setCookies: [] } as any)).toMatchObject({ + expect( + await handler({ setCookies: [] } as unknown as Parameters< + typeof handler + >[0]), + ).toMatchObject({ recording: { id: "rec-1" }, }); }, diff --git a/templates/clips/server/routes/api/thumbnail/[recordingId].get.test.ts b/templates/clips/server/routes/api/thumbnail/[recordingId].get.test.ts index 883c19ec301..a8277342bcd 100644 --- a/templates/clips/server/routes/api/thumbnail/[recordingId].get.test.ts +++ b/templates/clips/server/routes/api/thumbnail/[recordingId].get.test.ts @@ -216,7 +216,9 @@ describe("/api/thumbnail/:recordingId route", () => { ); const event = makeEvent(); - expect(await handler(event as any)).toEqual({ error: "Not found" }); + expect( + await handler(event as unknown as Parameters[0]), + ).toEqual({ error: "Not found" }); expect(event.status).toBe(404); expect(fetch).not.toHaveBeenCalled(); expect(mockFetchS3ObjectByUrl).not.toHaveBeenCalled(); @@ -237,7 +239,9 @@ describe("/api/thumbnail/:recordingId route", () => { headers: { "content-type": "image/jpeg" }, }), ); - const result = await handler(makeEvent() as any); + const result = await handler( + makeEvent() as unknown as Parameters[0], + ); expect(result).toBeInstanceOf(Response); expect(await (result as Response).text()).toBe("thumbnail"); }, diff --git a/templates/clips/server/routes/api/video/[recordingId].get.test.ts b/templates/clips/server/routes/api/video/[recordingId].get.test.ts index 72367206e17..b24f0153735 100644 --- a/templates/clips/server/routes/api/video/[recordingId].get.test.ts +++ b/templates/clips/server/routes/api/video/[recordingId].get.test.ts @@ -222,7 +222,9 @@ describe("/api/video/:recordingId route", () => { const event = makeEvent(); event.query = { t: "existing-token" }; - expect(await handler(event as any)).toEqual({ error: "Not found" }); + expect( + await handler(event as unknown as Parameters[0]), + ).toEqual({ error: "Not found" }); expect(event.status).toBe(404); expect(fetch).not.toHaveBeenCalled(); expect(mockFetchS3ObjectByUrl).not.toHaveBeenCalled(); @@ -247,7 +249,9 @@ describe("/api/video/:recordingId route", () => { vi.mocked(fetch).mockResolvedValue( new Response("video", { headers: { "content-type": "video/mp4" } }), ); - const result = await handler(makeEvent() as any); + const result = await handler( + makeEvent() as unknown as Parameters[0], + ); expect(result).toBeInstanceOf(Response); expect(await (result as Response).text()).toBe("video"); }, From cf8b0225c2909b93dd010e4e903a1973070f31fa Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Sun, 25 Oct 2026 18:03:51 -0400 Subject: [PATCH 6/7] Explain replacement of share retry assertions --- templates/clips/app/routes/share-auth-loading.test.ts | 1 + templates/clips/app/routes/share.$shareId.test.tsx | 1 + 2 files changed, 2 insertions(+) diff --git a/templates/clips/app/routes/share-auth-loading.test.ts b/templates/clips/app/routes/share-auth-loading.test.ts index 033ac54b0c0..3e71e90a33d 100644 --- a/templates/clips/app/routes/share-auth-loading.test.ts +++ b/templates/clips/app/routes/share-auth-loading.test.ts @@ -102,6 +102,7 @@ describe("authenticated recording route loading", () => { expect(processingView).not.toContain("ShareCopyRow"); }); + // oracle-change: Missing share links no longer retry; share.$shareId.test.tsx verifies the rendered unavailable state instead of the removed source assertions. it("keeps expired share loader data impersonal for CDN caching", () => { const route = readRoute("share.$shareId.tsx"); diff --git a/templates/clips/app/routes/share.$shareId.test.tsx b/templates/clips/app/routes/share.$shareId.test.tsx index 1d855b9c270..b3aa0fd0c71 100644 --- a/templates/clips/app/routes/share.$shareId.test.tsx +++ b/templates/clips/app/routes/share.$shareId.test.tsx @@ -1,4 +1,5 @@ // @vitest-environment happy-dom +// oracle-change(templates/clips/app/lib/missing-share-retry.test.ts): Missing share links now become unavailable after the first 404; these rendering tests replace the deleted retry-helper tests. import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { act } from "react"; From fb8ecb640cbc264ad42e45b35eba911d645670c0 Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Sun, 25 Oct 2026 18:27:15 -0400 Subject: [PATCH 7/7] Respect recording lifecycle in bug report context --- templates/clips/actions/view-screen.test.ts | 80 +++++++++++++++++++-- templates/clips/actions/view-screen.ts | 9 ++- 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/templates/clips/actions/view-screen.test.ts b/templates/clips/actions/view-screen.test.ts index b00ccdef869..294d6009026 100644 --- a/templates/clips/actions/view-screen.test.ts +++ b/templates/clips/actions/view-screen.test.ts @@ -4,6 +4,7 @@ const context = vi.hoisted(() => ({ navigation: { view: "library", recordingId: "rec-test" }, recording: null as Record | null, email: "viewer@example.com", + role: null as "owner" | "editor" | null, childReads: vi.fn(), })); @@ -18,6 +19,7 @@ const TABLES = vi.hoisted(() => ({ updatedAt: "recordings.updatedAt", }, meetings: { recordingId: "meetings.recordingId" }, + recordingBugReports: { recordingId: "bugReports.recordingId" }, folders: { ownerEmail: "folders.ownerEmail", spaceId: "folders.spaceId", @@ -48,15 +50,23 @@ const makeRealDb = vi.hoisted(() => () => ({ } context.childReads(table); const rows = - table === "recordingComments" + table === TABLES.recordingBugReports ? [ { - id: "comment-test", - content: "Fake discussion", - authorEmail: "author@example.com", + projectId: "fake-project", + title: "Fake report", + severity: "low", }, ] - : []; + : table === "recordingComments" + ? [ + { + id: "comment-test", + content: "Fake discussion", + authorEmail: "author@example.com", + }, + ] + : []; return { where: () => Object.assign(Promise.resolve(rows), { @@ -143,7 +153,7 @@ vi.mock("@agent-native/core/server/request-context", () => ({ vi.mock("@agent-native/core/sharing", () => ({ accessFilter: () => ({ kind: "access-filter" }), - resolveAccess: async () => null, + resolveAccess: async () => (context.role ? { role: context.role } : null), })); vi.mock("drizzle-orm", () => ({ @@ -172,6 +182,7 @@ vi.mock("../server/db/index.js", () => ({ recordingComments: "recordingComments", recordingTranscripts: { recordingId: "transcripts.recordingId" }, recordingBrowserDiagnostics: { recordingId: "diagnostics.recordingId" }, + recordingBugReports: TABLES.recordingBugReports, }, })); @@ -211,6 +222,7 @@ describe("view-screen library view on a cold start", () => { beforeEach(() => { vi.clearAllMocks(); context.recording = null; + context.role = null; context.navigation.view = "library"; }); @@ -259,6 +271,7 @@ describe("view-screen recording access", () => { vi.clearAllMocks(); context.navigation.view = "recording"; context.email = "viewer@example.com"; + context.role = null; recording = { id: "rec-test", title: "Fake recording", @@ -295,6 +308,61 @@ describe("view-screen recording access", () => { }, ); + it.each([ + { trashedAt: "2026-01-01T00:00:00Z" }, + { expiresAt: "2020-01-01T00:00:00Z" }, + ])( + "withholds completed bug-report context after access ends: %j", + async (settings) => { + context.navigation.view = "bug-report-done"; + context.role = "editor"; + Object.assign(recording, settings); + + const screen = JSON.parse(await action.run({})); + + expect(screen.recording).toBeUndefined(); + expect(screen.bugReport).toBeUndefined(); + expect(context.childReads).not.toHaveBeenCalled(); + }, + ); + + it.each([ + { + email: "owner@example.com", + role: "owner" as const, + trashedAt: "2026-01-01T00:00:00Z", + expiresAt: null, + }, + { + email: "owner@example.com", + role: "owner" as const, + trashedAt: null, + expiresAt: "2020-01-01T00:00:00Z", + }, + { + email: "viewer@example.com", + role: "editor" as const, + trashedAt: null, + expiresAt: null, + }, + ])( + "keeps authorized completed bug-report context: %j", + async ({ email, role, ...settings }) => { + context.navigation.view = "bug-report-done"; + context.email = email; + context.role = role; + Object.assign(recording, settings); + + const screen = JSON.parse(await action.run({})); + + expect(screen.recording.id).toBe("rec-test"); + expect(screen.bugReport).toMatchObject({ + title: "Fake report", + projectId: "fake-project", + }); + }, + ); + it.each([ { email: "owner@example.com", diff --git a/templates/clips/actions/view-screen.ts b/templates/clips/actions/view-screen.ts index f415fc0bfed..46c2706bad0 100644 --- a/templates/clips/actions/view-screen.ts +++ b/templates/clips/actions/view-screen.ts @@ -673,11 +673,10 @@ export default defineAction({ } case "bug-report-done": { if (nav.recordingId) { - const [recording, bugReportSummary] = await Promise.all([ - fetchRecording(nav.recordingId), - fetchBugReportSummary(nav.recordingId), - ]); - if (recording) screen.recording = recording; + const recording = await fetchRecording(nav.recordingId); + if (!recording) break; + const bugReportSummary = await fetchBugReportSummary(nav.recordingId); + screen.recording = recording; if (bugReportSummary) screen.bugReport = bugReportSummary; } break;