review-collect: keep the consumer scan deadline under the 180 s plan timeout (review fix)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-31 18:40:13 +00:00
1 parent ef818d7b04
commit cf82ee8b4c
4 files changed
+22 -10

No files matched your search

+2 -2
View File
@@ -2,7 +2,7 @@
### Changed
- **The consumer scan searches only files that can match** (`lib/review-collect.ts`). A cheap first pass lists the files that contain a changed file's extension, such as `.json`; the full search then reads only those files, a batch of paths per `git grep` call, with docs files left out. The result is the same as before.
- **One deadline for the whole scan:** 300 seconds by default, set in milliseconds with `GSTACK_CONSUMER_SCAN_TIMEOUT_MS` (docs/review-policy.md#review-depth). Past it the depth still fails closed with `consumer_scan_failed`, and the detail now says whether git timed out or exited with an error, and names the deadline. Other git reads keep their 60-second limit.
- **One deadline for the whole scan:** 120 seconds by default, a minute under the 180 seconds /ship gives the whole planner, set in milliseconds with `GSTACK_CONSUMER_SCAN_TIMEOUT_MS` (docs/review-policy.md#review-depth). Past it the depth still fails closed with `consumer_scan_failed`, and the detail now says whether git timed out or exited with an error, and names the deadline. Other git reads keep their 60-second limit.
### Tests
- `test/review-collect.test.ts`: the narrowed search prints the same lines as one whole-tree `git grep` at the head, the merge base and the working tree; the same holds when every file is its own call; consumed sets on a mixed fixture (a chain through a data manifest, an untracked consumer, docs mentions ignored); a tiny deadline fails closed and says it timed out. The three new behavior tests failed before the fix.
- `test/review-collect.test.ts`: the narrowed search prints the same lines as one whole-tree `git grep` at the head, the merge base and the working tree; the same holds when every file is its own call; consumed sets on a mixed fixture (a chain through a data manifest, an untracked consumer, docs mentions ignored); a tiny deadline fails closed and says it timed out; the default deadline stays a minute under the planner timeout in `gstack-review-dispatch` and `gstack-ship-preflight`. The new behavior tests failed before the fix.
+3 -2
View File
@@ -180,8 +180,9 @@ consumer each keep the path code, and YAML, TOML and SVG are always code. More t
2,000 data paths keep every one code; a `git grep` failure is `consumer_scan_failed`
and fails the depth closed. The scan first lists the files that contain a candidate's
extension and then searches only those, docs files left out, so a large data tree
costs one cheap pass. The whole scan gets one 300-second deadline, set in milliseconds
with `GSTACK_CONSUMER_SCAN_TIMEOUT_MS`; a scan that runs past it is also
costs one cheap pass. The whole scan gets one 120-second deadline, a minute under the
180 seconds /ship gives `gstack-review-plan`, set in milliseconds with
`GSTACK_CONSUMER_SCAN_TIMEOUT_MS`; a scan that runs past it is also
`consumer_scan_failed`, and its detail says it timed out and names the deadline. Only `classifyDiff` applies the rule (`lib/review-collect.ts`
finds the consumers, `gstack-ship-preflight`'s `RUNTIME_LINES` reads the same
function); every path-only caller (the receipt's gate-scan decision, the Step 16
+6 -3
View File
@@ -104,9 +104,12 @@ function readDiff(root: string, range: string[], errors: DepthError[]): Pick<Col
export const CONSUMER_SCAN_CAP = 2000;
/** How many data files deep a chain of data naming data is followed (a manifest naming a data file) before the rest count as consumed. */
export const CONSUMER_SCAN_DEPTH = 4;
/** One consumer scan's whole budget, every git grep in it (GSTACK_CONSUMER_SCAN_TIMEOUT_MS overrides it); past it the scan fails closed. */
export const CONSUMER_SCAN_TIMEOUT_MS = 300_000;
/** Pathspec bytes per git grep call, so a long list of files to search stays under Windows' 32 KB command line. */
/**
* One consumer scan's whole budget, every git grep in it (GSTACK_CONSUMER_SCAN_TIMEOUT_MS overrides it); past it the scan fails
* closed. It stays a minute under the 180 s gstack-review-dispatch and gstack-ship-preflight give all of gstack-review-plan.
*/
export const CONSUMER_SCAN_TIMEOUT_MS = 120_000;
/** Pathspec bytes per git grep call: a long list of files to search splits across calls instead of growing one command line. */
export const CONSUMER_SCAN_PATHSPEC_BYTES = 16_000;
export function consumerScanTimeoutMs(): number {
+11 -3
View File
@@ -3,7 +3,7 @@ import { spawnSync } from 'node:child_process';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { collectCommit, collectLive, CONSUMER_SCAN_CAP, grepConsumers, loadRegistryAt, scanConsumers } from '../lib/review-collect';
import { collectCommit, collectLive, CONSUMER_SCAN_CAP, CONSUMER_SCAN_TIMEOUT_MS, grepConsumers, loadRegistryAt, scanConsumers } from '../lib/review-collect';
import { depthOfCommitInput } from '../lib/review-depth';
const dirs: string[] = [];
@@ -236,7 +236,15 @@ describe('GST-544: the consumer scan reads only files that can match, under one
}
r.git('config', 'grep.lineNumber', 'not-a-bool');
const failed: Parameters<typeof scanConsumers>[5] = [];
expect(scanConsumers(r.dir, ['data/rows.json'], [r.head], false, r.dir, failed)).toBe('all');
expect(failed[0]!.detail).toMatch(/^git grep exited \d+: .*\(deadline 300 s\)$/);
expect(scanConsumers(r.dir, ['data/rows.json'], [r.head], false, r.dir, failed, CONSUMER_SCAN_TIMEOUT_MS)).toBe('all');
expect(failed[0]!.detail).toMatch(new RegExp(`^git grep exited \\d+: .*\\(deadline ${CONSUMER_SCAN_TIMEOUT_MS / 1000} s\\)$`));
});
// Value: protects=a slow scan ends in its own fail-closed consumer_scan_failed, not in the parent killing gstack-review-plan;
// fails_when=the scan deadline reaches the timeout the dispatch and preflight callers give the whole plan; seam=caller source
test('the default scan deadline leaves a minute of the plan timeout its callers set', () => {
const callers = ['bin/gstack-review-dispatch', 'lib/ship-preflight.ts'].map(f => fs.readFileSync(path.join(import.meta.dir, '..', f), 'utf8'));
const limits = callers.flatMap(src => [...src.matchAll(/gstack-review-plan'[^\n]*?(?:timeout: |\], )(\d[\d_]+)/g)].map(m => Number(m[1]!.replace(/_/g, ''))));
expect(limits.length).toBeGreaterThanOrEqual(2);
expect(CONSUMER_SCAN_TIMEOUT_MS).toBeLessThanOrEqual(Math.min(...limits) - 60_000);
});
});