mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
fix(runner): close the Greptile gaps on the real-process wait guard
Race each waitForCapabilityLiveProcess attempt against the deadline's remaining time, so a callback that never settles fails at the stated 10,000ms envelope instead of running past it. Keep the same deadline diagnostic (label, elapsed time, last observed error). Make the bare-real-process-wait guard inspect only a wait call's own options argument for an explicit timeout, not the whole argument list, so a same-shaped timeout option nested in the callback body no longer exempts a bare wait. Mask comments and string or template literals before the guard scans for vi.waitFor( or expect.poll(, so source text that only mentions those patterns can no longer trigger a false violation. Add regression tests for both gaps. Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
3 files changed
+256
-35
No files matched your search
@@ -90,3 +90,83 @@ test("allows an expect.poll( that passes an explicit timeout option", () => {
|
||||
|
||||
assert.deepEqual(findBareRealProcessWaits(source), []);
|
||||
});
|
||||
|
||||
test("rejects a vi.waitFor( whose callback body passes an unrelated timeout option", () => {
|
||||
const source = [
|
||||
"async function example() {",
|
||||
" await vi.waitFor(() => invoke({ timeout: 500 }));",
|
||||
"}",
|
||||
].join("\n");
|
||||
|
||||
assert.deepEqual(findBareRealProcessWaits(source), [
|
||||
{ line: 2, pattern: "vi.waitFor(" },
|
||||
]);
|
||||
});
|
||||
|
||||
test("allows a vi.waitFor( whose own second argument passes a timeout option, even with a same-shaped callback option", () => {
|
||||
const source = [
|
||||
"async function example() {",
|
||||
" await vi.waitFor(() => invoke({ timeout: 500 }), { timeout: 10_000 });",
|
||||
"}",
|
||||
].join("\n");
|
||||
|
||||
assert.deepEqual(findBareRealProcessWaits(source), []);
|
||||
});
|
||||
|
||||
test("ignores a vi.waitFor( written inside a line comment", () => {
|
||||
const source = [
|
||||
"async function example() {",
|
||||
" // do not call vi.waitFor( here, it is not spawned-process bound",
|
||||
" await Promise.resolve();",
|
||||
"}",
|
||||
].join("\n");
|
||||
|
||||
assert.deepEqual(findBareRealProcessWaits(source), []);
|
||||
});
|
||||
|
||||
test("ignores an expect.poll( written inside a block comment", () => {
|
||||
const source = [
|
||||
"async function example() {",
|
||||
" /* legacy note: this used to call",
|
||||
" expect.poll( on the old transport */",
|
||||
" await Promise.resolve();",
|
||||
"}",
|
||||
].join("\n");
|
||||
|
||||
assert.deepEqual(findBareRealProcessWaits(source), []);
|
||||
});
|
||||
|
||||
test("ignores a vi.waitFor( written inside a string literal", () => {
|
||||
const source = [
|
||||
"async function example() {",
|
||||
' const message = "replace this call with vi.waitFor( for a live wait";',
|
||||
" await Promise.resolve();",
|
||||
"}",
|
||||
].join("\n");
|
||||
|
||||
assert.deepEqual(findBareRealProcessWaits(source), []);
|
||||
});
|
||||
|
||||
test("ignores an expect.poll( written inside a template literal", () => {
|
||||
const source = [
|
||||
"async function example() {",
|
||||
" const message = `see expect.poll( for the retry loop`;",
|
||||
" await Promise.resolve();",
|
||||
"}",
|
||||
].join("\n");
|
||||
|
||||
assert.deepEqual(findBareRealProcessWaits(source), []);
|
||||
});
|
||||
|
||||
test("still flags a real bare vi.waitFor( on the line after a comment mentioning the pattern", () => {
|
||||
const source = [
|
||||
"async function example() {",
|
||||
" // vi.waitFor( is normally the wrong tool here, but this one needs it:",
|
||||
" await vi.waitFor(() => expect(x).toBe(1));",
|
||||
"}",
|
||||
].join("\n");
|
||||
|
||||
assert.deepEqual(findBareRealProcessWaits(source), [
|
||||
{ line: 3, pattern: "vi.waitFor(" },
|
||||
]);
|
||||
});
|
||||
@@ -48,59 +48,149 @@ async function collectSourceFiles(target) {
|
||||
return files;
|
||||
}
|
||||
|
||||
// Find the index of the close paren that matches the open paren at
|
||||
// `openParenIndex`, tracking nested `()`, `{}`, and `[]`, and skipping over
|
||||
// string and template literals so a brace or paren inside one never throws
|
||||
// off the count.
|
||||
function findMatchingCloseParenIndex(source, openParenIndex) {
|
||||
let depth = 0;
|
||||
for (let index = openParenIndex; index < source.length; index += 1) {
|
||||
// Replace every comment and every string- or template-literal body with
|
||||
// spaces, one for one, so the result keeps the same length and the same
|
||||
// line breaks as `source`. A line count taken from the result still lines
|
||||
// up with `source`, and neither a wait-call pattern nor a `timeout` option
|
||||
// written inside a comment or a literal can survive into the result.
|
||||
function maskCommentsAndStrings(source) {
|
||||
const masked = source.split("");
|
||||
let index = 0;
|
||||
while (index < source.length) {
|
||||
const twoCharacters = source.slice(index, index + 2);
|
||||
const character = source[index];
|
||||
if (twoCharacters === "//") {
|
||||
while (index < source.length && source[index] !== "\n") {
|
||||
masked[index] = " ";
|
||||
index += 1;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (twoCharacters === "/*") {
|
||||
masked[index] = " ";
|
||||
masked[index + 1] = " ";
|
||||
index += 2;
|
||||
while (index < source.length && source.slice(index, index + 2) !== "*/") {
|
||||
if (source[index] !== "\n") masked[index] = " ";
|
||||
index += 1;
|
||||
}
|
||||
if (index < source.length) {
|
||||
masked[index] = " ";
|
||||
masked[index + 1] = " ";
|
||||
index += 2;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (character === "'" || character === '"' || character === "`") {
|
||||
const quote = character;
|
||||
masked[index] = " ";
|
||||
index += 1;
|
||||
while (index < source.length && source[index] !== quote) {
|
||||
if (source[index] === "\\" && index + 1 < source.length) {
|
||||
masked[index] = " ";
|
||||
index += 1;
|
||||
if (source[index] !== "\n") masked[index] = " ";
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
if (source[index] !== "\n") masked[index] = " ";
|
||||
index += 1;
|
||||
}
|
||||
if (index < source.length) {
|
||||
masked[index] = " ";
|
||||
index += 1;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
index += 1;
|
||||
}
|
||||
return masked.join("");
|
||||
}
|
||||
|
||||
// Find the index of the close paren that matches the open paren at
|
||||
// `openParenIndex`, tracking nested `()`, `{}`, and `[]`. Callers pass a
|
||||
// comment- and string-masked source, so a stray bracket inside a comment or
|
||||
// a literal never throws off the count.
|
||||
function findMatchingCloseParenIndex(maskedSource, openParenIndex) {
|
||||
let depth = 0;
|
||||
for (let index = openParenIndex; index < maskedSource.length; index += 1) {
|
||||
const character = maskedSource[index];
|
||||
if (character === "(" || character === "{" || character === "[") {
|
||||
depth += 1;
|
||||
} else if (character === ")" || character === "}" || character === "]") {
|
||||
depth -= 1;
|
||||
if (depth === 0) return index;
|
||||
} else if (character === "'" || character === '"' || character === "`") {
|
||||
const quote = character;
|
||||
index += 1;
|
||||
while (index < source.length && source[index] !== quote) {
|
||||
if (source[index] === "\\") index += 1;
|
||||
index += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
return source.length;
|
||||
return maskedSource.length;
|
||||
}
|
||||
|
||||
// Split a call's argument-list text on its top-level commas, so a comma
|
||||
// nested inside a callback body, an object, or an array does not split a
|
||||
// single argument in two.
|
||||
function splitTopLevelArguments(maskedCallArguments) {
|
||||
const args = [];
|
||||
let depth = 0;
|
||||
let current = "";
|
||||
for (const character of maskedCallArguments) {
|
||||
if (character === "(" || character === "{" || character === "[") {
|
||||
depth += 1;
|
||||
current += character;
|
||||
} else if (character === ")" || character === "}" || character === "]") {
|
||||
depth -= 1;
|
||||
current += character;
|
||||
} else if (character === "," && depth === 0) {
|
||||
args.push(current);
|
||||
current = "";
|
||||
} else {
|
||||
current += character;
|
||||
}
|
||||
}
|
||||
args.push(current);
|
||||
return args;
|
||||
}
|
||||
|
||||
// Return the wait call's own options argument (`vi.waitFor(callback,
|
||||
// options)` or `expect.poll(callback, options)`), or `null` when the call
|
||||
// passes no second argument. Only this text, not the callback body, states
|
||||
// the call's own timeout.
|
||||
function findOptionsArgument(maskedSource, openParenIndex, closeParenIndex) {
|
||||
const callArguments = maskedSource.slice(openParenIndex + 1, closeParenIndex);
|
||||
const topLevelArguments = splitTopLevelArguments(callArguments);
|
||||
return topLevelArguments.length >= 2 ? topLevelArguments[1] : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Find each real-process-shaped wait call (`vi.waitFor(`, or `expect.poll(`,
|
||||
* including the two-line `expect\n .poll(` form) in `source` that has
|
||||
* neither an `IN_PROCESS_WAIT_MARKER` note on the line directly above it
|
||||
* nor an explicit `timeout` option in its own argument list. A call site
|
||||
* nor an explicit `timeout` option in its own options argument. A call site
|
||||
* with the marker declares, in place, that it settles fully in-process. A
|
||||
* call site with an explicit `timeout` option already states its own
|
||||
* envelope. Either one needs no real-process deadline.
|
||||
* envelope. Either one needs no real-process deadline. A pattern written
|
||||
* inside a comment or inside a string or template literal is source text,
|
||||
* not a call, and never counts as one.
|
||||
*/
|
||||
export function findBareRealProcessWaits(source) {
|
||||
const lines = source.split("\n");
|
||||
const masked = maskCommentsAndStrings(source);
|
||||
const found = [];
|
||||
for (const { name, regex } of BARE_WAIT_PATTERNS) {
|
||||
regex.lastIndex = 0;
|
||||
for (
|
||||
let match = regex.exec(source);
|
||||
let match = regex.exec(masked);
|
||||
match !== null;
|
||||
match = regex.exec(source)
|
||||
match = regex.exec(masked)
|
||||
) {
|
||||
const line = source.slice(0, match.index).split("\n").length;
|
||||
const line = masked.slice(0, match.index).split("\n").length;
|
||||
const precedingLine = lines[line - 2] ?? "";
|
||||
if (precedingLine.includes(IN_PROCESS_WAIT_MARKER)) {
|
||||
continue;
|
||||
}
|
||||
const openParenIndex = match.index + match[0].length - 1;
|
||||
const closeParenIndex = findMatchingCloseParenIndex(source, openParenIndex);
|
||||
const callArguments = source.slice(openParenIndex + 1, closeParenIndex);
|
||||
if (EXPLICIT_TIMEOUT_PATTERN.test(callArguments)) {
|
||||
const closeParenIndex = findMatchingCloseParenIndex(masked, openParenIndex);
|
||||
const optionsArgument = findOptionsArgument(masked, openParenIndex, closeParenIndex);
|
||||
if (optionsArgument !== null && EXPLICIT_TIMEOUT_PATTERN.test(optionsArgument)) {
|
||||
continue;
|
||||
}
|
||||
found.push({ line, pattern: name });
|
||||
|
||||
@@ -28,10 +28,59 @@
|
||||
// and for this helper's own 50ms poll granularity.
|
||||
export const CAPABILITY_LIVE_PROCESS_WAIT_DEADLINE_MS = 10_000;
|
||||
|
||||
// A sentinel error thrown when a single attempt runs past the wait's
|
||||
// remaining deadline. It never leaves `raceAttemptAgainstDeadline` to a
|
||||
// caller other than `waitForCapabilityLiveProcess`, so a plain class with no
|
||||
// message is enough to tell it apart from a real attempt failure.
|
||||
class CapabilityLiveProcessAttemptTimeout extends Error {}
|
||||
|
||||
// Run one attempt of `callback` against a deadline of `remainingMs`. A
|
||||
// pending attempt does not stop when the deadline wins the race, so this
|
||||
// attaches a no-op rejection handler to the attempt promise. Without that
|
||||
// handler, an attempt that rejects after its own race has already lost would
|
||||
// surface as an unhandled rejection instead of the deadline error below.
|
||||
async function raceAttemptAgainstDeadline<T>(
|
||||
callback: () => T | Promise<T>,
|
||||
remainingMs: number,
|
||||
): Promise<T> {
|
||||
const attempt = Promise.resolve().then(callback);
|
||||
attempt.catch(() => {});
|
||||
let timer: ReturnType<typeof setTimeout>;
|
||||
const timeout = new Promise<never>((_resolve, reject) => {
|
||||
timer = setTimeout(
|
||||
() => reject(new CapabilityLiveProcessAttemptTimeout()),
|
||||
remainingMs,
|
||||
);
|
||||
});
|
||||
try {
|
||||
return await Promise.race([attempt, timeout]);
|
||||
} finally {
|
||||
clearTimeout(timer!);
|
||||
}
|
||||
}
|
||||
|
||||
function buildDeadlineExceededError(
|
||||
label: string,
|
||||
elapsedMs: number,
|
||||
lastError: unknown,
|
||||
): Error {
|
||||
const detail =
|
||||
lastError instanceof Error
|
||||
? (lastError.stack ?? lastError.message)
|
||||
: String(lastError);
|
||||
return new Error(
|
||||
`Wait for "${label}" did not settle within ${CAPABILITY_LIVE_PROCESS_WAIT_DEADLINE_MS}ms ` +
|
||||
`(observed ${elapsedMs}ms). Last observed error: ${detail}`,
|
||||
{ cause: lastError },
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Poll a capability-live operation that settles only after a real spawned
|
||||
* operating-system process does work. Use a deadline derived from the real
|
||||
* process envelope described above. Unlike a bare `vi.waitFor` or
|
||||
* process envelope described above. Race each attempt against the
|
||||
* deadline's remaining time, so a callback that never settles fails at this
|
||||
* helper's deadline instead of bypassing it. Unlike a bare `vi.waitFor` or
|
||||
* `expect.poll`, report the last observed error when the deadline expires,
|
||||
* together with `label` and the elapsed time, so a failure names what the
|
||||
* wait was waiting for and how long it waited.
|
||||
@@ -46,22 +95,24 @@ export async function waitForCapabilityLiveProcess<T>(
|
||||
`no attempt of "${label}" settled before the deadline`,
|
||||
);
|
||||
for (;;) {
|
||||
const remainingMs = deadline - Date.now();
|
||||
if (remainingMs <= 0) {
|
||||
throw buildDeadlineExceededError(label, Date.now() - start, lastError);
|
||||
}
|
||||
try {
|
||||
return await callback();
|
||||
return await raceAttemptAgainstDeadline(callback, remainingMs);
|
||||
} catch (error) {
|
||||
if (error instanceof CapabilityLiveProcessAttemptTimeout) {
|
||||
throw buildDeadlineExceededError(
|
||||
label,
|
||||
Date.now() - start,
|
||||
lastError,
|
||||
);
|
||||
}
|
||||
lastError = error;
|
||||
}
|
||||
const elapsedMs = Date.now() - start;
|
||||
if (Date.now() >= deadline) {
|
||||
const detail =
|
||||
lastError instanceof Error
|
||||
? (lastError.stack ?? lastError.message)
|
||||
: String(lastError);
|
||||
throw new Error(
|
||||
`Wait for "${label}" did not settle within ${CAPABILITY_LIVE_PROCESS_WAIT_DEADLINE_MS}ms ` +
|
||||
`(observed ${elapsedMs}ms). Last observed error: ${detail}`,
|
||||
{ cause: lastError },
|
||||
);
|
||||
throw buildDeadlineExceededError(label, Date.now() - start, lastError);
|
||||
}
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user