mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix(version-gate): a release output regenerated at the base VERSION is not a stamp
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
8d9501373b
commit
01fea1b2d9
5 files changed
+125
-2
No files matched your search
@@ -1,5 +1,15 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
**An unstamped PR can change a release output again (GSTA-96 on #3434).** Regenerating `agents-digest/gstack-AGENTS.md` after a generator change made the Version Gate classify the head as stamped (the digest is in `release_outputs`) and demand base + bump, while changing only `scripts/gen-agents-digest.ts` failed `test/agents-digest.test.ts`'s freshness check, so a digest rewording could not land before its slot.
|
||||
|
||||
### Fixed
|
||||
- **Version Gate under a train policy** (`lib/version-gate-train.ts`, `scripts/compare-pr-version.ts`): a changed `release_outputs` file no longer counts as a stamp when it is byte for byte what the policy's `release_tool` generates at the base's `VERSION`; the gate runs the head's tool in a throwaway worktree with the base's `VERSION` and compares. `VERSION` and `package.json` version changes still classify the head as stamped, as does an output that differs (hand-edited, or generated at a bumped version) or a policy with no `release_tool`; a tool that exits non-zero is an error with its stderr, never a pass. The freshness test stays strict and a stamp commit still changes only the stamp files, since the slot's restamp regenerates the output. `docs/ship-policy.md` states the rule under "Lane: report ready".
|
||||
|
||||
### Tests
|
||||
- `test/compare-pr-version.test.ts`: a generator change with its regenerated output at the base VERSION passes as unstamped (red without the fix); a hand-edited output, an output generated at the bumped version, a VERSION bump without the output, a failing release tool and a policy without `release_tool` all still refuse.
|
||||
|
||||
## [1.91.226.0] - 2026-10-26
|
||||
|
||||
**/review can re-review just its own fixes after a fixing pass, under the same rules as /ship (#3339).** Before, every /review fix cycle reran the whole review over the whole branch, so a large branch could hit the three-cycle stop on findings in code the earlier passes had already read. Now the pass after an edited pass starts with `gstack-review-log --start review --fix-range` and its checklist, specialists and QA read only what changed since that pass. The Step 4.8 adversarial review still reads the whole branch on every pass, because its rows carry their own freshness. A narrowed pass is never the final review: when it makes no edits, /review runs one full pass before writing the final record, and a fix-range record is never CURRENT. Garry's "do it" on 2026-10-24 for a fix-range re-review with a final full pass covers it.
|
||||
|
||||
@@ -168,6 +168,16 @@ gate: command="<gate_command>" full_e2e=no gate-ahead=none
|
||||
SHIP_RECEIPT: pr=42 head=<head12> …
|
||||
```
|
||||
|
||||
CI's Version Gate (`scripts/compare-pr-version.ts`) accepts that head as
|
||||
unstamped when `VERSION` and `package.json`'s version match its merge base, the
|
||||
CHANGELOG change is confined to `[Unreleased]`, and any changed release output
|
||||
is byte for byte what `release_tool` generates at the base's `VERSION`, so a
|
||||
generator change ships with its regenerated output and the slot's restamp
|
||||
regenerates it again. A release output that differs from that (a hand edit, or
|
||||
one generated at a bumped version), a `VERSION` bump or a `package.json`
|
||||
version change classifies the head as stamped, and it must then carry base +
|
||||
bump. A `release_tool` that fails at the head is an error, never a pass.
|
||||
|
||||
It refuses (`QUEUE_NOT_READY`) when the working tree has tracked edits, HEAD
|
||||
is not the PR's pushed head, CI at that exact head is not green, or the pregate
|
||||
result is not a current pass. Send the block as printed.
|
||||
|
||||
@@ -8,6 +8,9 @@
|
||||
* stamped one to the version the restamp allocates (base + the policy bump).
|
||||
*/
|
||||
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 { parseChangelog } from './changelog-check';
|
||||
import { matchesAny } from './ship-policy';
|
||||
|
||||
@@ -30,6 +33,38 @@ export function bumpVersion(version: string, level: string): string | null {
|
||||
return v.map((n, i) => (i < at ? n : i === at ? n + 1 : 0)).join('.');
|
||||
}
|
||||
|
||||
/**
|
||||
* An unstamped head may regenerate a release output (a generator change, for
|
||||
* example) as long as what it committed is exactly what the policy's
|
||||
* release_tool produces at the base's VERSION: the slot's restamp regenerates
|
||||
* it anyway. Runs the head's tool in a throwaway worktree with VERSION set to
|
||||
* the base's and compares each changed output byte for byte. A stamp (new
|
||||
* VERSION inside the output) differs, so it still classifies as stamped.
|
||||
*/
|
||||
function regeneratedAtBase(o: { repoRoot: string; head: string; baseVersion: string; tool: string; outputs: string[] }): { kind: 'same' } | { kind: 'differs'; files: string[] } | { kind: 'error'; reason: string } {
|
||||
if (!o.baseVersion.trim()) return { kind: 'error', reason: 'the base has no VERSION to regenerate the release outputs at' };
|
||||
const staging = fs.mkdtempSync(path.join(os.tmpdir(), 'version-gate-regen-'));
|
||||
try {
|
||||
const add = spawnSync('git', ['worktree', 'add', '--detach', '-q', staging, o.head], { cwd: o.repoRoot, encoding: 'utf8', timeout: 60_000 });
|
||||
if (add.status !== 0) return { kind: 'error', reason: `cannot stage ${o.head.slice(0, 12)} to regenerate its release outputs: ${(add.stderr || '').trim().slice(0, 200)}` };
|
||||
fs.writeFileSync(path.join(staging, 'VERSION'), o.baseVersion);
|
||||
const nm = path.join(o.repoRoot, 'node_modules');
|
||||
if (fs.existsSync(nm) && !fs.existsSync(path.join(staging, 'node_modules'))) { try { fs.symlinkSync(nm, path.join(staging, 'node_modules'), 'dir'); } catch {} }
|
||||
const run = spawnSync('sh', ['-c', o.tool], { cwd: staging, encoding: 'utf8', timeout: 600_000, env: { ...process.env, GSTACK_RESTAMP_VERSION: o.baseVersion.trim() } });
|
||||
if (run.status !== 0) return { kind: 'error', reason: `release_tool \`${o.tool}\` exited ${run.status} at ${o.head.slice(0, 12)} with the base's VERSION, so its release outputs cannot be told from a stamp: ${(run.stderr || run.stdout || '').trim().slice(0, 200)}` };
|
||||
const files = o.outputs.filter(f => {
|
||||
const committed = spawnSync('git', ['show', `${o.head}:${f}`], { cwd: o.repoRoot, timeout: 30_000, maxBuffer: 64 * 1024 * 1024 });
|
||||
let generated: Buffer | null = null;
|
||||
try { generated = fs.readFileSync(path.join(staging, f)); } catch { generated = null; }
|
||||
return committed.status !== 0 || generated === null || !committed.stdout.equals(generated);
|
||||
});
|
||||
return files.length ? { kind: 'differs', files } : { kind: 'same' };
|
||||
} finally {
|
||||
spawnSync('git', ['worktree', 'remove', '--force', staging], { cwd: o.repoRoot, timeout: 60_000 });
|
||||
fs.rmSync(staging, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
export function classifyTrainHead(o: { repoRoot: string; baseRef: string; head: string }): TrainHead {
|
||||
const base = `origin/${o.baseRef.replace(/^origin\//, '')}`;
|
||||
const show = (rev: string, file: string) => git(o.repoRoot, ['show', `${rev}:${file}`]);
|
||||
@@ -42,8 +77,14 @@ export function classifyTrainHead(o: { repoRoot: string; baseRef: string; head:
|
||||
const npmVersion = (rev: string) => { try { return JSON.parse(show(rev, 'package.json') ?? '{}').version ?? null; } catch { return null; } };
|
||||
const changed = (git(o.repoRoot, ['diff', '--name-only', mergeBase, o.head]) ?? '').split('\n').filter(Boolean);
|
||||
const releaseOutputs: string[] = Array.isArray(policy.release_outputs) ? policy.release_outputs : [];
|
||||
const touched = changed.filter(f => f === 'VERSION' || matchesAny(releaseOutputs, f));
|
||||
const touched: string[] = changed.filter(f => f === 'VERSION');
|
||||
if (npmVersion(mergeBase) !== npmVersion(o.head)) touched.push('package.json version');
|
||||
const outputs = changed.filter(f => matchesAny(releaseOutputs, f));
|
||||
if (!touched.length && outputs.length && typeof policy.release_tool === 'string') {
|
||||
const regen = regeneratedAtBase({ repoRoot: o.repoRoot, head: o.head, baseVersion: show(base, 'VERSION') ?? '', tool: policy.release_tool, outputs });
|
||||
if (regen.kind === 'error') return { kind: 'invalid', reason: regen.reason };
|
||||
if (regen.kind === 'differs') touched.push(...regen.files);
|
||||
} else touched.push(...outputs);
|
||||
if (touched.length) {
|
||||
const expected = bumpVersion(show(base, 'VERSION') ?? '', policy.bump ?? 'patch');
|
||||
return expected ? { kind: 'stamped', expected } : { kind: 'invalid', reason: `${base}:VERSION is not a 4-part version` };
|
||||
|
||||
@@ -41,7 +41,7 @@ const prVersionEnv = (process.env.PR_VERSION ?? "").trim();
|
||||
if (process.env.BASE_REF && process.env.HEAD_SHA) {
|
||||
const train = classifyTrainHead({ repoRoot: process.cwd(), baseRef: process.env.BASE_REF, head: process.env.HEAD_SHA });
|
||||
if (train.kind === "unstamped") {
|
||||
console.log(`✓ ${tag} is unstamped under the base's train policy: VERSION, package.json and the release outputs match its merge base, and the CHANGELOG change is confined to [Unreleased]. The lane stamps at its slot.`);
|
||||
console.log(`✓ ${tag} is unstamped under the base's train policy: VERSION and package.json match its merge base, any changed release output is what the release tool generates at the base's VERSION, and the CHANGELOG change is confined to [Unreleased]. The lane stamps at its slot.`);
|
||||
process.exit(0);
|
||||
}
|
||||
if (train.kind === "invalid") {
|
||||
|
||||
@@ -168,6 +168,68 @@ describe('version gate: unstamped heads under a train policy', () => {
|
||||
}
|
||||
});
|
||||
|
||||
describe('a release output regenerated at the base VERSION is not a stamp (GSTA-96: an unstamped PR could not reword the agents digest)', () => {
|
||||
const gen = (r: FixtureRepo, body: string) => write(r, 'gen.js', `const fs=require('fs');const v=fs.readFileSync('VERSION','utf8').trim();fs.writeFileSync('generated/digest.md','# digest v'+v+'\\n${body}\\n');\n`);
|
||||
|
||||
test('a generator change with its regenerated output at the base VERSION passes as unstamped', () => {
|
||||
const r = fixture();
|
||||
branch(r);
|
||||
gen(r, 'reworded body');
|
||||
write(r, 'generated/digest.md', '# digest v1.2.3.0\nreworded body\n');
|
||||
write(r, 'CHANGELOG.md', withUnreleased(r));
|
||||
const res = run(r, commit(r, 'reword the digest'));
|
||||
expect(res.code).toBe(0);
|
||||
expect(res.out).toContain('is unstamped under the base');
|
||||
});
|
||||
|
||||
test('an output that is not what the generator produces at the base VERSION still counts as a stamp', () => {
|
||||
for (const edit of [
|
||||
(r: FixtureRepo) => write(r, 'generated/digest.md', '# digest v1.2.3.0\nhand-edited body\n'),
|
||||
(r: FixtureRepo) => { gen(r, 'reworded body'); write(r, 'generated/digest.md', '# digest v1.2.4.0\nreworded body\n'); },
|
||||
]) {
|
||||
const r = fixture();
|
||||
branch(r);
|
||||
write(r, 'CHANGELOG.md', withUnreleased(r));
|
||||
edit(r);
|
||||
const res = run(r, commit(r, 'not a regeneration'));
|
||||
expect(res.code).toBe(1);
|
||||
expect(res.out).toContain('must carry v1.2.4.0');
|
||||
}
|
||||
});
|
||||
|
||||
test('a VERSION bump without the regenerated output is a stamp, never an unstamped head', () => {
|
||||
const r = fixture();
|
||||
branch(r);
|
||||
write(r, 'VERSION', '1.2.4.0\n');
|
||||
write(r, 'CHANGELOG.md', withUnreleased(r));
|
||||
const res = run(r, commit(r, 'bump without digest'), '1.2.3.0');
|
||||
expect(res.code).toBe(1);
|
||||
expect(res.out).toContain('must carry v1.2.4.0');
|
||||
expect(res.out).not.toContain('is unstamped under the base');
|
||||
});
|
||||
|
||||
test('a release tool that fails at the head is an error, not a pass', () => {
|
||||
const r = fixture();
|
||||
branch(r);
|
||||
write(r, 'gen.js', 'process.stderr.write("gen broke\\n");process.exit(3);\n');
|
||||
write(r, 'generated/digest.md', '# digest v1.2.3.0\nreworded body\n');
|
||||
write(r, 'CHANGELOG.md', withUnreleased(r));
|
||||
const res = run(r, commit(r, 'broken generator'));
|
||||
expect(res.code).toBe(1);
|
||||
expect(res.out).toContain('release_tool `node gen.js` exited 3');
|
||||
expect(res.out).toContain('gen broke');
|
||||
});
|
||||
|
||||
test('without a release_tool a changed output keeps counting as a stamp', () => {
|
||||
const r = fixture({ ...BASE_POLICY, release_tool: null, train: TRAIN });
|
||||
branch(r);
|
||||
write(r, 'generated/digest.md', '# digest v1.2.3.0\nreworded body\n');
|
||||
write(r, 'CHANGELOG.md', withUnreleased(r));
|
||||
const res = run(r, commit(r, 'no tool'));
|
||||
expect(res.code).toBe(1);
|
||||
expect(res.out).toContain('must carry v1.2.4.0');
|
||||
});
|
||||
});
|
||||
test('negative controls: CHANGELOG edits outside [Unreleased] fail an unstamped head', () => {
|
||||
const cases: Array<(r: FixtureRepo) => string> = [
|
||||
r => withUnreleased(r).replace('- older', '- older, rewritten'),
|
||||
|
||||
Reference in new issue
Block a user