Skip to content

Commit 733af24

Browse files
Merge branch 'main' into feat/scoring-branch-eligibility-breakdown
2 parents 460313b + 5326fc7 commit 733af24

6 files changed

Lines changed: 112 additions & 8 deletions

File tree

review-enrichment/src/analyzers/doc-comment-drift.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type { EnrichRequest, DocCommentDriftFinding } from "../types.js";
1010
const MAX_FILES = 20;
1111
const MAX_FINDINGS = 50;
1212
const MAX_SIGNATURE_LINES = 40;
13+
const MAX_FETCH_BYTES = 1_000_000;
1314
const SOURCE_RE = /\.(?:ts|tsx|js|jsx|mjs|cjs)$/;
1415
const SKIP_RE = /(?:\.d\.ts$|\.min\.|\.test\.|\.spec\.|__tests__\/|(?:^|\/)tests?\/)/;
1516
const SLUG_RE = /^[A-Za-z0-9._-]+$/;
@@ -22,6 +23,34 @@ interface ScanOptions {
2223
signal?: AbortSignal;
2324
}
2425

26+
async function readBoundedText(resp: Response, signal?: AbortSignal): Promise<string | null> {
27+
const length = Number(resp.headers.get("content-length"));
28+
if (Number.isFinite(length) && length > MAX_FETCH_BYTES) return null;
29+
if (!resp.body) return null;
30+
31+
const reader = resp.body.getReader();
32+
const decoder = new TextDecoder();
33+
let size = 0;
34+
let text = "";
35+
try {
36+
while (true) {
37+
if (signal?.aborted) return null;
38+
const { done, value } = await reader.read();
39+
if (done) break;
40+
size += value.byteLength;
41+
if (size > MAX_FETCH_BYTES) {
42+
await reader.cancel();
43+
return null;
44+
}
45+
text += decoder.decode(value, { stream: true });
46+
}
47+
text += decoder.decode();
48+
return text;
49+
} finally {
50+
reader.releaseLock();
51+
}
52+
}
53+
2554
/** Reconstruct the pre-PR content of a file by reverse-applying its unified `patch` to the post-PR `newContent`:
2655
* context and removed (`-`) lines rebuild the old text; added (`+`) lines are dropped. Returns null if a hunk's
2756
* position runs past the content (so the caller falls back to "no old parameters" and reports nothing). Pure. */
@@ -325,7 +354,7 @@ export async function scanDocCommentDrift(
325354
`https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${path}?ref=${encodeURIComponent(headSha)}`,
326355
{ headers, signal: options.signal },
327356
);
328-
if (resp.ok) content = await resp.text();
357+
if (resp.ok) content = await readBoundedText(resp, options.signal);
329358
} catch {
330359
content = null;
331360
}

review-enrichment/test/doc-comment-drift.test.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ const baseReq = (files) => ({
1919
githubToken: "ght",
2020
files,
2121
});
22-
const fileWith = (content) => async () => ({ ok: true, text: async () => content });
23-
const status = (code) => async () => ({ ok: code >= 200 && code < 300, status: code, text: async () => "" });
22+
const fileWith = (content, init) => async () => new Response(content, init);
23+
const status = (code) => async () => new Response("", { status: code });
2424
const oldParams = (entries) => new Map(entries.map(([name, ids]) => [name, new Set(ids)]));
2525

2626
const DRIFTED = `/**\n * @param oldName the old one\n */\nexport function doThing(newName) {\n return newName;\n}\n`;
@@ -227,6 +227,43 @@ test("scanDocCommentDrift: fetches the file at headSha and reports drift", async
227227
assert.deepEqual(findings[0].staleParams, ["oldName"]);
228228
});
229229

230+
test("scanDocCommentDrift: skips oversized file responses before reading the body", async () => {
231+
let bodyAccessed = false;
232+
const out = await scanDocCommentDrift(
233+
baseReq([{ path: "src/a.ts", patch: DRIFT_PATCH }]),
234+
async () => ({
235+
ok: true,
236+
headers: new Headers({ "content-length": "1000001" }),
237+
get body() {
238+
bodyAccessed = true;
239+
return new Response(DRIFTED).body;
240+
},
241+
}),
242+
);
243+
assert.deepEqual(out, []);
244+
assert.equal(bodyAccessed, false);
245+
});
246+
247+
test("scanDocCommentDrift: cancels streamed file responses that exceed the byte cap", async () => {
248+
let canceled = false;
249+
const chunk = new Uint8Array(500_001);
250+
const stream = new ReadableStream({
251+
start(controller) {
252+
controller.enqueue(chunk);
253+
controller.enqueue(chunk);
254+
},
255+
cancel() {
256+
canceled = true;
257+
},
258+
});
259+
const out = await scanDocCommentDrift(
260+
baseReq([{ path: "src/a.ts", patch: DRIFT_PATCH }]),
261+
async () => new Response(stream),
262+
);
263+
assert.deepEqual(out, []);
264+
assert.equal(canceled, true);
265+
});
266+
230267
test("scanDocCommentDrift: requires a github token and a head sha", async () => {
231268
assert.deepEqual(await scanDocCommentDrift({ repoFullName: "o/r", prNumber: 1, headSha: "x", files: [{ path: "src/a.ts", patch: DRIFT_PATCH }] }, fileWith(DRIFTED)), []);
232269
assert.deepEqual(await scanDocCommentDrift({ repoFullName: "o/r", prNumber: 1, githubToken: "t", files: [{ path: "src/a.ts", patch: DRIFT_PATCH }] }, fileWith(DRIFTED)), []);

scripts/deploy-selfhost-image.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,8 @@ validate_inputs() {
129129
exit 1
130130
fi
131131
case "$image" in
132-
*[[:space:]\"\'\\]*)
133-
echo "error: image contains unsupported whitespace, quote, or backslash characters" >&2
132+
*[[:space:]\"\'\\\$\{\}]*)
133+
echo "error: image contains unsupported whitespace, quote, backslash, or compose interpolation characters" >&2
134134
exit 1
135135
;;
136136
esac

src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ export default {
5252
resetAt,
5353
}),
5454
);
55-
message.retry({ delaySeconds: delayUntil(resetAt) });
55+
await env.JOBS.send(message.body, { delaySeconds: delayUntil(resetAt) });
56+
message.ack();
5657
continue;
5758
}
5859
}

test/unit/index.test.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,12 @@ describe("worker entrypoint", () => {
169169
await recordGitHubRateLimitObservation(env, { repoFullName: "owner/repo", resource: "rest", path: "/x", statusCode: 200, limitValue: 5000, remaining: 120, resetAt: "2026-06-24T12:10:00.000Z", observedAt: "2026-06-24T12:00:00.000Z" });
170170
const acked: string[] = [];
171171
const retries: Array<{ delaySeconds?: number } | undefined> = [];
172+
const requeued: Array<{ message: import("../../src/types").JobMessage; delaySeconds?: number }> = [];
173+
env.JOBS = {
174+
async send(message: import("../../src/types").JobMessage, options?: { delaySeconds?: number }) {
175+
requeued.push({ message, ...(options?.delaySeconds === undefined ? {} : { delaySeconds: options.delaySeconds }) });
176+
},
177+
} as unknown as Queue;
172178
const batch = {
173179
messages: [
174180
{
@@ -182,8 +188,20 @@ describe("worker entrypoint", () => {
182188

183189
await worker.queue(batch, env);
184190

185-
expect(acked).toEqual([]);
186-
expect(retries).toEqual([{ delaySeconds: 615 }]);
191+
expect(acked).toEqual(["background-regate"]);
192+
expect(retries).toEqual([]);
193+
expect(requeued).toEqual([
194+
{
195+
message: {
196+
type: "agent-regate-pr",
197+
deliveryId: "sweep:owner/repo#7",
198+
repoFullName: "owner/repo",
199+
prNumber: 7,
200+
installationId: 123,
201+
},
202+
delaySeconds: 615,
203+
},
204+
]);
187205
vi.useRealTimers();
188206
});
189207

test/unit/selfhost-image-deploy.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,4 +192,23 @@ describe("self-host image deploy script", () => {
192192
harness.cleanup();
193193
}
194194
});
195+
196+
it.each([
197+
"registry.example/gittensory:${GITHUB_OAUTH_CLIENT_SECRET}",
198+
"registry.example/gittensory:$GITHUB_OAUTH_CLIENT_SECRET",
199+
"registry.example/gittensory:{GITHUB_OAUTH_CLIENT_SECRET}",
200+
])("rejects compose interpolation characters in image %s", (image) => {
201+
const { harness, result } = runHarness({ args: [image], envFile: "EXISTING=1\n" });
202+
try {
203+
expect(result.status).not.toBe(0);
204+
expect(result.stderr).toContain(
205+
"image contains unsupported whitespace, quote, backslash, or compose interpolation characters",
206+
);
207+
expect(readFileSync(harness.envPath, "utf8")).toBe("EXISTING=1\n");
208+
expect(harness.readImages()).toBe("");
209+
expect(harness.readCalls()).not.toContain(" pull ");
210+
} finally {
211+
harness.cleanup();
212+
}
213+
});
195214
});

0 commit comments

Comments
 (0)