Skip to content

Commit e444749

Browse files
d-csclaude
d-cs
and
claude
committed
fix(webapp): dashboard typecheck + Devin auth-throw swallowing
Three issues: - spans.\$spanParam/route.tsx loader returned a raw \`Response\` (204 on spanId-mismatch) alongside typedjson, collapsing the discriminated \`{type: "run"|"span"}\` union in the useTypedFetcher consumer. Switched to \`throw new Response(null, { status: 204 })\` so Remix exits cleanly without polluting the return type. - resources.taskruns.\$runParam.debug.ts loader's buffered branch returned \`engine: "V2"\` with \`runtimeEnvironment: null\`, widening the V2 variant in debugRun.tsx and breaking every MarQS / RunQueue key call site (run.runtimeEnvironment now nullable). Switched to \`engine: "BUFFERED" as const\` so debugRun.tsx narrows the buffered case to a dedicated panel — the V1/V2 panels recover their non-null runtimeEnvironment assumption. - Devin r3305402673: bare \`catch {}\` was swallowing the intentional \`throw new Response("Not Found", { status: 404 })\` auth-check fallthrough. Narrowed the try to only wrap \`buffer.getEntry\` (the one call that can transient-fail). Authorization and snapshot parsing now live outside the try, so an intentional 404 throw isn't masked by the transient-error catch. Keeps \`throw\` rather than Devin's suggested \`return\` because the consumer is typedjson-typed and a raw-Response return collapses the discriminated union. Also adds a dedicated DebugRunDataBuffered panel for buffered runs that renders the snapshot's queue / concurrencyKey / task identifier (buffered runs aren't on a PG queue, so the existing MarQS/RunQueue panels would 404 anyway). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b2aba52 commit e444749

3 files changed

Lines changed: 113 additions & 44 deletions

File tree

‎apps/webapp/app/components/admin/debugRun.tsx‎

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,66 @@ function DebugRunContent({ friendlyId }: { friendlyId: string }) {
6666
);
6767
}
6868

69+
// PG-resident variants (V1 + V2). Carries `run.runtimeEnvironment` as
70+
// non-null and the queue/env concurrency fields; the BUFFERED variant
71+
// has neither and renders via `DebugRunDataBuffered`.
72+
type PgVariant = Exclude<UseDataFunctionReturn<typeof loader>, { engine: "BUFFERED" }>;
73+
6974
function DebugRunData(props: UseDataFunctionReturn<typeof loader>) {
75+
// BUFFERED is the mollifier-buffer branch — no PG queue presence,
76+
// so the V1/V2 panels (which dereference run.runtimeEnvironment for
77+
// MarQS / RunQueue key construction) would crash. Render the
78+
// snapshot fields directly. Narrowing on `engine === "BUFFERED"`
79+
// here also strips the buffered variant from the union TS sees in
80+
// the V1/V2 branches below, restoring their non-null
81+
// runtimeEnvironment assumption. The casts are inside the
82+
// discriminated arms — `props.engine === "BUFFERED"` proves the
83+
// shape but TS doesn't follow the narrowing through a `{...props}`
84+
// spread, so we annotate the variant explicitly.
85+
if (props.engine === "BUFFERED") {
86+
return <DebugRunDataBuffered {...(props as BufferedVariant)} />;
87+
}
88+
7089
if (props.engine === "V1") {
71-
return <DebugRunDataEngineV1 {...props} />;
90+
return <DebugRunDataEngineV1 {...(props as PgVariant)} />;
7291
}
7392

74-
return <DebugRunDataEngineV2 {...props} />;
93+
return <DebugRunDataEngineV2 {...(props as PgVariant)} />;
94+
}
95+
96+
type BufferedVariant = Extract<UseDataFunctionReturn<typeof loader>, { engine: "BUFFERED" }>;
97+
98+
function DebugRunDataBuffered({ run }: BufferedVariant) {
99+
return (
100+
<Property.Table>
101+
<Property.Item>
102+
<Property.Label>ID</Property.Label>
103+
<Property.Value className="flex items-center gap-2">
104+
<ClipboardField value={run.id} variant="tertiary/small" iconButton />
105+
</Property.Value>
106+
</Property.Item>
107+
<Property.Item>
108+
<Property.Label>State</Property.Label>
109+
<Property.Value>Buffered (mollifier) — not yet materialised to Postgres</Property.Value>
110+
</Property.Item>
111+
<Property.Item>
112+
<Property.Label>Task identifier</Property.Label>
113+
<Property.Value>{run.taskIdentifier ?? "—"}</Property.Value>
114+
</Property.Item>
115+
<Property.Item>
116+
<Property.Label>Queue (from snapshot)</Property.Label>
117+
<Property.Value>{run.queue ?? "—"}</Property.Value>
118+
</Property.Item>
119+
<Property.Item>
120+
<Property.Label>Concurrency key (from snapshot)</Property.Label>
121+
<Property.Value>{run.concurrencyKey ?? "—"}</Property.Value>
122+
</Property.Item>
123+
<Property.Item>
124+
<Property.Label>Buffered at</Property.Label>
125+
<Property.Value>{new Date(run.queueTimestamp).toISOString()}</Property.Value>
126+
</Property.Item>
127+
</Property.Table>
128+
);
75129
}
76130

77131
function DebugRunDataEngineV1({
@@ -82,7 +136,7 @@ function DebugRunDataEngineV1({
82136
envCurrentConcurrency,
83137
queueReserveConcurrency,
84138
envReserveConcurrency,
85-
}: UseDataFunctionReturn<typeof loader>) {
139+
}: PgVariant) {
86140
const keys = new MarQSShortKeyProducer("marqs:");
87141

88142
const withPrefix = (key: string) => `marqs:${key}`;
@@ -354,7 +408,7 @@ function DebugRunDataEngineV2({
354408
envConcurrencyLimit,
355409
envCurrentConcurrency,
356410
keys,
357-
}: UseDataFunctionReturn<typeof loader>) {
411+
}: PgVariant) {
358412
return (
359413
<Property.Table>
360414
<Property.Item>

‎apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,11 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
142142
// Don't toast "Event not found" — that's noisy for the initial-render
143143
// request the dashboard fires before the root span auto-selects.
144144
// 204 No Content matches what the PG path returns for the same case.
145-
return new Response(null, { status: 204 });
145+
// THROWN so the loader's return type stays a clean TypedJson union;
146+
// returning a raw Response collapses the discriminated `type:
147+
// "run" | "span"` union into `Response` downstream and the
148+
// useTypedFetcher consumer loses .run / .span access.
149+
throw new Response(null, { status: 204 });
146150
}
147151

148152
const run = await buildSyntheticSpanRun({

‎apps/webapp/app/routes/resources.taskruns.$runParam.debug.ts‎

Lines changed: 50 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -51,47 +51,58 @@ export async function loader({ request, params }: LoaderFunctionArgs) {
5151
// snapshot so the Debug panel can show *something* instead of 404'ing.
5252
const buffer = getMollifierBuffer();
5353
if (buffer) {
54+
// The `try` only wraps the buffer.getEntry call — that's the
55+
// operation that can fail with a transient Redis error we want
56+
// to mask as a 404 fallthrough. Authorization and snapshot
57+
// parsing are split out below so a deliberate 404
58+
// (`throw new Response(...)`) or a malformed-snapshot crash
59+
// isn't silently swallowed.
60+
let entry: Awaited<ReturnType<typeof buffer.getEntry>> | null = null;
5461
try {
55-
const entry = await buffer.getEntry(runParam);
56-
if (entry) {
57-
// Same org-membership gate as the PG path above. Without it,
58-
// any authenticated user who knows a runId could read the
59-
// buffered run's queue/concurrencyKey snapshot across orgs.
60-
const member = await $replica.orgMember.findFirst({
61-
where: { userId, organizationId: entry.orgId },
62-
select: { id: true },
63-
});
64-
if (!member) {
65-
throw new Response("Not Found", { status: 404 });
66-
}
67-
const snapshot = deserialiseSnapshot<{
68-
taskIdentifier?: string;
69-
queue?: string;
70-
concurrencyKey?: string;
71-
}>(entry.payload);
72-
return typedjson({
73-
engine: "V2" as const,
74-
buffered: true,
75-
run: {
76-
id: entry.runId,
77-
engine: "V2" as const,
78-
friendlyId: entry.runId,
79-
queue: snapshot.queue ?? null,
80-
concurrencyKey: snapshot.concurrencyKey ?? null,
81-
queueTimestamp: entry.createdAt,
82-
runtimeEnvironment: null,
83-
},
84-
queueConcurrencyLimit: undefined,
85-
envConcurrencyLimit: undefined,
86-
queueCurrentConcurrency: undefined,
87-
envCurrentConcurrency: undefined,
88-
queueReserveConcurrency: undefined,
89-
envReserveConcurrency: undefined,
90-
keys: [],
91-
});
92-
}
62+
entry = await buffer.getEntry(runParam);
9363
} catch {
94-
// fall through to 404 on buffer error
64+
// Transient buffer failure — fall through to 404 below.
65+
}
66+
if (entry) {
67+
// Org-membership gate. Without it, any authenticated user who
68+
// knows a runId could read the buffered run's queue /
69+
// concurrencyKey snapshot across orgs. Thrown so the typedjson
70+
// discriminated union below isn't polluted by a raw `Response`
71+
// return — the consumer in `debugRun.tsx` switches on
72+
// `engine`, and a Response variant collapses the union.
73+
const member = await $replica.orgMember.findFirst({
74+
where: { userId, organizationId: entry.orgId },
75+
select: { id: true },
76+
});
77+
if (!member) {
78+
throw new Response("Not Found", { status: 404 });
79+
}
80+
const snapshot = deserialiseSnapshot<{
81+
taskIdentifier?: string;
82+
queue?: string;
83+
concurrencyKey?: string;
84+
}>(entry.payload);
85+
// `engine: "BUFFERED"` is a distinct discriminant from V1/V2
86+
// so the `debugRun.tsx` consumer can narrow on it. Returning
87+
// `engine: "V2"` here widened the V2 variant's
88+
// `run.runtimeEnvironment` to nullable, which broke the
89+
// MarQS-key call sites in the existing V1/V2 panels (they
90+
// assume non-null). A buffered run has no PG queue presence
91+
// so those panels would 404 anyway; the dedicated buffered
92+
// handler in debugRun.tsx renders the snapshot's queue +
93+
// concurrencyKey directly.
94+
return typedjson({
95+
engine: "BUFFERED" as const,
96+
buffered: true,
97+
run: {
98+
id: entry.runId,
99+
friendlyId: entry.runId,
100+
queue: snapshot.queue ?? null,
101+
concurrencyKey: snapshot.concurrencyKey ?? null,
102+
queueTimestamp: entry.createdAt,
103+
taskIdentifier: snapshot.taskIdentifier ?? null,
104+
},
105+
});
95106
}
96107
}
97108
throw new Response("Not Found", { status: 404 });

0 commit comments

Comments
 (0)