mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 23:14:33 +01:00
fix(ship-receipt): carry across a base merge whose path listing passes 1 MiB (GST-552)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
3bfe5667fb
commit
82057955d8
5 files changed
+100
-15
No files matched your search
+22
-10
@@ -45,7 +45,7 @@ import {
|
||||
receiptIssuesFrom, receiptSummaryLine, renderCensus, renderReceiptBlock, validateReceipt, type CensusPr,
|
||||
} from '../lib/ship-receipt';
|
||||
import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs';
|
||||
import { deeper, depthBelow, exceptionProblems, parseReviewLoopException, parseReviewTargeted, receiptDepthAtHead, reviewLoopExceptionLine } from '../lib/ship-receipt-depth';
|
||||
import { deeper, depthBelow, exceptionProblems, GitRunError, parseReviewLoopException, parseReviewTargeted, receiptDepthAtHead, receiptGit, reviewLoopExceptionLine } from '../lib/ship-receipt-depth';
|
||||
import { checkReceiptProse, proseLine } from '../lib/ship-receipt-prose';
|
||||
import { resolveStateRoot } from '../lib/state-root';
|
||||
import { readPause, receiptBaseRed } from '../lib/base-red';
|
||||
@@ -118,8 +118,8 @@ function jsonArg(raw: string | undefined): Record<string, unknown> | undefined {
|
||||
return parsed;
|
||||
}
|
||||
function git(args: string[], cwd?: string): string | undefined {
|
||||
const r = spawnSync('git', args, { cwd, encoding: 'utf8', timeout: 30_000 });
|
||||
return r.status === 0 ? r.stdout.trim() : undefined;
|
||||
const r = receiptGit(cwd, args, 30_000);
|
||||
return r.ok ? r.out.trim() : undefined;
|
||||
}
|
||||
function gh(args: string[]): { ok: boolean; out: string; err: string } {
|
||||
const r = spawnSync('gh', args, { encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 });
|
||||
@@ -210,7 +210,7 @@ function cmdWrite(rest: string[], json: boolean): number {
|
||||
let loop: unknown; try { loop = jsonArg(flag(rest, '--review-loop')); } catch {}
|
||||
if (!parseReviewLoop(loop)?.fix_checks) return refuse('--review-loop-exception needs --review-loop with fix_checks >= 1: the stop it accepts follows a fix-check');
|
||||
const fc = loopException.exception.fix_check.head;
|
||||
if (spawnSync('git', ['merge-base', '--is-ancestor', fc, head], { cwd: repoRoot }).status !== 0) return refuse(`fix_check.head ${fc.slice(0, 12)} is not ${head.slice(0, 12)} or an ancestor of it`);
|
||||
if (!receiptGit(repoRoot, ['merge-base', '--is-ancestor', fc, head], 30_000).ok) return refuse(`fix_check.head ${fc.slice(0, 12)} is not ${head.slice(0, 12)} or an ancestor of it`);
|
||||
const line = reviewLoopExceptionLine(loopException.exception);
|
||||
if (bodyFile && !bodyText.split(/\r?\n/).some(l => l.trim() === line)) return refuse(`${bodyFile} does not carry the exception line verbatim: ${JSON.stringify(line)}`);
|
||||
}
|
||||
@@ -399,10 +399,10 @@ function writeRunRecord(a: { receipt: ShipReceipt; head: string; pr: string | un
|
||||
function patchId(repoRoot: string, from: string, to: string, pathspec: string[]): string | { error: string } {
|
||||
const firstLine = (s: string | null | undefined, fallback: string) => (s ?? '').trim().split('\n')[0]?.slice(0, 200) || fallback;
|
||||
const diff = spawnSync('git', ['diff', '--no-color', '--no-ext-diff', '--full-index', from, to, '--', ...pathspec], { cwd: repoRoot, encoding: 'utf8', timeout: 120_000, maxBuffer: 256 * 1024 * 1024 });
|
||||
if (diff.status !== 0) return { error: `git diff ${from.slice(0, 12)} ${to.slice(0, 12)}: ${firstLine(diff.stderr, diff.signal ? `killed by ${diff.signal}` : `exit ${diff.status}`)}` };
|
||||
if (diff.status !== 0) return { error: `git diff ${from.slice(0, 12)} ${to.slice(0, 12)}: ${firstLine(diff.error?.message ?? diff.stderr, diff.signal ? `killed by ${diff.signal}` : `exit ${diff.status}`)}` };
|
||||
if (!diff.stdout.trim()) return '';
|
||||
const id = spawnSync('git', ['patch-id', '--verbatim'], { cwd: repoRoot, input: diff.stdout, encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 });
|
||||
if (id.status !== 0) return { error: `git patch-id --verbatim: ${firstLine(id.stderr, id.signal ? `killed by ${id.signal}` : `exit ${id.status}`)}` };
|
||||
if (id.status !== 0) return { error: `git patch-id --verbatim: ${firstLine(id.error?.message ?? id.stderr, id.signal ? `killed by ${id.signal}` : `exit ${id.status}`)}` };
|
||||
return id.stdout.trim().split(/\s+/)[0] ?? '';
|
||||
}
|
||||
|
||||
@@ -463,7 +463,7 @@ function productRules(repoRoot: string, base: string, outputs: readonly string[]
|
||||
function productSpec(repoRoot: string, rules: ProductRules, from: string, to: string): { changed: Array<{ path: string; class: ReturnType<typeof classifyCarryPath> }>; product: string[]; tests: string[] } | undefined {
|
||||
const changedRaw = git(['diff', '--name-only', from, to], repoRoot);
|
||||
if (changedRaw === undefined) return undefined;
|
||||
const blobAt = (ref: string, p: string): string | null => { const r = spawnSync('git', ['show', `${ref}:${p}`], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 16 * 1024 * 1024 }); return r.status === 0 ? r.stdout : null; };
|
||||
const blobAt = (ref: string, p: string): string | null => { const r = receiptGit(repoRoot, ['show', `${ref}:${p}`], 30_000); return r.ok ? r.out : null; };
|
||||
const versionAt = (ref: string): string | null => {
|
||||
const file = blobAt(ref, 'VERSION');
|
||||
if (file !== null) return file.trim() || null;
|
||||
@@ -554,7 +554,7 @@ function cmdCarry(rest: string[], json: boolean): number {
|
||||
const dirty = status.stdout.split('\n').filter(Boolean).map(l => l.slice(3).replace(/^.* -> /, ''));
|
||||
if (dirty.length) return refused('RECEIPT_CARRY_REFUSED', `the working tree has uncommitted changes (${dirty.slice(0, 5).join(', ')}${dirty.length > 5 ? ` +${dirty.length - 5} more` : ''}); commit them and carry at the new head, or run /ship`);
|
||||
const fromKnown = git(['cat-file', '-e', `${from}^{commit}`], repoRoot) !== undefined;
|
||||
const ancestor = fromKnown && spawnSync('git', ['merge-base', '--is-ancestor', from, head], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000 }).status === 0;
|
||||
const ancestor = fromKnown && receiptGit(repoRoot, ['merge-base', '--is-ancestor', from, head], 30_000).ok;
|
||||
const fromTree = fromKnown ? git(['rev-parse', `${from}^{tree}`], repoRoot) : undefined;
|
||||
const covered = [...(receipt.head_tree ? [receipt.head_tree] : fromTree ? [fromTree] : []), ...(receipt.carry?.trees ?? [])];
|
||||
if (!ancestor && !covered.length) return refused('RECEIPT_CARRY_REFUSED', `the receipt's head ${from.slice(0, 12)} is not an ancestor of HEAD ${head.slice(0, 12)} (rebased or force-pushed), and the receipt records no tree to compare; merge the base in next time, and run /ship now`);
|
||||
@@ -636,7 +636,7 @@ function cmdCarry(rest: string[], json: boolean): number {
|
||||
return refused('RECEIPT_CARRY_PREGATE_STALE', `the receipt records a pregate for ${receipt.head.slice(0, 12)}; run gstack-pregate at HEAD ${head.slice(0, 12)} and pass --pregate <pregate.json>`);
|
||||
}
|
||||
let depthField: Partial<ShipReceipt> = {};
|
||||
const isAncestor = (commit: string) => !ancestor || spawnSync('git', ['merge-base', '--is-ancestor', commit, head], { cwd: repoRoot, timeout: 30_000 }).status === 0;
|
||||
const isAncestor = (commit: string) => !ancestor || receiptGit(repoRoot, ['merge-base', '--is-ancestor', commit, head], 30_000).ok;
|
||||
const problems = exceptionProblems(body, receipt, isAncestor);
|
||||
if (problems.length) return refused('RECEIPT_CARRY_REFUSED', `the receipt's owner exception does not stand: ${problems.join('; ')}; run /ship on ${head.slice(0, 12)}`);
|
||||
if (receipt.review_depth || receipt.gstack_version) {
|
||||
@@ -750,4 +750,16 @@ function main(argv: string[]): number {
|
||||
return EXIT.usage;
|
||||
}
|
||||
|
||||
process.exitCode = main(process.argv.slice(2));
|
||||
/** A git answer the receipt could not read (output past the buffer, a timeout) refuses by name instead of reading as "no" (GST-552). */
|
||||
function mainRefusingGitErrors(argv: string[]): number {
|
||||
try { return main(argv); } catch (e) {
|
||||
if (!(e instanceof GitRunError)) throw e;
|
||||
const [cmd, ...rest] = argv;
|
||||
const code = cmd === 'carry' ? 'RECEIPT_CARRY_REFUSED' : 'RECEIPT_INVALID';
|
||||
process.stderr.write(resultLineWithDocs(`gstack-ship-receipt ${cmd}`, code, e.message) + '\n');
|
||||
if (cmd === 'carry') process.stdout.write(rest.includes('--json') ? JSON.stringify({ skill: 'ship', status: 'refused', run: '-', code, message: e.message }) + '\n' : `CARRY: refused (${code})\nGSTACK_RESULT: skill=ship status=refused run=-\n`);
|
||||
return EXIT.refused;
|
||||
}
|
||||
}
|
||||
|
||||
process.exitCode = mainRefusingGitErrors(process.argv.slice(2));
|
||||
@@ -0,0 +1,5 @@
|
||||
**`gstack-ship-receipt carry` works again across a base merge that adds thousands of files.** After harness-evals#147 merged a base that added about 8,300 files, every carry across that merge was refused with "git diff … failed". The carry lists the paths that changed between the reviewed head and HEAD, and that list was 1.37 MB. The git calls ran with the default 1 MiB output limit, so git was stopped mid-listing (ENOBUFS) and the carry read it as a failed diff.
|
||||
|
||||
### Fixed
|
||||
- **`bin/gstack-ship-receipt`, `lib/ship-receipt-depth.ts`**: the receipt's git calls go through one helper, `receiptGit`, with a 256 MiB output limit (the same one `lib/review-collect.ts` uses). When git's output still passes that limit or the call times out, the command now refuses by name (`RECEIPT_CARRY_REFUSED` for carry) and says the buffer was exceeded, instead of treating the missing answer as "no". The `--is-ancestor` check in `write --review-loop-exception` now has a 30-second timeout.
|
||||
- **`lib/tree-receipt.ts`**: the gated-to-current diff runs with a 256 MiB limit, and a failed diff is an `EVIDENCE_IDENTITY_UNKNOWN` error instead of an empty diff that classified no changed files.
|
||||
@@ -35,9 +35,23 @@ const sha = (v: unknown): v is string => typeof v === 'string' && /^[0-9a-f]{40}
|
||||
const BIN_DIR = path.join(import.meta.dir, '..', 'bin');
|
||||
const short = (s: string | null | undefined) => (s ? s.slice(0, 12) : 'none');
|
||||
|
||||
/** The receipt's git output cap: a base merge can list thousands of paths (harness-evals#147: 1.37 MB of names), past spawnSync's 1 MiB default. */
|
||||
export const GIT_MAX_BUFFER = 256 * 1024 * 1024;
|
||||
/** A git run whose answer could not be read (output past the buffer, a timeout): a named refusal, never a quiet "no" (GST-552). */
|
||||
export class GitRunError extends Error {}
|
||||
/** git for the receipt: `ok` is git's own verdict; a spawn error (ENOBUFS, a timeout) throws GitRunError. */
|
||||
export function receiptGit(cwd: string | undefined, args: string[], timeout = 60_000, maxBuffer = GIT_MAX_BUFFER): { ok: boolean; out: string } {
|
||||
const r = spawnSync('git', args, { cwd, encoding: 'utf8', timeout, maxBuffer });
|
||||
if (r.error || (r.status === null && r.signal)) {
|
||||
const code = (r.error as NodeJS.ErrnoException | undefined)?.code;
|
||||
const why = code === 'ENOBUFS' ? `its output exceeded the ${maxBuffer / (1024 * 1024)} MiB buffer` : code === 'ETIMEDOUT' || r.signal ? `it was stopped after ${timeout / 1000} s (${r.error?.message ?? r.signal})` : r.error!.message;
|
||||
throw new GitRunError(`git ${args.slice(0, 2).join(' ')} could not be read: ${why}`);
|
||||
}
|
||||
return { ok: r.status === 0, out: r.stdout ?? '' };
|
||||
}
|
||||
function git(root: string, args: string[]): { ok: boolean; out: string } {
|
||||
const r = spawnSync('git', args, { cwd: root, encoding: 'utf8', timeout: 60_000 });
|
||||
return { ok: r.status === 0, out: (r.stdout ?? '').trim() };
|
||||
const r = receiptGit(root, args);
|
||||
return { ok: r.ok, out: r.out.trim() };
|
||||
}
|
||||
|
||||
/** gstack-diff-scope on the committed range `base`...`head` (`--head`): the same signals at write, carry and the queue. */
|
||||
|
||||
+5
-3
@@ -42,7 +42,7 @@ export interface TreeReceipt {
|
||||
}
|
||||
|
||||
function git(cwd: string, args: string[]): string | null {
|
||||
const r = spawnSync('git', args, { cwd, encoding: 'utf8', timeout: 60_000, maxBuffer: 64 * 1024 * 1024 });
|
||||
const r = spawnSync('git', args, { cwd, encoding: 'utf8', timeout: 60_000, maxBuffer: 256 * 1024 * 1024 });
|
||||
return r.status === 0 ? (r.stdout ?? '').replace(/\n$/, '') : null;
|
||||
}
|
||||
function blob(cwd: string, treeish: string, rel: string): string | null {
|
||||
@@ -170,11 +170,13 @@ export function buildReceipt(o: ReceiptOptions): TreeReceipt | { error: string;
|
||||
const versionRel = o.versionRel ?? ('rel' in cv ? cv.rel : 'rel' in gv ? gv.rel : 'VERSION');
|
||||
const oldV = 'version' in gv ? gv.version : null;
|
||||
const newV = 'version' in cv ? cv.version : null;
|
||||
const diffText = git(o.repoRoot, ['diff', '--unified=0', '--no-color', '--no-ext-diff', gatedHead, currentHead]) ?? '';
|
||||
const diffText = git(o.repoRoot, ['diff', '--unified=0', '--no-color', '--no-ext-diff', gatedHead, currentHead]);
|
||||
if (diffText === null) return { error: `git diff ${gatedHead.slice(0, 12)} ${currentHead.slice(0, 12)} failed (past the 256 MiB buffer, a timeout, or a missing object), so the change between the gated and current trees cannot be classified`, code: 'EVIDENCE_IDENTITY_UNKNOWN' };
|
||||
const files = classifyFiles(o.repoRoot, parseUnifiedDiff(diffText), gatedHead, currentHead, o.policy, versionRel, oldV, newV);
|
||||
const residual = files.filter(f => f.verdict === 'changed').map(f => f.path);
|
||||
const stampFiles = files.filter(f => f.verdict === 'stamp').map(f => f.path);
|
||||
const stampDiff = stampFiles.length ? (git(o.repoRoot, ['diff', '--no-color', '--no-ext-diff', gatedHead, currentHead, '--', ...stampFiles]) ?? '') : '';
|
||||
const stampDiff = stampFiles.length ? git(o.repoRoot, ['diff', '--no-color', '--no-ext-diff', gatedHead, currentHead, '--', ...stampFiles]) : '';
|
||||
if (stampDiff === null) return { error: `git diff of the stamp files ${gatedHead.slice(0, 12)} ${currentHead.slice(0, 12)} failed, so the stamp cannot be shown`, code: 'EVIDENCE_IDENTITY_UNKNOWN' };
|
||||
const predecessor = resolvePredecessor(o);
|
||||
|
||||
const current = executionIdentity(o.repoRoot, currentHead, o.env);
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
/**
|
||||
* gstack-ship-receipt carry across a base merge whose path listing passes 1 MiB (GST-552, harness-evals#147:
|
||||
* about 8,300 added files, a 1.37 MB `git diff --name-only`). spawnSync's default 1 MiB maxBuffer made that
|
||||
* listing fail with ENOBUFS, so every carry across the merge was refused as "git diff ... failed".
|
||||
*/
|
||||
import { afterEach, describe, expect, test } from 'bun:test';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { EXIT } from '../lib/headless-artifacts';
|
||||
import { extractReceipt } from '../lib/ship-receipt';
|
||||
import { GitRunError, receiptGit } from '../lib/ship-receipt-depth';
|
||||
import { cleanup, commit, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/pregate-fixture';
|
||||
|
||||
const repos: FixtureRepo[] = [];
|
||||
afterEach(() => { while (repos.length) cleanup(repos.pop()!); });
|
||||
|
||||
/** 4,600 empty files under long paths: about 1.15 MB of `git diff --name-only`, one shared blob. */
|
||||
function addManyFiles(work: string): void {
|
||||
const dir = path.join(work, 'vendor', 'x'.repeat(200));
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
for (let i = 0; i < 4600; i++) fs.writeFileSync(path.join(dir, `${'f'.repeat(30)}-${i}.txt`), '');
|
||||
}
|
||||
|
||||
describe('carry across a base merge with a listing over 1 MiB (GST-552)', () => {
|
||||
// Value: protects=a receipt carries across a huge base merge (harness-evals#147 refused every carry with ENOBUFS);
|
||||
// fails_when=the carry's git listings run with the 1 MiB default buffer; seam=none; no_claim=gh pr edit (--body stands in)
|
||||
test('the product-unchanged branch carries, and a listing past the buffer is a named error, never a false', () => {
|
||||
const repo = makeFixtureRepo(); repos.push(repo);
|
||||
const reviewed = commit(repo, { 'lib/util.js': "exports.add = (a, b) => a + b + 0;\n" });
|
||||
const body = path.join(repo.root, 'body.md');
|
||||
const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '7', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]);
|
||||
expect(w.status).toBe(EXIT.ok);
|
||||
repo.git('checkout', '-q', 'main');
|
||||
addManyFiles(repo.work);
|
||||
repo.git('add', '-A'); repo.git('commit', '-qm', 'main: vendor drop'); repo.git('push', '-q', 'origin', 'main');
|
||||
repo.git('checkout', '-q', 'feature');
|
||||
repo.git('merge', '-q', '--no-edit', 'origin/main');
|
||||
const head = repo.git('rev-parse', 'HEAD');
|
||||
const listing = spawnSync('git', ['diff', '--name-only', reviewed, head], { cwd: repo.work, env: { ...process.env, ...repo.env }, encoding: 'utf8', timeout: 30_000, maxBuffer: 64 * 1024 * 1024 });
|
||||
expect(listing.stdout.length).toBeGreaterThan(1024 * 1024);
|
||||
expect(() => receiptGit(repo.work, ['diff', '--name-only', reviewed, head], 30_000, 1024 * 1024)).toThrow(GitRunError);
|
||||
expect(() => receiptGit(repo.work, ['diff', '--name-only', reviewed, head], 30_000, 1024 * 1024)).toThrow('git diff --name-only could not be read: its output exceeded the 1 MiB buffer');
|
||||
expect(receiptGit(repo.work, ['merge-base', '--is-ancestor', head, reviewed]).ok).toBe(false);
|
||||
|
||||
const out = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']);
|
||||
expect(out.err).not.toContain('failed');
|
||||
expect(out.status).toBe(EXIT.ok);
|
||||
expect(out.out).toContain(`CARRY: carried from ${reviewed.slice(0, 12)} to ${head.slice(0, 12)}`);
|
||||
expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.head).toBe(head);
|
||||
}, 90_000);
|
||||
});
|
||||
Reference in new issue
Block a user