Skip to content

Commit e03a185

Browse files
committed
fix(run-store): forward the parameters two members were dropping
Typing the forwarders closed the wrong-member and reordered-argument holes but not this one: omitting a trailing OPTIONAL argument still compiles. Two forwarders did exactly that, because the retyping pass read parameter names with a pattern that a preceding inline comment defeated, and both affected parameters happened to be optional and commented. The effects were silent and not small. findLatestExecutionSnapshot stopped applying its tenant scope, so a direct use of the base could read across the environment boundary. upsertWaitpointTag stopped applying its residency hint, so a tag write for a new-database environment would land on legacy. A source-level guard now asserts that every single-signature member forwards exactly the parameters it declares, in order. It reads the interface and the base and compares them, because that property is invisible to the compiler by definition. It carries a vacuity check, so a parse failure fails the suite instead of quietly matching nothing, and that check earned itself immediately by catching a parser that skipped every generic member. Verified: with a parameter dropped again, typecheck reports zero errors and the guard names the member and the missing argument.
1 parent 0f6c6d1 commit e03a185

2 files changed

Lines changed: 190 additions & 2 deletions

File tree

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
// Every declared parameter must actually reach the delegate.
2+
//
3+
// The compiler cannot check this. A forwarder that omits a trailing OPTIONAL argument compiles
4+
// cleanly, and the effect is silent: `findLatestExecutionSnapshot` would stop applying its tenant
5+
// scope, and `upsertWaitpointTag` would stop applying its residency hint, so a write would land on
6+
// the wrong database. Both of those shipped in this file before this test existed.
7+
//
8+
// So this reads the source of the base against the source of the interface and asserts that each
9+
// forward passes exactly the parameters its signature declares, in order. Source-level, because
10+
// that is the only place the property is visible.
11+
import { readFileSync } from "node:fs";
12+
import { join } from "node:path";
13+
import { describe, expect, it } from "vitest";
14+
15+
const dir = join(import.meta.dirname);
16+
const interfaceSource = readFileSync(join(dir, "types.ts"), "utf8");
17+
const baseSource = readFileSync(join(dir, "delegatingRunStore.ts"), "utf8");
18+
19+
/** Replaces comments and string bodies with spaces, so neither can shift a brace depth. */
20+
function blank(text: string): string {
21+
let out = "";
22+
let i = 0;
23+
while (i < text.length) {
24+
const two = text.slice(i, i + 2);
25+
if (two === "//") {
26+
const end = text.indexOf("\n", i);
27+
const stop = end === -1 ? text.length : end;
28+
out += " ".repeat(stop - i);
29+
i = stop;
30+
} else if (two === "/*") {
31+
const end = text.indexOf("*/", i + 2);
32+
const stop = end === -1 ? text.length : end + 2;
33+
out += text.slice(i, stop).replace(/[^\n]/g, " ");
34+
i = stop;
35+
} else if (text[i] === '"' || text[i] === "'" || text[i] === "`") {
36+
const quote = text[i];
37+
let j = i + 1;
38+
while (j < text.length && text[j] !== quote) j += text[j] === "\\" ? 2 : 1;
39+
out += quote + " ".repeat(Math.max(0, j - i - 1)) + (text[j] ?? "");
40+
i = j + 1;
41+
} else {
42+
out += text[i];
43+
i += 1;
44+
}
45+
}
46+
return out;
47+
}
48+
49+
function interfaceBody(source: string): string {
50+
const blanked = blank(source);
51+
const decl = "export interface RunStore {";
52+
const start = blanked.indexOf(decl) + decl.length;
53+
let depth = 1;
54+
let end = start;
55+
while (depth > 0 && end < blanked.length) {
56+
if (blanked[end] === "{") depth += 1;
57+
else if (blanked[end] === "}") depth -= 1;
58+
if (depth > 0) end += 1;
59+
}
60+
return blanked.slice(start, end);
61+
}
62+
63+
/** Splits a balanced parameter list on top-level commas. */
64+
function splitParams(signature: string): string[] {
65+
// A generic member reads `name<T extends X>(...)`, so the parameter list starts after the
66+
// balanced angle block, not at the first parenthesis.
67+
let searchFrom = 0;
68+
const angle = signature.indexOf("<");
69+
const paren = signature.indexOf("(");
70+
if (angle !== -1 && angle < paren) {
71+
let angleDepth = 0;
72+
for (let i = angle; i < signature.length; i++) {
73+
if (signature[i] === "<") angleDepth += 1;
74+
else if (signature[i] === ">") {
75+
angleDepth -= 1;
76+
if (angleDepth === 0) {
77+
searchFrom = i;
78+
break;
79+
}
80+
}
81+
}
82+
}
83+
84+
const open = signature.indexOf("(", searchFrom);
85+
let depth = 0;
86+
let close = open;
87+
for (let i = open; i < signature.length; i++) {
88+
if ("([{<".includes(signature[i]!)) depth += 1;
89+
else if (")]}>".includes(signature[i]!)) {
90+
depth -= 1;
91+
if (depth === 0) {
92+
close = i;
93+
break;
94+
}
95+
}
96+
}
97+
const inner = signature.slice(open + 1, close);
98+
const parts: string[] = [];
99+
let level = 0;
100+
let current = "";
101+
for (const ch of inner) {
102+
if ("([{<".includes(ch)) level += 1;
103+
else if (")]}>".includes(ch)) level -= 1;
104+
if (ch === "," && level === 0) {
105+
parts.push(current);
106+
current = "";
107+
} else {
108+
current += ch;
109+
}
110+
}
111+
if (current.trim()) parts.push(current);
112+
return parts;
113+
}
114+
115+
function paramNames(signature: string): string[] {
116+
return splitParams(signature)
117+
.map((p) => /^\s*([A-Za-z_$][\w$]*)\s*\??\s*:/.exec(p)?.[1])
118+
.filter((n): n is string => Boolean(n));
119+
}
120+
121+
/** Member name to its declared parameter names, for members with a single signature. */
122+
function declaredParams(): Map<string, string[]> {
123+
const body = interfaceBody(interfaceSource);
124+
const spans: string[] = [];
125+
let level = 0;
126+
let from = 0;
127+
for (let i = 0; i < body.length; i++) {
128+
const ch = body[i]!;
129+
if ("{([".includes(ch)) level += 1;
130+
else if ("})]".includes(ch)) level -= 1;
131+
else if (ch === ";" && level === 0) {
132+
spans.push(body.slice(from, i));
133+
from = i + 1;
134+
}
135+
}
136+
137+
const seen = new Map<string, string[][]>();
138+
for (const span of spans) {
139+
const match = /^\s*(?:readonly\s+)?([A-Za-z_$][\w$]*)\s*\??\s*[(<]/.exec(span);
140+
if (!match) continue;
141+
const name = match[1]!;
142+
seen.set(name, [...(seen.get(name) ?? []), paramNames(span)]);
143+
}
144+
145+
// Overloaded members forward through a cast and apply the whole argument list, so they are not
146+
// subject to this check.
147+
return new Map(
148+
[...seen].filter(([, sigs]) => sigs.length === 1).map(([n, sigs]) => [n, sigs[0]!])
149+
);
150+
}
151+
152+
describe("the pass-through forwards every declared parameter", () => {
153+
const declared = declaredParams();
154+
155+
it("parsed the interface, so a parse failure cannot pass this suite", () => {
156+
expect(declared.size).toBeGreaterThan(50);
157+
expect(declared.get("expireParkedRun")).toEqual(["runId", "data", "tx"]);
158+
expect(declared.get("findLatestExecutionSnapshot")).toEqual([
159+
"runId",
160+
"client",
161+
"environmentId",
162+
]);
163+
});
164+
165+
it("passes exactly the declared parameters, in order, for every single-signature member", () => {
166+
const wrong: string[] = [];
167+
168+
for (const [name, params] of declared) {
169+
const forward = new RegExp(`return this\\.delegate\\.${name}\\(([^;]*)\\);`).exec(baseSource);
170+
171+
if (!forward) {
172+
wrong.push(`${name}: no forward found`);
173+
continue;
174+
}
175+
176+
const passed = forward[1]!
177+
.split(",")
178+
.map((a) => a.trim())
179+
.filter(Boolean);
180+
181+
if (passed.join(",") !== params.join(",")) {
182+
wrong.push(`${name}: declares (${params.join(", ")}) but forwards (${passed.join(", ")})`);
183+
}
184+
}
185+
186+
expect(wrong).toEqual([]);
187+
});
188+
});

internal-packages/run-store/src/delegatingRunStore.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ export class DelegatingRunStore implements RunStore {
459459
): Promise<Prisma.TaskRunExecutionSnapshotGetPayload<{
460460
include: { completedWaitpoints: true; checkpoint: true };
461461
}> | null> {
462-
return this.delegate.findLatestExecutionSnapshot(runId, client);
462+
return this.delegate.findLatestExecutionSnapshot(runId, client, environmentId);
463463
}
464464

465465
findExecutionSnapshot<T extends Prisma.TaskRunExecutionSnapshotFindFirstArgs>(
@@ -717,7 +717,7 @@ export class DelegatingRunStore implements RunStore {
717717
// instead of defaulting to LEGACY. Single-store impls ignore it.
718718
residency?: Residency
719719
): Promise<WaitpointTag> {
720-
return this.delegate.upsertWaitpointTag(data, tx);
720+
return this.delegate.upsertWaitpointTag(data, tx, residency);
721721
}
722722

723723
findManyWaitpointTags(

0 commit comments

Comments
 (0)