Skip to content

Commit ecc5601

Browse files
vincentkocvincentkoc
committed
fix(github): bound proof comment API bodies
1 parent 14795dc commit ecc5601

4 files changed

Lines changed: 370 additions & 44 deletions

File tree

‎scripts/github/real-behavior-proof-check.mjs‎

Lines changed: 110 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@ import {
66
evaluateClawSweeperExactHeadProof,
77
evaluateRealBehaviorProof,
88
isMaintainerTeamMember,
9+
readBoundedGitHubApiJson,
910
withGitHubApiTimeout,
1011
} from "./real-behavior-proof-policy.mjs";
1112

13+
const PROOF_COMMENTS_PER_PAGE = 100;
14+
const MAX_PROOF_COMMENT_PAGES = 10;
15+
1216
function escapeCommandValue(value) {
1317
return String(value)
1418
.replace(/%/g, "%25")
@@ -17,6 +21,100 @@ function escapeCommandValue(value) {
1721
.replace(/:/g, "%3A");
1822
}
1923

24+
function isTooLargeBodyError(error) {
25+
return error?.code === "ETOOBIG";
26+
}
27+
28+
async function fetchProofCommentPage({
29+
owner,
30+
repo,
31+
issueNumber,
32+
token,
33+
fetchImpl,
34+
timeoutMs,
35+
page,
36+
perPage,
37+
}) {
38+
const url = new URL(
39+
`https://api.github.com/repos/${owner}/${repo}/issues/${issueNumber}/comments`,
40+
);
41+
url.searchParams.set("per_page", String(perPage));
42+
url.searchParams.set("page", String(page));
43+
const response = await withGitHubApiTimeout(
44+
`proof comment lookup page ${page}`,
45+
timeoutMs,
46+
(signal) =>
47+
fetchImpl(url, {
48+
headers: {
49+
Accept: "application/vnd.github+json",
50+
Authorization: `Bearer ${token}`,
51+
"X-GitHub-Api-Version": "2022-11-28",
52+
},
53+
signal,
54+
}),
55+
);
56+
if (!response.ok) {
57+
throw new Error(`comments API returned ${response.status}`);
58+
}
59+
return await withGitHubApiTimeout(`proof comment response page ${page}`, timeoutMs, (signal) =>
60+
readBoundedGitHubApiJson(response, `proof comment response page ${page}`, undefined, {
61+
signal,
62+
}),
63+
);
64+
}
65+
66+
async function fetchOversizedProofCommentPageIndividually({
67+
owner,
68+
repo,
69+
issueNumber,
70+
token,
71+
fetchImpl,
72+
timeoutMs,
73+
page,
74+
perPage,
75+
}) {
76+
const comments = [];
77+
const firstSinglePage = (page - 1) * perPage + 1;
78+
for (let offset = 0; offset < perPage; offset += 1) {
79+
try {
80+
const pageComments = await fetchProofCommentPage({
81+
owner,
82+
repo,
83+
issueNumber,
84+
token,
85+
fetchImpl,
86+
timeoutMs,
87+
page: firstSinglePage + offset,
88+
perPage: 1,
89+
});
90+
comments.push(...pageComments);
91+
if (pageComments.length === 0) {
92+
return { comments, exhausted: true };
93+
}
94+
} catch (error) {
95+
if (!isTooLargeBodyError(error)) {
96+
throw error;
97+
}
98+
}
99+
}
100+
return { comments, exhausted: false };
101+
}
102+
103+
async function fetchProofCommentPageWithFallback(params) {
104+
try {
105+
const comments = await fetchProofCommentPage(params);
106+
return {
107+
comments,
108+
exhausted: comments.length < params.perPage,
109+
};
110+
} catch (error) {
111+
if (!isTooLargeBodyError(error) || params.perPage === 1) {
112+
throw error;
113+
}
114+
return await fetchOversizedProofCommentPageIndividually(params);
115+
}
116+
}
117+
20118
export async function fetchProofComments({
21119
owner,
22120
repo,
@@ -29,35 +127,19 @@ export async function fetchProofComments({
29127
for (const token of tokens.filter(Boolean)) {
30128
const comments = [];
31129
try {
32-
for (let page = 1; page <= 10; page += 1) {
33-
const url = new URL(
34-
`https://api.github.com/repos/${owner}/${repo}/issues/${issueNumber}/comments`,
35-
);
36-
url.searchParams.set("per_page", "100");
37-
url.searchParams.set("page", String(page));
38-
const response = await withGitHubApiTimeout(
39-
`proof comment lookup page ${page}`,
130+
for (let page = 1; page <= MAX_PROOF_COMMENT_PAGES; page += 1) {
131+
const result = await fetchProofCommentPageWithFallback({
132+
owner,
133+
repo,
134+
issueNumber,
135+
token,
136+
fetchImpl,
40137
timeoutMs,
41-
(signal) =>
42-
fetchImpl(url, {
43-
headers: {
44-
Accept: "application/vnd.github+json",
45-
Authorization: `Bearer ${token}`,
46-
"X-GitHub-Api-Version": "2022-11-28",
47-
},
48-
signal,
49-
}),
50-
);
51-
if (!response.ok) {
52-
throw new Error(`comments API returned ${response.status}`);
53-
}
54-
const pageComments = await withGitHubApiTimeout(
55-
`proof comment response page ${page}`,
56-
timeoutMs,
57-
() => response.json(),
58-
);
59-
comments.push(...pageComments);
60-
if (pageComments.length < 100) {
138+
page,
139+
perPage: PROOF_COMMENTS_PER_PAGE,
140+
});
141+
comments.push(...result.comments);
142+
if (result.exhausted) {
61143
break;
62144
}
63145
}

‎scripts/github/real-behavior-proof-policy.mjs‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1+
import { readBoundedResponseText } from "../lib/bounded-response.mjs";
2+
13
export const PROOF_OVERRIDE_LABEL = "proof: override";
24
export const PROOF_SUPPLIED_LABEL = "proof: supplied";
35
export const PROOF_SUFFICIENT_LABEL = "proof: sufficient";
46
export const NEEDS_REAL_BEHAVIOR_PROOF_LABEL = "triage: needs-real-behavior-proof";
57
export const MOCK_ONLY_PROOF_LABEL = "triage: mock-only-proof";
68
export const MAINTAINER_TEAM_SLUG = "maintainer";
79
export const DEFAULT_GITHUB_API_TIMEOUT_MS = 30_000;
10+
export const GITHUB_API_RESPONSE_BODY_MAX_BYTES = 1024 * 1024;
811

912
export const CLAWSWEEPER_PROOF_VERDICT_STATUS = "clawsweeper_exact_head_pass";
1013
const CLAWSWEEPER_BOT_LOGINS = new Set(["clawsweeper[bot]", "openclaw-clawsweeper[bot]"]);
@@ -88,6 +91,12 @@ function createTimeoutError(label, timeoutMs) {
8891
return error;
8992
}
9093

94+
function createTooLargeGitHubApiBodyError(label, maxBytes) {
95+
const error = new Error(`${label} response body exceeded ${maxBytes} bytes`);
96+
error.code = "ETOOBIG";
97+
return error;
98+
}
99+
91100
export async function withGitHubApiTimeout(label, timeoutMs, run) {
92101
const boundedTimeoutMs = Math.max(1, timeoutMs);
93102
const controller = new AbortController();
@@ -110,6 +119,19 @@ export async function withGitHubApiTimeout(label, timeoutMs, run) {
110119
}
111120
}
112121

122+
export async function readBoundedGitHubApiJson(
123+
response,
124+
label,
125+
maxBytes = GITHUB_API_RESPONSE_BODY_MAX_BYTES,
126+
options = {},
127+
) {
128+
const text = await readBoundedResponseText(response, label, maxBytes, {
129+
...options,
130+
createTooLargeError: () => createTooLargeGitHubApiBodyError(label, maxBytes),
131+
});
132+
return JSON.parse(text);
133+
}
134+
113135
function normalizeLineEndings(text = "") {
114136
return text.replace(/\r\n?/g, "\n");
115137
}
@@ -178,7 +200,10 @@ export async function isMaintainerTeamMember({
178200
const body = await withGitHubApiTimeout(
179201
`maintainer membership response for ${login}`,
180202
timeoutMs,
181-
() => response.json(),
203+
(signal) =>
204+
readBoundedGitHubApiJson(response, `maintainer membership response for ${login}`, undefined, {
205+
signal,
206+
}),
182207
);
183208
return body?.state === "active";
184209
}

‎test/scripts/real-behavior-proof-check.test.ts‎

Lines changed: 142 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,63 @@
11
import { describe, expect, it, vi } from "vitest";
22
import { fetchProofComments } from "../../scripts/github/real-behavior-proof-check.mjs";
33

4+
function stalledResponse() {
5+
let keepAlive: ReturnType<typeof setTimeout> | undefined;
6+
const reader = {
7+
read: () =>
8+
new Promise<ReadableStreamReadResult<Uint8Array>>(() => {
9+
keepAlive = setTimeout(() => {}, 10_000);
10+
}),
11+
cancel: vi.fn(() => {
12+
if (keepAlive) {
13+
clearTimeout(keepAlive);
14+
}
15+
return Promise.resolve();
16+
}),
17+
releaseLock: vi.fn(),
18+
};
19+
return {
20+
ok: true,
21+
status: 200,
22+
headers: new Headers(),
23+
body: {
24+
getReader: () => reader,
25+
},
26+
};
27+
}
28+
29+
function contentLengthResponse(contentLength: number) {
30+
const cancel = vi.fn(() => Promise.resolve());
31+
return {
32+
ok: true,
33+
status: 200,
34+
headers: new Headers({ "content-length": String(contentLength) }),
35+
body: { cancel },
36+
cancel,
37+
};
38+
}
39+
40+
function chunkedResponse(chunks: Uint8Array[]) {
41+
const cancel = vi.fn(() => Promise.resolve());
42+
const read = vi.fn();
43+
for (const chunk of chunks) {
44+
read.mockResolvedValueOnce({ done: false, value: chunk });
45+
}
46+
read.mockResolvedValueOnce({ done: true, value: undefined });
47+
return {
48+
ok: true,
49+
status: 200,
50+
headers: new Headers(),
51+
body: {
52+
getReader: () => ({
53+
read,
54+
cancel,
55+
releaseLock: vi.fn(),
56+
}),
57+
},
58+
};
59+
}
60+
461
describe("real-behavior-proof-check GitHub lookups", () => {
562
it("aborts stalled proof comment fetches", async () => {
663
const fetch = vi.fn((_url: URL, init: RequestInit) => {
@@ -22,11 +79,7 @@ describe("real-behavior-proof-check GitHub lookups", () => {
2279
});
2380

2481
it("times out stalled proof comment response bodies", async () => {
25-
const fetch = vi.fn().mockResolvedValue({
26-
ok: true,
27-
status: 200,
28-
json: () => new Promise(() => {}),
29-
});
82+
const fetch = vi.fn().mockResolvedValue(stalledResponse());
3083

3184
await expect(
3285
fetchProofComments({
@@ -39,4 +92,88 @@ describe("real-behavior-proof-check GitHub lookups", () => {
3992
}),
4093
).rejects.toThrow(/proof comment response page 1 timed out after 5ms/);
4194
});
95+
96+
it("skips oversized proof comment bodies by content length after narrow fallback", async () => {
97+
const response = contentLengthResponse(1024 * 1024 + 1);
98+
const fetch = vi.fn((url: URL) => {
99+
const perPage = url.searchParams.get("per_page");
100+
const page = url.searchParams.get("page");
101+
if ((perPage === "100" && page === "1") || (perPage === "1" && page === "1")) {
102+
return Promise.resolve(response);
103+
}
104+
return Promise.resolve(new Response("[]", { status: 200 }));
105+
});
106+
107+
await expect(
108+
fetchProofComments({
109+
fetchImpl: fetch as typeof globalThis.fetch,
110+
issueNumber: 123,
111+
owner: "openclaw",
112+
repo: "openclaw",
113+
tokens: ["tok"],
114+
}),
115+
).resolves.toEqual([]);
116+
expect(response.cancel).toHaveBeenCalled();
117+
});
118+
119+
it("skips oversized streamed proof comment bodies after narrow fallback", async () => {
120+
const encoder = new TextEncoder();
121+
const oversizedResponse = () =>
122+
chunkedResponse([
123+
encoder.encode("["),
124+
encoder.encode(" ".repeat(1024 * 1024)),
125+
encoder.encode("]"),
126+
]);
127+
const fetch = vi.fn((url: URL) => {
128+
const perPage = url.searchParams.get("per_page");
129+
const page = url.searchParams.get("page");
130+
if ((perPage === "100" && page === "1") || (perPage === "1" && page === "1")) {
131+
return Promise.resolve(oversizedResponse());
132+
}
133+
return Promise.resolve(new Response("[]", { status: 200 }));
134+
});
135+
136+
await expect(
137+
fetchProofComments({
138+
fetchImpl: fetch as typeof globalThis.fetch,
139+
issueNumber: 123,
140+
owner: "openclaw",
141+
repo: "openclaw",
142+
tokens: ["tok"],
143+
}),
144+
).resolves.toEqual([]);
145+
});
146+
147+
it("falls back to one-comment pages when a bulk comment page is oversized", async () => {
148+
const fetch = vi.fn((url: URL) => {
149+
const perPage = url.searchParams.get("per_page");
150+
const page = url.searchParams.get("page");
151+
if (perPage === "100" && page === "1") {
152+
return Promise.resolve(contentLengthResponse(1024 * 1024 + 1));
153+
}
154+
if (perPage === "1" && page === "1") {
155+
return Promise.resolve(contentLengthResponse(1024 * 1024 + 1));
156+
}
157+
if (perPage === "1" && page === "2") {
158+
return Promise.resolve(new Response('[{"id":2,"body":"trusted proof"}]', { status: 200 }));
159+
}
160+
return Promise.resolve(new Response("[]", { status: 200 }));
161+
});
162+
163+
await expect(
164+
fetchProofComments({
165+
fetchImpl: fetch as typeof globalThis.fetch,
166+
issueNumber: 123,
167+
owner: "openclaw",
168+
repo: "openclaw",
169+
tokens: ["tok"],
170+
}),
171+
).resolves.toEqual([{ id: 2, body: "trusted proof" }]);
172+
173+
expect(
174+
fetch.mock.calls.map(
175+
([url]) => `${url.searchParams.get("per_page")}:${url.searchParams.get("page")}`,
176+
),
177+
).toEqual(["100:1", "1:1", "1:2", "1:3"]);
178+
});
42179
});

0 commit comments

Comments
 (0)