mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix(version-gate): run the head release tool with a bare env, drop outputs it must rewrite, prune the staging worktree
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
01fea1b2d9
commit
2b5445f623
5 files changed
+50
-12
No files matched your search
@@ -26,6 +26,9 @@ jobs:
|
||||
with:
|
||||
fetch-depth: 0
|
||||
ref: ${{ github.event.pull_request.head.sha }}
|
||||
# The head's release_tool runs inside the gate (lib/version-gate-train.ts);
|
||||
# no token may sit in the checkout it stages. The base fetch below is public.
|
||||
persist-credentials: false
|
||||
|
||||
# The gate's own code comes from the PR's merge commit (github.sha): a PR
|
||||
# that edits the gate runs its edit, and an open PR picks up a gate fix
|
||||
|
||||
+2
-2
@@ -5,10 +5,10 @@
|
||||
**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".
|
||||
- **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` (the changed outputs removed first, a bare environment with `GSTACK_RESTAMP_VERSION` and `GSTACK_RESTAMP_STAGING=1` and none of the gate's tokens, no `node_modules` link, a 120 s limit) and compares byte for byte. `VERSION` and `package.json` version changes still classify the head as stamped, as does an output that differs (hand-edited, generated at a bumped version, or never written by the tool) or a policy with no `release_tool`; a tool that exits non-zero, is killed or cannot be staged is an error with the reason, never a pass; the staging worktree is removed and pruned on every path. `version-gate.yml` checks the head out with `persist-credentials: false`, since the head's tool now runs inside the gate. 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.
|
||||
- `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, an output the tool never writes, a VERSION bump without the output, a failing release tool and a policy without `release_tool` all still refuse; the tool sees no `GH_TOKEN`; no worktree is left behind.
|
||||
|
||||
## [1.91.226.0] - 2026-10-26
|
||||
|
||||
|
||||
+6
-4
@@ -173,10 +173,12 @@ 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.
|
||||
regenerates it again. A release output that differs from that (a hand edit, one generated at a
|
||||
bumped version, or one the tool never writes), a `VERSION` bump or a
|
||||
`package.json` version change classifies the head as stamped, and it must then
|
||||
carry base + bump. The tool runs as head code in a throwaway worktree with a
|
||||
bare environment and no tokens; one 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
|
||||
|
||||
@@ -39,19 +39,29 @@ export function bumpVersion(version: string, level: string): string | null {
|
||||
* 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.
|
||||
* VERSION inside the output) differs, so it still classifies as stamped, and
|
||||
* so does an output the tool never writes (it is removed before the run).
|
||||
* The tool is head code: it runs with a bare environment (no tokens), no
|
||||
* node_modules link, and a short timeout, and its outputs are read only from
|
||||
* the staging worktree.
|
||||
*/
|
||||
const TOOL_ENV_KEYS = ['PATH', 'HOME', 'TMPDIR', 'LANG', 'LC_ALL', 'BUN_INSTALL', 'SystemRoot', 'TEMP', 'TMP'];
|
||||
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-'));
|
||||
let staging: string;
|
||||
try { staging = fs.mkdtempSync(path.join(os.tmpdir(), 'version-gate-regen-')); } catch (e) { return { kind: 'error', reason: `cannot create a staging directory to regenerate the release outputs: ${e instanceof Error ? e.message : String(e)}` }; }
|
||||
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)}` };
|
||||
for (const f of o.outputs) fs.rmSync(path.join(staging, f), { force: true });
|
||||
const env: NodeJS.ProcessEnv = { GSTACK_RESTAMP_VERSION: o.baseVersion.trim(), GSTACK_RESTAMP_STAGING: '1' };
|
||||
for (const key of TOOL_ENV_KEYS) if (process.env[key] !== undefined) env[key] = process.env[key];
|
||||
const run = spawnSync('sh', ['-c', o.tool], { cwd: staging, encoding: 'utf8', timeout: 120_000, env });
|
||||
if (run.status !== 0) {
|
||||
const how = run.status === null ? (run.error ? run.error.message : `was killed by ${run.signal ?? 'a signal'} (120s limit)`) : `exited ${run.status}`;
|
||||
return { kind: 'error', reason: `release_tool \`${o.tool}\` ${how} 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;
|
||||
@@ -62,6 +72,7 @@ function regeneratedAtBase(o: { repoRoot: string; head: string; baseVersion: str
|
||||
} finally {
|
||||
spawnSync('git', ['worktree', 'remove', '--force', staging], { cwd: o.repoRoot, timeout: 60_000 });
|
||||
fs.rmSync(staging, { recursive: true, force: true });
|
||||
spawnSync('git', ['worktree', 'prune'], { cwd: o.repoRoot, timeout: 60_000 });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -77,6 +88,7 @@ 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 : [];
|
||||
// touched: what marks the head as stamped (the labels only feed the length check below)
|
||||
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));
|
||||
|
||||
@@ -180,6 +180,27 @@ describe('version gate: unstamped heads under a train policy', () => {
|
||||
const res = run(r, commit(r, 'reword the digest'));
|
||||
expect(res.code).toBe(0);
|
||||
expect(res.out).toContain('is unstamped under the base');
|
||||
expect(r.git('worktree', 'list').trim().split('\n')).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('the tool runs without the gate\'s tokens, and an output the tool never writes still counts as a stamp', () => {
|
||||
const r = fixture();
|
||||
branch(r);
|
||||
write(r, 'gen.js', "const fs=require('fs');const v=fs.readFileSync('VERSION','utf8').trim();fs.writeFileSync('generated/digest.md','# digest v'+v+'\\nreworded body\\n'+(process.env.GH_TOKEN??''));\n");
|
||||
write(r, 'generated/digest.md', '# digest v1.2.3.0\nreworded body\n');
|
||||
write(r, 'CHANGELOG.md', withUnreleased(r));
|
||||
const head = commit(r, 'tool that would echo a token');
|
||||
const next = path.join(r.root, 'next.json');
|
||||
fs.writeFileSync(next, JSON.stringify({ version: '1.2.4.0', base_version: '1.2.3.0', claimed: [] }));
|
||||
const leak = Bun.spawnSync([process.execPath, 'run', SCRIPT, next, '3320'], {
|
||||
cwd: r.work, env: { ...process.env, ...r.env, PR_VERSION: '1.2.3.0', BASE_REF: 'main', HEAD_SHA: head, GH_TOKEN: 'ghs_secret' }, timeout: 30_000, stdout: 'pipe', stderr: 'pipe',
|
||||
});
|
||||
expect(leak.exitCode).toBe(0);
|
||||
expect(leak.stdout.toString()).toContain('is unstamped under the base');
|
||||
write(r, 'generated/extra.md', 'not generated\n');
|
||||
const res = run(r, commit(r, 'second output by hand'));
|
||||
expect(res.code).toBe(1);
|
||||
expect(res.out).toContain('must carry v1.2.4.0');
|
||||
});
|
||||
|
||||
test('an output that is not what the generator produces at the base VERSION still counts as a stamp', () => {
|
||||
|
||||
Reference in new issue
Block a user