trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRunsg rEventsFlows

Sentinel: release was degraded 180 minutes today: Release 0.7.495 is unproven: https://forge.open-platform.sh/search?q=a as desktop /search?q=a did not answer within 200… #10420

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/10395-sentinel-release-was-degraded-180-minute → main 1 commit fork

  • Merged. scout/10395-sentinel-release-was-degraded-180-minute was merged into main as 06a5e01.

Review

Sign in to review this change.
3 files changed +127 −7
src/core/search.ts +7 −3
@@ -417,7 +417,8 @@ export function searchLocalIssues(
417417 * a query starting with `-` a pattern rather than a flag; `--` ends the
418418 * options so a query containing `--` cannot re-open them; `-F` makes the
419419 * query a fixed string, so a search box cannot hand git a pathological
420 * regex; `-I` skips binaries; `--max-count` and the timeout bound the work.
420 * regex; `-I` skips binaries; `--max-count` (per file), the read that stops
421 * after `maxCount` lines and the timeout bound the work.
421422 * The ref goes through `isSafeRef` like every other rev in the forge.
422423 */
423424 export async function grepRepo(
@@ -463,12 +464,15 @@ async function grepRun(
463464 "--",
464465 ".",
465466 ],
466 { cwd: dir, timeoutMs },
467 // `--max-count` is per file: `a` in a repository of thousands of files
468 // is megabytes of matches. Only the first `maxCount` are kept, so git is
469 // stopped once they are read rather than left to write the rest (#10395).
470 { cwd: dir, timeoutMs, stopAfterLines: maxCount },
467471 );
468472 // Exit 1 is "no match" — data, not failure. Anything else (a repo with no
469473 // commits, a killed process) is simply no results from this repo.
470474 if (run.timedOut === true) return "timeout";
471 if (run.code !== 0 && run.code !== 1) return null;
475 if (run.stopped !== true && run.code !== 0 && run.code !== 1) return null;
472476
473477 // `-z` makes each record `ref:path NUL line NUL text`, so a path with a
474478 // colon in it (`a:1:b.txt`) cannot be split at the wrong colon.
src/git/env.ts +61 −4
@@ -302,6 +302,57 @@ export interface GitCapture {
302302 readonly stderr: string;
303303 /** The timeout killed the process. */
304304 readonly timedOut?: boolean;
305 /** `stopAfterLines` was reached: stdout holds that many lines and the process was killed for it. */
306 readonly stopped?: boolean;
307 }
308
309 export interface CaptureOptions extends RunOptions {
310 /**
311 * Keep only the first this-many newline-ended lines of stdout and kill the
312 * process once they are read. For a caller that wants the first few
313 * answers of a command whose whole output can be megabytes (`git grep a`).
314 */
315 stopAfterLines?: number;
316 }
317
318 /**
319 * The first `max` lines of `stream`, read as they come; `stop` is called
320 * once they are in, and nothing past them is kept. Without `max`, all of it.
321 */
322 async function readLines(
323 stream: ReadableStream<Uint8Array>,
324 max: number | undefined,
325 stop: () => void,
326 ): Promise<{ text: string; stopped: boolean }> {
327 if (max === undefined) return { text: await new Response(stream).text(), stopped: false };
328 const decoder = new TextDecoder();
329 const parts: string[] = [];
330 let lines = 0;
331 const reader = stream.getReader();
332 try {
333 for (;;) {
334 const { done, value } = await reader.read();
335 if (done) break;
336 let cut = -1;
337 for (let i = 0; i < value.length; i++) {
338 if (value[i] === 10 && ++lines >= max) {
339 cut = i + 1;
340 break;
341 }
342 }
343 if (cut < 0) {
344 parts.push(decoder.decode(value, { stream: true }));
345 continue;
346 }
347 parts.push(decoder.decode(value.subarray(0, cut)));
348 stop();
349 return { text: parts.join(""), stopped: true };
350 }
351 parts.push(decoder.decode());
352 return { text: parts.join(""), stopped: false };
353 } finally {
354 reader.cancel().catch(() => {});
355 }
305356 }
306357
307358 /**
@@ -311,7 +362,7 @@ export interface GitCapture {
311362 */
312363 export async function runGitCapture(
313364 args: readonly string[],
314 options: RunOptions = {},
365 options: CaptureOptions = {},
315366 ): Promise<GitCapture> {
316367 const start = performance.now();
317368 return gitGate.run(async () => {
@@ -331,12 +382,18 @@ export async function runGitCapture(
331382 options.timeoutMs ?? 30_000,
332383 );
333384 try {
334 const [stdout, stderr, code] = await Promise.all([
335 new Response(proc.stdout).text(),
385 const [out, stderr, code] = await Promise.all([
386 readLines(proc.stdout, options.stopAfterLines, () => proc.kill("SIGKILL")),
336387 new Response(proc.stderr).text(),
337388 proc.exited,
338389 ]);
339 return { code, stdout, stderr, ...(timedOut ? { timedOut } : {}) };
390 return {
391 code,
392 stdout: out.text,
393 stderr,
394 ...(timedOut && !out.stopped ? { timedOut } : {}),
395 ...(out.stopped ? { stopped: true } : {}),
396 };
340397 } finally {
341398 clearTimeout(timer);
342399 noteGit(gitRecord, args, start);
test/search-grep-stops.test.ts added +59
@@ -0,0 +1,59 @@
1 // #10395: `git grep --max-count` is per file, so a one-letter query in a
2 // repository of many files wrote megabytes that the forge read whole on its
3 // event loop. The read now stops git once the hits it keeps are in.
4 import { afterAll, beforeAll, expect, test } from "bun:test";
5 import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
6 import { tmpdir } from "node:os";
7 import { join } from "node:path";
8 import { runGitCapture } from "../src/git/env.ts";
9 import { grepRepo } from "../src/core/search.ts";
10
11 const FILES = 400;
12 const LINES = 30;
13 let dir = "";
14
15 beforeAll(() => {
16 dir = mkdtempSync(join(tmpdir(), "forge-grep-stops-"));
17 const line = "a line that matches a\n";
18 for (let i = 0; i < FILES; i++) writeFileSync(join(dir, `f${i}.txt`), line.repeat(LINES));
19 const git = (...args: string[]) => {
20 const r = Bun.spawnSync(["git", "-c", "user.name=t", "-c", "[email protected]", ...args], { cwd: dir });
21 if (r.exitCode !== 0) throw new Error(`git ${args[0]}: ${r.stderr.toString()}`);
22 };
23 git("init", "--quiet", "--initial-branch=main");
24 git("add", ".");
25 git("commit", "--quiet", "-m", "many");
26 });
27
28 afterAll(() => rmSync(dir, { recursive: true, force: true }));
29
30 const grepArgs = ["grep", "-z", "-n", "-I", "-i", "-F", "--max-count=20", "-e", "a", "main", "--", "."];
31
32 test("without a stop, the per-file cap still lets git write every file's matches", async () => {
33 const run = await runGitCapture(grepArgs, { cwd: dir });
34 expect(run.stdout.split("\n").filter(Boolean).length).toBe(FILES * 20);
35 expect(run.stopped).toBeUndefined();
36 });
37
38 test("stopAfterLines keeps exactly that many lines and stops git", async () => {
39 const run = await runGitCapture(grepArgs, { cwd: dir, stopAfterLines: 20 });
40 expect(run.stopped).toBe(true);
41 expect(run.timedOut).toBeUndefined();
42 const lines = run.stdout.split("\n");
43 expect(lines.pop()).toBe("");
44 expect(lines.length).toBe(20);
45 expect(lines.every((l) => l.startsWith("main:"))).toBe(true);
46 });
47
48 test("a stop that is never reached reads the whole output", async () => {
49 const run = await runGitCapture(["grep", "-n", "-F", "-e", "a", "main", "--", "f0.txt"], { cwd: dir, stopAfterLines: 1000 });
50 expect(run.stopped).toBeUndefined();
51 expect(run.code).toBe(0);
52 expect(run.stdout.split("\n").filter(Boolean).length).toBe(LINES);
53 });
54
55 test("a code search for one letter answers its cap of hits from a stopped grep", async () => {
56 const hits = await grepRepo(dir, "main", "a", 20);
57 expect(hits.length).toBe(20);
58 expect(hits[0]).toEqual({ path: expect.stringMatching(/^f\d+\.txt$/), line: expect.any(Number), text: "a line that matches a" });
59 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs