mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 07:18:03 +01:00
Merge commit 'f2c98fe5d5f8ab9b0775d389da4c3da730076462' into HEAD
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> # Conflicts: # bin/gstack-gate-diff # lib/gate-diff/git.ts
This commit is contained in:
commit
6789cb4f4c
7 files changed
+152
-24
No files matched your search
@@ -39,7 +39,8 @@ const BIN_DIR = path.dirname(Bun.fileURLToPath(import.meta.url));
|
||||
const USAGE = `usage: gstack-gate-diff [base-ref] [--format table|jsonl] [--commit <sha>] [--help]
|
||||
|
||||
Lists every hunk in a test, CI, runner/lint-config, snapshot or golden path of the
|
||||
candidate (merge-base of <base-ref> and HEAD -> working tree + untracked files),
|
||||
candidate (merge-base of <base-ref> and HEAD -> working tree + untracked files;
|
||||
a branch name resolves to origin/<base-ref>, the local ref only when no remote has it),
|
||||
tags the gate edits it recognises (RH-1 RH-3 RH-4 RH-12 RH-13 RH-14 RH-15 RH-16),
|
||||
and prints the read-level listing after one GATE_SUMMARY: line. --commit <sha>
|
||||
scans that commit's first-parent delta instead of the working tree.
|
||||
@@ -148,7 +149,8 @@ function main(argv: string[]): number {
|
||||
process.stdout.write(`GATE_ERROR=no_base ref=${baseRef} fix=${noBaseFix(baseRef, root)}\n`);
|
||||
return 2;
|
||||
}
|
||||
if (base.local) console.error(`gstack-gate-diff: ${base.local === 'unrelated' ? `local ${baseRef} shares no history with HEAD` : base.local === 'differs' ? `local ${baseRef} is not ${base.ref}` : `no local branch ${baseRef}`}; base is ${base.ref}`);
|
||||
if (base.local === 'fallback') console.error(`gstack-gate-diff: ${base.unrelatedRemote ? `${base.unrelatedRemote} shares no history with HEAD` : `no remote-tracking branch for ${baseRef}`}; base is the local ${baseRef}, which may lag its remote`);
|
||||
else if (base.local) console.error(`gstack-gate-diff: ${base.local === 'unrelated' ? `local ${baseRef} shares no history with HEAD` : base.local === 'differs' ? `local ${baseRef} is not ${base.ref}` : `no local branch ${baseRef}`}; base is ${base.ref}`);
|
||||
const mb = mergeBase(base.sha, head, root);
|
||||
if (!mb) {
|
||||
process.stdout.write(`GATE_ERROR=no_base ref=${baseRef} fix=${noBaseFix(baseRef, root)}\n`);
|
||||
|
||||
@@ -116,8 +116,9 @@ function main(): number {
|
||||
const m = /GATE_SUMMARY: .*?tagged=\{([^}]*)\}/.exec(g.stdout ?? '');
|
||||
return g.status === 0 && m ? m[1]!.split(',').reduce((n, kv) => n + (Number(kv.split(':')[1]) || 0), 0) : null;
|
||||
};
|
||||
const scope = scopeOf([base]);
|
||||
let gateTagged = flag('--gate-tagged') !== undefined ? Number(flag('--gate-tagged')) : gateTagsOf([base]);
|
||||
const diffBase = collected.mergeBase ?? base;
|
||||
const scope = scopeOf([diffBase]);
|
||||
let gateTagged = flag('--gate-tagged') !== undefined ? Number(flag('--gate-tagged')) : gateTagsOf([diffBase]);
|
||||
if (gateTagged !== null && !Number.isFinite(gateTagged)) gateTagged = null;
|
||||
const force = flag('--force') === 'all' ? 'all' as const : list('--force');
|
||||
const tiersSetting = runScriptSync(path.join(binDir, 'gstack-config'), ['get', 'review_effort_tiers'], { encoding: 'utf8', timeout: 15_000 });
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
**A stale local `main` no longer turns a /ship review into a full-depth gate review (GST-883).** `gstack-review-plan` handed the raw base name to `gstack-gate-diff`, which diffed against the local branch. With a local `main` four commits behind `origin/main`, the gate scan read a test edit that was already on `origin/main`, so a plain code diff planned as gate class at full depth. On gstack-evals#123 that cost about 15-20M tokens of readers the change did not need (GEVA-117).
|
||||
|
||||
### Fixed
|
||||
- **`gstack-gate-diff <branch>`** resolves a branch name to its remote-tracking branch (`origin/main` for `main`, origin first), the way `gstack-review-plan`'s own diff and review dispatch already did (#3780). It uses the local branch only when no remote has the branch, and says so on stderr: `no remote-tracking branch for <base>`, or `<remote>/<base> shares no history with HEAD`. A repository with no remote at all uses the local branch without a note. Branches are read by their full ref names, so a tag named `main` or `origin/main` can no longer stand in for the base and hide committed edits from the scan.
|
||||
- **`gstack-review-plan`** passes the merge base its own diff used to `gstack-diff-scope` and `gstack-gate-diff`, so the scope flags, the gate scan and the line counts all read the same commits. Its diff reads `refs/remotes/origin/<base>` and `refs/heads/<base>` by full name too.
|
||||
|
||||
### Tests
|
||||
- `test/review-plan-origin-base.test.ts`: a fixture whose `origin/main` is one commit ahead of the local `main`, with a test edit on `origin/main` and a branch forked from it. `gstack-review-plan --base main` and `gstack-gate-diff main` plan the same class, depth and gate-tagged count as with a fresh `main` (before the fix: gate, full, `RH-1:1` against code, light, none). A tag named `origin/main` or `main` at HEAD leaves the plan and the scanned range unchanged. Also covers the stderr note for a branch no remote has, an unrelated `origin/main`, and the quiet path with no remote.
|
||||
@@ -180,9 +180,12 @@ only history need is calibration, which `--commit <sha>` covers by scanning
|
||||
one commit's first-parent delta without the working tree.) Positional
|
||||
base defaults the way `gstack-diff-scope` does (the remote's `HEAD` target,
|
||||
origin first, then the other remotes → `main` → `master`); an
|
||||
already-qualified ref is used as given, and a branch name with no local
|
||||
branch resolves through its remote-tracking twin (`origin/main` for `main`:
|
||||
the merge-base is the same commit), noted once on stderr. stdout: one
|
||||
already-qualified ref is used as given, and a branch name resolves through
|
||||
its remote-tracking twin (`origin/main` for `main`), so a stale local `main`
|
||||
never adds commits the branch only inherited (GST-883). Only when no remote
|
||||
has the branch does the local ref stand, noted once on stderr (a repository
|
||||
with no remote at all uses it quietly). Branches are read by full ref name,
|
||||
so a tag named `main` or `origin/main` cannot stand in for them. stdout: one
|
||||
`GATE_SUMMARY: listed=N read=N tagged={RH-1:2,RH-15:1} unmatched=N
|
||||
coverage=<patterns> languages_unlisted=<…> candidate=<sha+wtree>` line, then
|
||||
the read-level listing (bounded; the full listing goes to `--out`).
|
||||
|
||||
+35
-16
@@ -85,33 +85,52 @@ export function isShallow(cwd: string): boolean {
|
||||
|
||||
/**
|
||||
* The base as a ref that resolves here and, when `head` is given, shares
|
||||
* history with it. For a bare branch name that is origin's tracking branch
|
||||
* (`refs/remotes/origin/main` for `main`), the base /ship merges against,
|
||||
* else the related local branch, else another remote's tracking branch. A
|
||||
* local `main` that is behind (a linked worktree or a Capy clone never moves
|
||||
* it, #3650), missing or unrelated to HEAD (a fetched-over clone) yields to the
|
||||
* twin, so the fix line's fetch into the tracking ref is a fix and not a loop.
|
||||
* A remote-qualified name (`upstream/main`) is taken as given, and the twin is
|
||||
* read by its full ref, so a fork's branch or a tag cannot stand in for it.
|
||||
* history with it. A bare branch name resolves to origin's tracking branch
|
||||
* (`refs/remotes/origin/main` for `main`), the base /ship merges against and
|
||||
* the one review-collect's collectLive and review-dispatch read: a stale local
|
||||
* `main` behind it would scan upstream commits the branch only inherited
|
||||
* (#3650, GST-883), so a local branch that differs is reported as `differs`.
|
||||
* Without a related origin twin the related local branch wins over any other
|
||||
* remote, so a fork's branch cannot replace it; it is `fallback` when origin
|
||||
* lacks the branch or shares no history with HEAD (named as `unrelatedRemote`),
|
||||
* and plain with no origin. Another remote's tracking branch is used only when
|
||||
* no related local branch exists. A clone that checked out only the feature
|
||||
* branch has no local `main` (`missing`); a local `main` that shares no
|
||||
* history with HEAD is `unrelated`, so the fix line's fetch into the tracking
|
||||
* ref is a fix and not a loop. Branches are read by their full names
|
||||
* (`refs/heads/`, `refs/remotes/`), so a tag named `main` or `origin/main`
|
||||
* cannot stand in for them, and a remote-qualified name (`upstream/main`) is
|
||||
* taken as given.
|
||||
*/
|
||||
export function resolveBase(ref: string, cwd: string, head?: string): { ref: string; sha: string; local?: 'missing' | 'unrelated' | 'differs' } | null {
|
||||
export function resolveBase(ref: string, cwd: string, head?: string): { ref: string; sha: string; local?: 'missing' | 'unrelated' | 'differs' | 'fallback'; unrelatedRemote?: string } | null {
|
||||
const related = (sha: string) => !head || mergeBase(sha, head, cwd) !== null;
|
||||
const direct = resolvesToCommit(ref, cwd);
|
||||
const direct = () => resolvesToCommit(ref, cwd);
|
||||
if (!isBranchName(ref)) { const sha = direct(); return sha ? { ref, sha } : null; }
|
||||
const qualified = resolvesToCommit(`refs/remotes/` + ref, cwd);
|
||||
if (qualified) return { ref, sha: qualified };
|
||||
const local = resolvesToCommit(`refs/heads/` + ref, cwd);
|
||||
const remotes = remoteNames(cwd);
|
||||
if (!isBranchName(ref) || remotes.some(remote => ref.startsWith(`${remote}/`))) return direct ? { ref, sha: direct } : null;
|
||||
const localRelated = !!local && related(local);
|
||||
const twin = (remote: string) => {
|
||||
const sha = resolvesToCommit(`refs/remotes/${remote}/${ref}`, cwd);
|
||||
const sha = resolvesToCommit(`refs/remotes/` + remote + '/' + ref, cwd);
|
||||
if (!sha || !related(sha)) return null;
|
||||
return sha === direct ? { ref: `${remote}/${ref}`, sha } : { ref: `${remote}/${ref}`, sha, local: !direct ? 'missing' as const : related(direct) ? 'differs' as const : 'unrelated' as const };
|
||||
const tracking = remote + '/' + ref;
|
||||
return sha === local ? { ref: tracking, sha } : { ref: tracking, sha, local: !local ? 'missing' as const : localRelated ? 'differs' as const : 'unrelated' as const };
|
||||
};
|
||||
const origin = remotes.includes('origin') ? twin('origin') : null;
|
||||
const hasOrigin = remotes.includes('origin');
|
||||
const origin = hasOrigin ? twin('origin') : null;
|
||||
if (origin) return origin;
|
||||
if (direct && related(direct)) return { ref, sha: direct };
|
||||
const originRef = resolvesToCommit(`refs/remotes/origin/` + ref, cwd);
|
||||
const fallback = hasOrigin ? { local: 'fallback' as const, ...(originRef ? { unrelatedRemote: 'origin/' + ref } : {}) } : {};
|
||||
if (localRelated) return { ref, sha: local!, ...fallback };
|
||||
for (const remote of remotes) {
|
||||
if (remote === 'origin') continue;
|
||||
const other = twin(remote);
|
||||
if (other) return other;
|
||||
}
|
||||
return direct ? { ref, sha: direct } : null;
|
||||
if (local) return { ref, sha: local, ...(remotes.length ? { local: 'fallback' as const, ...(originRef ? { unrelatedRemote: 'origin/' + ref } : {}) } : {}) };
|
||||
const sha = direct();
|
||||
return sha ? { ref, sha } : null;
|
||||
}
|
||||
|
||||
/** A word for a printed shell line: as is when it is plain, single-quoted otherwise (ref names may hold `;`, `$`, `(`, `{1,2}`). */
|
||||
|
||||
@@ -174,7 +174,7 @@ export function collectCommit(root: string, baseSha: string, headSha: string): C
|
||||
/** The working tree against the merge base with origin/<base> (or <base>), untracked files counted per path; the registry is the base ref's commit. */
|
||||
export function collectLive(root: string, base: string): Collected {
|
||||
const errors: DepthError[] = [];
|
||||
const ref = git(root, ['rev-parse', '--verify', '-q', `origin/${base}^{commit}`]).ok ? `origin/${base}` : base;
|
||||
const ref = [`refs/remotes/origin/${base}`, `refs/heads/${base}`].find(r => git(root, ['rev-parse', '--verify', '-q', `${r}^{commit}`]).ok) ?? base;
|
||||
const registry = loadRegistryAt(root, ref);
|
||||
const mb = git(root, ['merge-base', ref, 'HEAD']);
|
||||
if (!mb.ok) {
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
/**
|
||||
* gstack-review-plan and gstack-gate-diff diff against origin/<base> (GST-883): with a local <base> that lags
|
||||
* origin/<base>, review-plan handed the raw name to gstack-gate-diff, which merged against the stale local ref and
|
||||
* scanned upstream commits the branch only inherited, so a code diff planned as gate class at full depth.
|
||||
*/
|
||||
import { afterEach, describe, expect, test } from 'bun:test';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
|
||||
const PLAN = path.join(import.meta.dir, '..', 'bin', 'gstack-review-plan');
|
||||
const GATE_DIFF = path.join(import.meta.dir, '..', 'bin', 'gstack-gate-diff');
|
||||
const FIXTURES = path.join(import.meta.dir, 'fixtures', 'gate-diff');
|
||||
const dirs: string[] = [];
|
||||
afterEach(() => { while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); });
|
||||
|
||||
function fixture() {
|
||||
const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'rp-origin-')));
|
||||
dirs.push(root);
|
||||
const work = path.join(root, 'work');
|
||||
const env = { ...process.env, GSTACK_STATE_ROOT: path.join(root, 'state'), GIT_AUTHOR_NAME: 't', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 't', GIT_COMMITTER_EMAIL: 't@t', GIT_CONFIG_GLOBAL: path.join(root, 'gitconfig') };
|
||||
fs.writeFileSync(path.join(root, 'gitconfig'), '[commit]\n\tgpgsign = false\n');
|
||||
const sh = (cwd: string, c: string) => { const r = spawnSync('bash', ['-c', c], { cwd, env, encoding: 'utf8', timeout: 60_000 }); if (r.status !== 0) throw new Error(`${c}: ${r.stderr}`); return r.stdout.trim(); };
|
||||
sh(root, 'git init -q --bare -b main origin.git && git clone -q origin.git work 2>/dev/null');
|
||||
sh(work, `git apply '${FIXTURES}/rh1-skip.base.patch' && git add -A && git commit -qm base && git push -q origin main`);
|
||||
sh(root, `git clone -q origin.git other && cd other && git apply '${FIXTURES}/rh1-skip.patch' && git commit -qam upstream && git push -q origin main`);
|
||||
sh(work, "git fetch -q origin && git checkout -qb feat origin/main && echo 'export const v = 1;' > src/v.ts && git add -A && git commit -qm feat");
|
||||
return { work, env, sh };
|
||||
}
|
||||
|
||||
const planOf = (work: string, env: NodeJS.ProcessEnv) => {
|
||||
const r = spawnSync(PLAN, ['--base', 'main', '--json'], { cwd: work, env, encoding: 'utf8', timeout: 180_000 });
|
||||
expect(r.status, r.stdout + r.stderr).toBe(0);
|
||||
const p = JSON.parse(r.stdout) as { class: string; depth: string; gate_scan: unknown };
|
||||
return { class: p.class, depth: p.depth, gate_scan: p.gate_scan };
|
||||
};
|
||||
const gateDiff = (work: string, env: NodeJS.ProcessEnv, base: string) => {
|
||||
const r = spawnSync(GATE_DIFF, [base], { cwd: work, env, encoding: 'utf8', timeout: 120_000 });
|
||||
expect(r.status, r.stdout + r.stderr).toBe(0);
|
||||
return { tagged: /tagged=\{([^}]*)\}/.exec(r.stdout)?.[1], candidate: /candidate=(\S+)/.exec(r.stdout)?.[1], stderr: r.stderr };
|
||||
};
|
||||
|
||||
describe('review-plan and gate-diff read the origin merge base (GST-883)', () => {
|
||||
// Value: protects=a stale local <base> never turns inherited upstream commits into gate hunks that force full depth;
|
||||
// fails_when=gstack-gate-diff merges against the local <base> while origin/<base> is ahead, or review-plan hands it a base the plan's own diff did not use;
|
||||
// why_new=GEVA-117 paid ~15-20M tokens of full-depth readers for a gate hunk already on origin/main; seam=bin/gstack-review-plan, bin/gstack-gate-diff;
|
||||
// no_claim=which gate rules fire
|
||||
test('local main behind origin/main plans the same class, depth and gate-tagged count as a fresh main', () => {
|
||||
const { work, env, sh } = fixture();
|
||||
expect(sh(work, 'git merge-base main HEAD')).not.toBe(sh(work, 'git merge-base origin/main HEAD'));
|
||||
const stale = { plan: planOf(work, env), tags: gateDiff(work, env, 'main').tagged };
|
||||
sh(work, 'git branch -f main origin/main');
|
||||
const fresh = { plan: planOf(work, env), tags: gateDiff(work, env, 'main').tagged };
|
||||
expect(fresh).toEqual({ plan: { class: 'code', depth: 'light', gate_scan: expect.anything() }, tags: '' });
|
||||
expect(stale).toEqual(fresh);
|
||||
}, 240_000);
|
||||
|
||||
// Value: protects=a tag cannot stand in for the base branch, so it cannot move the merge base to HEAD and hide committed edits;
|
||||
// fails_when=resolveBase or collectLive read `origin/main` or `main` by short name while a tag of that name exists;
|
||||
// why_new=outside-voice finding on the whole diff (shadow tag reproduced); seam=bin/gstack-review-plan, bin/gstack-gate-diff
|
||||
for (const [tag, branch] of [['origin/main', 'refs/remotes/origin/main'], ['main', 'refs/heads/main']] as const) {
|
||||
test(`a tag named ${tag} at HEAD does not shadow ${branch}`, () => {
|
||||
const { work, env, sh } = fixture();
|
||||
if (branch === 'refs/heads/main') sh(work, 'git remote remove origin');
|
||||
const before = { plan: planOf(work, env), candidate: gateDiff(work, env, 'main').candidate };
|
||||
sh(work, `git tag '${tag}' HEAD`);
|
||||
const after = { plan: planOf(work, env), candidate: gateDiff(work, env, 'main').candidate };
|
||||
expect(after).toEqual(before);
|
||||
expect(after.candidate?.startsWith(sh(work, `git merge-base ${branch} HEAD`).slice(0, 12))).toBe(true);
|
||||
}, 240_000);
|
||||
}
|
||||
|
||||
// Value: protects=without a remote-tracking twin the local base still works and the fallback is named on stderr;
|
||||
// fails_when=the origin preference turns a remote-less branch into no_base, or falls back silently; why_new=the fallback is new; seam=bin/gstack-gate-diff
|
||||
test('no origin/<base>: gate-diff falls back to the local ref and says so', () => {
|
||||
const { work, env, sh } = fixture();
|
||||
sh(work, 'git branch trunk origin/main');
|
||||
const r = gateDiff(work, env, 'trunk');
|
||||
expect(r.tagged).toBe('');
|
||||
expect(r.stderr).toContain('gstack-gate-diff: no remote-tracking branch for trunk; base is the local trunk, which may lag its remote');
|
||||
}, 240_000);
|
||||
|
||||
// Value: protects=the fallback note names why the local ref stands, and a remote-less repository stays quiet;
|
||||
// fails_when=an unrelated origin/<base> is reported as missing, or a repository with no remote prints a lag note; why_new=review finding on the fallback; seam=bin/gstack-gate-diff
|
||||
test('an unrelated origin/<base> is named in the note; no remote at all uses the local base quietly', () => {
|
||||
const { work, env, sh } = fixture();
|
||||
sh(work, 'git branch -f main origin/main && git update-ref refs/remotes/origin/main "$(git commit-tree "HEAD^{tree}" -m orphan)"');
|
||||
expect(gateDiff(work, env, 'main').stderr).toContain('gstack-gate-diff: origin/main shares no history with HEAD; base is the local main, which may lag its remote');
|
||||
sh(work, 'git remote remove origin');
|
||||
const none = gateDiff(work, env, 'main');
|
||||
expect(none.tagged).toBe('');
|
||||
expect(none.stderr).not.toContain('base is');
|
||||
}, 240_000);
|
||||
});
|
||||
Reference in new issue
Block a user