Skip to content

Commit f5db19a

Browse files
committed
fix(webapp): stamp Session.chatSnapshotStoragePath at row creation
Address CodeRabbit findings on PR #3679: - Stamp `chatSnapshotStoragePath` once at session create + upsert-create paths so the new presign route only reads. Drops the lazy backfill branch (and its empty `.catch()`) from the route. - Return `json(...)` for the 405 in the action handler so Remix doesn't coerce the plain object into a 200 with a misleading body. - Extract `chatSnapshotStoragePathForSession(friendlyId)` next to the other session-addressing helpers so creation and the GET fallback agree on the canonical path.
1 parent ce895ca commit f5db19a

3 files changed

Lines changed: 35 additions & 32 deletions

File tree

apps/webapp/app/routes/api.v1.sessions.$sessionId.snapshot-url.ts

Lines changed: 15 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,30 @@
11
import type { ActionFunctionArgs } from "@remix-run/server-runtime";
22
import { json } from "@remix-run/server-runtime";
33
import { z } from "zod";
4-
import { $replica, prisma } from "~/db.server";
5-
import { env } from "~/env.server";
4+
import { $replica } from "~/db.server";
65
import { authenticateApiRequest } from "~/services/apiAuth.server";
7-
import { resolveSessionByIdOrExternalId } from "~/services/realtime/sessions.server";
6+
import {
7+
chatSnapshotStoragePathForSession,
8+
resolveSessionByIdOrExternalId,
9+
} from "~/services/realtime/sessions.server";
810
import { createLoaderApiRoute } from "~/services/routeBuilders/apiBuilder.server";
911
import { generatePresignedUrl } from "~/v3/objectStore.server";
1012

1113
const ParamsSchema = z.object({
1214
sessionId: z.string(),
1315
});
1416

15-
// Canonical key for new sessions, prefixed with the default protocol so
16-
// PUT/GET resolve to the same store on multi-provider deployments.
17-
function defaultSnapshotKey(sessionId: string): string {
18-
const path = `sessions/${sessionId}/snapshot.json`;
19-
const protocol = env.OBJECT_STORE_DEFAULT_PROTOCOL;
20-
return protocol ? `${protocol}://${path}` : path;
17+
// `chatSnapshotStoragePath` is stamped on every new Session at row creation
18+
// (see api.v1.sessions.ts). The fallback handles sessions created before
19+
// the column existed — read against the currently-configured default
20+
// protocol and compute the same path the SDK uploaded under.
21+
function snapshotKey(session: { friendlyId: string; chatSnapshotStoragePath: string | null }) {
22+
return session.chatSnapshotStoragePath ?? chatSnapshotStoragePathForSession(session.friendlyId);
2123
}
2224

2325
export async function action({ request, params }: ActionFunctionArgs) {
2426
if (request.method.toUpperCase() !== "PUT") {
25-
return { status: 405, body: "Method Not Allowed" };
27+
return json({ error: "Method Not Allowed" }, { status: 405 });
2628
}
2729

2830
const auth = await authenticateApiRequest(request);
@@ -40,31 +42,16 @@ export async function action({ request, params }: ActionFunctionArgs) {
4042
return json({ error: "Session not found" }, { status: 404 });
4143
}
4244

43-
// Reuse the stored path on subsequent writes; persist on first write so
44-
// future default-protocol changes don't strand existing snapshots.
45-
const key = session.chatSnapshotStoragePath ?? defaultSnapshotKey(parsed.sessionId);
46-
4745
const signed = await generatePresignedUrl(
4846
auth.environment.project.externalRef,
4947
auth.environment.slug,
50-
key,
48+
snapshotKey(session),
5149
"PUT"
5250
);
5351
if (!signed.success) {
5452
return json({ error: `Failed to generate presigned URL: ${signed.error}` }, { status: 500 });
5553
}
5654

57-
if (session.chatSnapshotStoragePath === null) {
58-
await prisma.session
59-
.updateMany({
60-
where: { id: session.id, chatSnapshotStoragePath: null },
61-
data: { chatSnapshotStoragePath: key },
62-
})
63-
.catch(() => {
64-
// Best-effort; concurrent writers may have already set it.
65-
});
66-
}
67-
6855
return json({ presignedUrl: signed.url });
6956
}
7057

@@ -76,18 +63,15 @@ export const loader = createLoaderApiRoute(
7663
findResource: async (params, auth) =>
7764
resolveSessionByIdOrExternalId($replica, auth.environment.id, params.sessionId),
7865
},
79-
async ({ params, authentication, resource: session }) => {
66+
async ({ authentication, resource: session }) => {
8067
if (!session) {
8168
return json({ error: "Session not found" }, { status: 404 });
8269
}
8370

84-
// Stored path wins; fall back to computed default for pre-column sessions.
85-
const key = session.chatSnapshotStoragePath ?? defaultSnapshotKey(params.sessionId);
86-
8771
const signed = await generatePresignedUrl(
8872
authentication.environment.project.externalRef,
8973
authentication.environment.slug,
90-
key,
74+
snapshotKey(session),
9175
"GET"
9276
);
9377
if (!signed.success) {

apps/webapp/app/routes/api.v1.sessions.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ import {
1717
ensureRunForSession,
1818
type SessionTriggerConfig,
1919
} from "~/services/realtime/sessionRunManager.server";
20-
import { serializeSession } from "~/services/realtime/sessions.server";
20+
import {
21+
chatSnapshotStoragePathForSession,
22+
serializeSession,
23+
} from "~/services/realtime/sessions.server";
2124
import { SessionsRepository } from "~/services/sessionsRepository/sessionsRepository.server";
2225
import {
2326
anyResource,
@@ -181,6 +184,7 @@ const { action } = createActionApiRoute(
181184
environmentType: authentication.environment.type,
182185
organizationId: authentication.environment.organizationId,
183186
streamBasinName: authentication.environment.organization.streamBasinName,
187+
chatSnapshotStoragePath: chatSnapshotStoragePathForSession(friendlyId),
184188
},
185189
update: { triggerConfig: triggerConfigJson },
186190
});
@@ -201,6 +205,7 @@ const { action } = createActionApiRoute(
201205
environmentType: authentication.environment.type,
202206
organizationId: authentication.environment.organizationId,
203207
streamBasinName: authentication.environment.organization.streamBasinName,
208+
chatSnapshotStoragePath: chatSnapshotStoragePathForSession(friendlyId),
204209
},
205210
});
206211
}

apps/webapp/app/services/realtime/sessions.server.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,20 @@
11
import type { PrismaClient, Session } from "@trigger.dev/database";
22
import type { SessionItem } from "@trigger.dev/core/v3";
33
import { $replica } from "~/db.server";
4+
import { env } from "~/env.server";
5+
6+
/**
7+
* Canonical storage URI for a session's chat.agent snapshot. Stamped on
8+
* `Session.chatSnapshotStoragePath` at row creation so PUT/GET presigns
9+
* resolve to the same store even if `OBJECT_STORE_DEFAULT_PROTOCOL`
10+
* changes later. The protocol prefix (when set) is what makes
11+
* generic-packets PUT and GET agree on the target provider.
12+
*/
13+
export function chatSnapshotStoragePathForSession(friendlyId: string): string {
14+
const path = `sessions/${friendlyId}/snapshot.json`;
15+
const protocol = env.OBJECT_STORE_DEFAULT_PROTOCOL;
16+
return protocol ? `${protocol}://${path}` : path;
17+
}
418

519
/**
620
* Prefix that {@link SessionId.generate} attaches to every Session friendlyId.

0 commit comments

Comments
 (0)