Skip to content

Commit c397160

Browse files
committed
fix(webapp): address #3417 PR review feedback (round 2)
Two fixes from Devin review on the sessions-as-run-manager commit: - `SessionItem.currentRunId`'s contract is the `run_*` friendlyId, but `serializeSession` returns the raw Prisma cuid. The `POST /sessions` create path overrides correctly via a TaskRun lookup, but GET, PATCH, and the three return paths in close.ts were passing the cuid through. A consumer using `currentRunId` from those endpoints in a downstream `GET /api/v1/runs/:runId` call would 404. Add a `serializeSessionWithFriendlyRunId` helper next to `serializeSession` that resolves via `$replica.taskRun.findFirst` (TaskRun friendlyIds are immutable, so replica lag is harmless), and switch the five affected return sites to use it. List endpoints stay on `serializeSession` to avoid N+1 lookups when paginating. The create endpoint keeps its existing manual lookup because it also needs the friendlyId for the response's `runId` field, and `session.currentRunId` is stale relative to the post-`ensureRunForSession` claim outcome. - Drop dead `lastChunkType` recomputation in `streamResponseFromSessionStream`. The variable was bound but never used; the conditional below it re-evaluated the same expression. Use the bound value in the condition.
1 parent a349d02 commit c397160

4 files changed

Lines changed: 51 additions & 16 deletions

File tree

apps/webapp/app/routes/api.v1.sessions.$session.close.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { z } from "zod";
77
import { $replica, prisma } from "~/db.server";
88
import {
99
resolveSessionByIdOrExternalId,
10-
serializeSession,
10+
serializeSessionWithFriendlyRunId,
1111
} from "~/services/realtime/sessions.server";
1212
import { createActionApiRoute } from "~/services/routeBuilders/apiBuilder.server";
1313

@@ -43,7 +43,9 @@ const { action, loader } = createActionApiRoute(
4343
// Idempotent: if already closed, return the current row without clobbering
4444
// the original closedAt / closedReason.
4545
if (existing.closedAt) {
46-
return json<RetrieveSessionResponseBody>(serializeSession(existing));
46+
return json<RetrieveSessionResponseBody>(
47+
await serializeSessionWithFriendlyRunId(existing)
48+
);
4749
}
4850

4951
// `closedAt: null` on the where clause makes the update conditional at
@@ -61,12 +63,16 @@ const { action, loader } = createActionApiRoute(
6163
if (count === 0) {
6264
const final = await prisma.session.findFirst({ where: { id: existing.id } });
6365
if (!final) return json({ error: "Session not found" }, { status: 404 });
64-
return json<RetrieveSessionResponseBody>(serializeSession(final));
66+
return json<RetrieveSessionResponseBody>(
67+
await serializeSessionWithFriendlyRunId(final)
68+
);
6569
}
6670

6771
const updated = await prisma.session.findFirst({ where: { id: existing.id } });
6872
if (!updated) return json({ error: "Session not found" }, { status: 404 });
69-
return json<RetrieveSessionResponseBody>(serializeSession(updated));
73+
return json<RetrieveSessionResponseBody>(
74+
await serializeSessionWithFriendlyRunId(updated)
75+
);
7076
}
7177
);
7278

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { z } from "zod";
88
import { $replica, prisma } from "~/db.server";
99
import {
1010
resolveSessionByIdOrExternalId,
11-
serializeSession,
11+
serializeSessionWithFriendlyRunId,
1212
} from "~/services/realtime/sessions.server";
1313
import {
1414
createActionApiRoute,
@@ -34,7 +34,9 @@ export const loader = createLoaderApiRoute(
3434
},
3535
},
3636
async ({ resource: session }) => {
37-
return json<RetrieveSessionResponseBody>(serializeSession(session));
37+
return json<RetrieveSessionResponseBody>(
38+
await serializeSessionWithFriendlyRunId(session)
39+
);
3840
}
3941
);
4042

@@ -80,7 +82,9 @@ const { action } = createActionApiRoute(
8082
},
8183
});
8284

83-
return json<RetrieveSessionResponseBody>(serializeSession(updated));
85+
return json<RetrieveSessionResponseBody>(
86+
await serializeSessionWithFriendlyRunId(updated)
87+
);
8488
} catch (error) {
8589
// A duplicate externalId in the same environment violates the
8690
// `(runtimeEnvironmentId, externalId)` unique constraint. Surface that

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -329,11 +329,7 @@ export class S2RealtimeStreams implements StreamResponder, StreamIngestor {
329329
lastChunk != null && typeof lastChunk === "object"
330330
? (lastChunk as { type?: unknown }).type
331331
: null;
332-
if (
333-
lastChunk != null &&
334-
typeof lastChunk === "object" &&
335-
(lastChunk as { type?: unknown }).type === "trigger:turn-complete"
336-
) {
332+
if (lastChunkType === "trigger:turn-complete") {
337333
settled = true;
338334
waitSeconds = 0;
339335
}

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

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { PrismaClient, Session } from "@trigger.dev/database";
22
import type { SessionItem } from "@trigger.dev/core/v3";
3+
import { $replica } from "~/db.server";
34

45
/**
56
* Prefix that {@link SessionId.generate} attaches to every Session friendlyId.
@@ -74,10 +75,10 @@ export function canonicalSessionAddressingKey(
7475
*
7576
* Note: `currentRunId` is left as-is — Prisma stores the internal run id
7677
* (cuid), but `SessionItem.currentRunId` is the *friendly* form. Routes
77-
* that emit `SessionItem` are responsible for resolving the friendlyId
78-
* (typically via a separate TaskRun lookup) and overriding the field.
79-
* `serializeSession` returns it raw so list endpoints don't pay an N+1
80-
* lookup just to surface the friendly form.
78+
* that emit a single `SessionItem` should use
79+
* {@link serializeSessionWithFriendlyRunId} instead, which resolves the
80+
* friendlyId via a TaskRun lookup. List endpoints stay on this raw form
81+
* to avoid N+1 lookups when paginating.
8182
*/
8283
export function serializeSession(session: Session): SessionItem {
8384
return {
@@ -96,3 +97,31 @@ export function serializeSession(session: Session): SessionItem {
9697
updatedAt: session.updatedAt,
9798
};
9899
}
100+
101+
/**
102+
* Same as {@link serializeSession} but resolves `currentRunId` from the
103+
* internal cuid to the public `run_*` friendlyId via a TaskRun lookup.
104+
* Single-row endpoints (`POST/GET/PATCH/close /api/v1/sessions/:s`) use
105+
* this so the wire-side `currentRunId` is consistent with the rest of
106+
* the public API (which only accepts friendlyIds for run lookups).
107+
*
108+
* Skips the lookup when `currentRunId` is null. The read goes through
109+
* `$replica` — a TaskRun's `friendlyId` is immutable so replica lag is
110+
* harmless, and serializing on the writer would just add hot-path load.
111+
*/
112+
export async function serializeSessionWithFriendlyRunId(
113+
session: Session
114+
): Promise<SessionItem> {
115+
const base = serializeSession(session);
116+
if (!session.currentRunId) return base;
117+
118+
const run = await $replica.taskRun.findFirst({
119+
where: { id: session.currentRunId },
120+
select: { friendlyId: true },
121+
});
122+
123+
return {
124+
...base,
125+
currentRunId: run?.friendlyId ?? null,
126+
};
127+
}

0 commit comments

Comments
 (0)