Public: anyone can read this; the crew can change it.
Sentinel: release was degraded 190 minutes today: Release 0.7.509 is unproven: https://forge.open-platform.sh/search?q=a as desktop /search?q=a took 2511 ms, over 1000 m… #10742
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/10730-sentinel-release-was-degraded-190-minute → main 3 commits fork
5 files
changed +74
−16
src/actions/search.ts +12 −11
| @@ -184,17 +184,18 @@ export async function search( | ||
| 184 | 184 | if (wants("issues")) phase("issues", performance.now() - issuesAt); |
| 185 | 185 | |
| 186 | 186 | const codeAt = performance.now(); |
| 187 | const code = wants("code") && input.withCode !== false | |
| 188 | ? await grepRepos( | |
| 189 | app.reposRoot, | |
| 190 | visible, | |
| 191 | q, | |
| 192 | Math.min(limit * 5, MAX_CODE_HITS), | |
| 193 | input.codeBudgetMs ?? Infinity, | |
| 194 | // The slot is keyed on the actor the session resolved, never on input. | |
| 195 | { memberId: actor?.id }, | |
| 196 | ) | |
| 197 | : { hits: [], truncated: false, more: false, skipped: false }; | |
| 187 | const code = | |
| 188 | wants("code") && input.withCode !== false | |
| 189 | ? await grepRepos( | |
| 190 | app.reposRoot, | |
| 191 | visible, | |
| 192 | q, | |
| 193 | Math.min(limit * 5, MAX_CODE_HITS), | |
| 194 | input.codeBudgetMs ?? Infinity, | |
| 195 | // The slot is keyed on the actor the session resolved, never on input. | |
| 196 | { memberId: actor?.id }, | |
| 197 | ) | |
| 198 | : { hits: [], truncated: false, more: false, skipped: false }; | |
| 198 | 199 | if (wants("code")) phase("code", performance.now() - codeAt); |
| 199 | 200 | |
| 200 | 201 | return { |
src/core/search.ts +12 −5
| @@ -444,6 +444,7 @@ async function grepRun( | ||
| 444 | 444 | query: string, |
| 445 | 445 | maxCount: number, |
| 446 | 446 | timeoutMs = GREP_TIMEOUT_MS, |
| 447 | signal?: AbortSignal, | |
| 447 | 448 | ): Promise<RawHit[] | null | "timeout"> { |
| 448 | 449 | const pattern = cleanQuery(query); |
| 449 | 450 | if (pattern === "" || !isSafeRef(ref)) return []; |
| @@ -467,7 +468,7 @@ async function grepRun( | ||
| 467 | 468 | // `--max-count` is per file: `a` in a repository of thousands of files |
| 468 | 469 | // is megabytes of matches. Only the first `maxCount` are kept, so git is |
| 469 | 470 | // stopped once they are read rather than left to write the rest (#10395). |
| 470 | { cwd: dir, timeoutMs, stopAfterLines: maxCount }, | |
| 471 | { cwd: dir, timeoutMs, stopAfterLines: maxCount, ...(signal === undefined ? {} : { signal }) }, | |
| 471 | 472 | ); |
| 472 | 473 | // Exit 1 is "no match" — data, not failure. Anything else (a repo with no |
| 473 | 474 | // commits, a killed process) is simply no results from this repo. |
| @@ -541,13 +542,14 @@ async function grepCached( | ||
| 541 | 542 | query: string, |
| 542 | 543 | who: GrepWho, |
| 543 | 544 | timeoutMs = GREP_TIMEOUT_MS, |
| 545 | signal?: AbortSignal, | |
| 544 | 546 | ): Promise<RawHit[] | "skipped" | "timeout"> { |
| 545 | 547 | const ref = repo.defaultBranch; |
| 546 | 548 | const oid = await branchOid(dir, ref); |
| 547 | 549 | const slotted = async (at: string): Promise<RawHit[] | null | "skipped" | "timeout"> => { |
| 548 | 550 | const got = await (who.slots ?? grepSlots).run(who.memberId, () => { |
| 549 | 551 | grepsRun++; |
| 550 | return (who.grep ?? grepRun)(dir, at, query, MAX_HITS_PER_REPO, timeoutMs); | |
| 552 | return (who.grep ?? grepRun)(dir, at, query, MAX_HITS_PER_REPO, timeoutMs, signal); | |
| 551 | 553 | }); |
| 552 | 554 | return got.ran ? got.value : "skipped"; |
| 553 | 555 | }; |
| @@ -612,9 +614,8 @@ function budgetTimer(ms: number): BudgetTimer { | ||
| 612 | 614 | * a busy forge holding the page; what answered by then is kept, the rest is |
| 613 | 615 | * `more` (#9402). Each later batch races the time |
| 614 | 616 | * left: when the budget runs out first the answer goes back with `more`, and |
| 615 | * the batch's greps are left to finish on their own. They keep the slot | |
| 616 | * `search/grep_slots.ts` gave them until git exits — that is the only cap on | |
| 617 | * abandoned greps, and it is charged to the caller who started them. | |
| 617 | * the batch's greps are killed, so no abandoned git outlives the answer | |
| 618 | * (#10730); the slot `search/grep_slots.ts` gave them is freed as git exits. | |
| 618 | 619 | */ |
| 619 | 620 | export async function grepRepos( |
| 620 | 621 | reposRoot: string, |
| @@ -639,8 +640,12 @@ export async function grepRepos( | ||
| 639 | 640 | // Set when this search answers at its budget: the batch it left behind |
| 640 | 641 | // starts no further pair. Local to this call, never shared between callers. |
| 641 | 642 | let answered = false; |
| 643 | // Fired when this search answers at its budget: the greps it leaves behind | |
| 644 | // are killed (and not cached), so they stop holding git's gate and a slot. | |
| 645 | const abandon = new AbortController(); | |
| 642 | 646 | const more = () => { |
| 643 | 647 | answered = true; |
| 648 | abandon.abort(); | |
| 644 | 649 | return { hits, truncated: repos.length > MAX_SEARCH_REPOS, more: true, skipped }; |
| 645 | 650 | }; |
| 646 | 651 | // Each grep writes its answer into its own place as it settles, so a batch |
| @@ -655,6 +660,7 @@ export async function grepRepos( | ||
| 655 | 660 | query, |
| 656 | 661 | who, |
| 657 | 662 | timeoutMs, |
| 663 | abandon.signal, | |
| 658 | 664 | ).then((got) => { |
| 659 | 665 | found[k + j] = got; |
| 660 | 666 | }), |
| @@ -685,6 +691,7 @@ export async function grepRepos( | ||
| 685 | 691 | timer.clear(); |
| 686 | 692 | if (raced === "cut") { |
| 687 | 693 | answered = true; |
| 694 | abandon.abort(); | |
| 688 | 695 | cut = true; |
| 689 | 696 | } |
| 690 | 697 | } |
src/git/env.ts +9
| @@ -322,6 +322,8 @@ export interface CaptureOptions extends RunOptions { | ||
| 322 | 322 | * answers of a command whose whole output can be megabytes (`git grep a`). |
| 323 | 323 | */ |
| 324 | 324 | stopAfterLines?: number; |
| 325 | /** Kill the process when this fires (reported as `timedOut`); one already fired never starts it. */ | |
| 326 | signal?: AbortSignal; | |
| 325 | 327 | } |
| 326 | 328 | |
| 327 | 329 | /** |
| @@ -375,6 +377,7 @@ export async function runGitCapture( | ||
| 375 | 377 | ): Promise<GitCapture> { |
| 376 | 378 | const start = performance.now(); |
| 377 | 379 | return gitGate.run(async () => { |
| 380 | if (options.signal?.aborted === true) return { code: 1, stdout: "", stderr: "", timedOut: true }; | |
| 378 | 381 | const proc = Bun.spawn(["git", ...args], { |
| 379 | 382 | ...(options.cwd === undefined ? {} : { cwd: options.cwd }), |
| 380 | 383 | env: gitEnv(options.env), |
| @@ -390,6 +393,11 @@ export async function runGitCapture( | ||
| 390 | 393 | }, |
| 391 | 394 | options.timeoutMs ?? 30_000, |
| 392 | 395 | ); |
| 396 | const abort = () => { | |
| 397 | timedOut = true; | |
| 398 | proc.kill("SIGKILL"); | |
| 399 | }; | |
| 400 | options.signal?.addEventListener("abort", abort, { once: true }); | |
| 393 | 401 | try { |
| 394 | 402 | const [out, stderr, code] = await Promise.all([ |
| 395 | 403 | readLines(proc.stdout, options.stopAfterLines, () => proc.kill("SIGKILL")), |
| @@ -405,6 +413,7 @@ export async function runGitCapture( | ||
| 405 | 413 | }; |
| 406 | 414 | } finally { |
| 407 | 415 | clearTimeout(timer); |
| 416 | options.signal?.removeEventListener("abort", abort); | |
| 408 | 417 | noteGit(gitRecord, args, start); |
| 409 | 418 | } |
| 410 | 419 | }); |
test/git-capture-signal.test.ts added +19
| @@ -0,0 +1,19 @@ | ||
| 1 | import { expect, test } from "bun:test"; | |
| 2 | import { runGitCapture } from "../src/git/env.ts"; | |
| 3 | ||
| 4 | test("an aborted signal kills a running git and reports it as timed out (#10730)", async () => { | |
| 5 | const stop = new AbortController(); | |
| 6 | const started = performance.now(); | |
| 7 | const run = runGitCapture(["-c", "alias.nap=!sleep 3 >/dev/null 2>&1 #", "nap"], { timeoutMs: 30_000, signal: stop.signal }); | |
| 8 | setTimeout(() => stop.abort(), 100); | |
| 9 | const out = await run; | |
| 10 | expect(out.timedOut).toBe(true); | |
| 11 | expect(performance.now() - started).toBeLessThan(5_000); | |
| 12 | }); | |
| 13 | ||
| 14 | test("a signal already aborted starts no git", async () => { | |
| 15 | const stop = new AbortController(); | |
| 16 | stop.abort(); | |
| 17 | const out = await runGitCapture(["-c", "alias.nap=!sleep 3 >/dev/null 2>&1 #", "nap"], { signal: stop.signal }); | |
| 18 | expect(out.timedOut).toBe(true); | |
| 19 | }); |
test/search-batch-race.test.ts +22
| @@ -237,3 +237,25 @@ test("a search with no budget never takes the first batch's ceiling", async () = | ||
| 237 | 237 | expect((await search).hits.length).toBe(2); |
| 238 | 238 | expect(first.asked()).toBe(0); |
| 239 | 239 | }); |
| 240 | ||
| 241 | test("a grep left behind at the budget is told to stop (#10730)", async () => { | |
| 242 | const signals: AbortSignal[] = []; | |
| 243 | const first = handTimer(); | |
| 244 | // Hangs until told to stop, as a git that the answer no longer needs. | |
| 245 | const grep = ((_d: string, _a: string, _q: string, _n: number, _t: number, signal?: AbortSignal) => { | |
| 246 | signals.push(signal!); | |
| 247 | return new Promise<Raw>((done) => signal!.addEventListener("abort", () => done(null))); | |
| 248 | }) as never; | |
| 249 | const search = grepRepos("/nowhere", [0, 1, 2, 3, 4, 5].map(repo), "q", 200, 10, { | |
| 250 | slots: createGrepSlots(), | |
| 251 | grep, | |
| 252 | now: () => 0, | |
| 253 | firstTimer: first.timer, | |
| 254 | }); | |
| 255 | while (signals.length < 4) await new Promise((r) => setTimeout(r, 1)); | |
| 256 | expect(signals.some((s) => s.aborted)).toBe(false); | |
| 257 | first.fire(); | |
| 258 | const out = await search; | |
| 259 | expect(out.more).toBe(true); | |
| 260 | expect(signals.every((s) => s.aborted)).toBe(true); | |
| 261 | }); |