mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix(version-gate): kill a release tool that ignores TERM at the limit, read a bounded log
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
8a656ced1a
commit
f5061445b1
3 files changed
+28
-9
No files matched your search
+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 head's merge base's `VERSION` (the head's own, since `VERSION` is unchanged on this path; the base tip may have stamped since, and CREW rule 14 says not to merge it); the gate runs the head's tool in a throwaway worktree with that `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, output read from a file so a lingering child cannot hold the gate open) 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".
|
||||
- **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 head's merge base's `VERSION` (the head's own, since `VERSION` is unchanged on this path; the base tip may have stamped since, and CREW rule 14 says not to merge it); the gate runs the head's tool in a throwaway worktree with that `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 enforced with SIGKILL so a tool that ignores TERM cannot hold the gate open, output read from a file, first 4 KB, so a lingering child cannot either) 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 merge base VERSION passes as unstamped (red without the fix), and stays unstamped after main stamps again; a tool whose background child outlives it does not hold the gate open; 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.
|
||||
- `test/compare-pr-version.test.ts`: a generator change with its regenerated output at the merge base VERSION passes as unstamped (red without the fix), and stays unstamped after main stamps again; a tool whose background child outlives it does not hold the gate open, nor does one that traps TERM (`regeneratedAtBase` is exported for that test); 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
|
||||
|
||||
|
||||
@@ -40,8 +40,9 @@ export function bumpVersion(version: string, level: string): string | null {
|
||||
* VERSION is unchanged on this path; the base tip may have stamped since):
|
||||
* the slot's restamp regenerates it anyway. Runs the head's tool in a
|
||||
* throwaway worktree with that VERSION and compares each changed output byte
|
||||
* for byte, reading the tool's output from a file so a lingering child cannot
|
||||
* hold the gate open. A stamp (new
|
||||
* for byte, reading the tool's output from a file (first 4 KB) so a lingering
|
||||
* child cannot hold the gate open, and killing a tool that outlives the limit
|
||||
* with SIGKILL so one that ignores TERM cannot either. A stamp (new
|
||||
* 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
|
||||
@@ -49,7 +50,8 @@ export function bumpVersion(version: string, level: string): string | null {
|
||||
* 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 } {
|
||||
export function regeneratedAtBase(o: { repoRoot: string; head: string; baseVersion: string; tool: string; outputs: string[]; timeoutMs?: number }): { kind: 'same' } | { kind: 'differs'; files: string[] } | { kind: 'error'; reason: string } {
|
||||
const timeoutMs = o.timeoutMs ?? 120_000;
|
||||
if (!o.baseVersion.trim()) return { kind: 'error', reason: 'the merge base has no VERSION to regenerate the release outputs at' };
|
||||
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)}` }; }
|
||||
@@ -64,10 +66,13 @@ function regeneratedAtBase(o: { repoRoot: string; head: string; baseVersion: str
|
||||
const logPath = path.join(os.tmpdir(), path.basename(staging) + '.log');
|
||||
const log = fs.openSync(logPath, 'w');
|
||||
let run: ReturnType<typeof spawnSync>;
|
||||
try { run = spawnSync('sh', ['-c', o.tool], { cwd: staging, stdio: ['ignore', log, log], timeout: 120_000, env }); } finally { fs.closeSync(log); }
|
||||
const output = (() => { try { return fs.readFileSync(logPath, 'utf8'); } catch { return ''; } finally { fs.rmSync(logPath, { force: true }); } })();
|
||||
try { run = spawnSync('sh', ['-c', o.tool], { cwd: staging, stdio: ['ignore', log, log], timeout: timeoutMs, killSignal: 'SIGKILL', env }); } finally { fs.closeSync(log); }
|
||||
const output = (() => {
|
||||
try { const fd = fs.openSync(logPath, 'r'); try { const buf = Buffer.alloc(4096); return buf.subarray(0, fs.readSync(fd, buf)).toString('utf8'); } finally { fs.closeSync(fd); } } catch { return ''; } finally { fs.rmSync(logPath, { force: true }); }
|
||||
})();
|
||||
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}`;
|
||||
const timedOut = run.signal !== null || (run.error as NodeJS.ErrnoException | undefined)?.code === 'ETIMEDOUT';
|
||||
const how = run.status !== null ? `exited ${run.status}` : timedOut ? `was killed at the ${Math.round(timeoutMs / 1000)}s limit` : (run.error?.message ?? 'did not exit');
|
||||
return { kind: 'error', reason: `release_tool \`${o.tool}\` ${how} at ${o.head.slice(0, 12)} with its merge base's VERSION, so its release outputs cannot be told from a stamp: ${output.trim().slice(0, 200)}` };
|
||||
}
|
||||
const files = o.outputs.filter(f => {
|
||||
|
||||
@@ -9,7 +9,7 @@ import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { BASE_POLICY, cleanup, makeFixtureRepo, type FixtureRepo } from './helpers/restamp-fixture';
|
||||
import { bumpVersion } from '../lib/version-gate-train';
|
||||
import { bumpVersion, regeneratedAtBase } from '../lib/version-gate-train';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const SCRIPT = path.join(ROOT, 'scripts', 'compare-pr-version.ts');
|
||||
@@ -216,6 +216,20 @@ describe('version gate: unstamped heads under a train policy', () => {
|
||||
expect(res.out).toContain('is unstamped under the base');
|
||||
});
|
||||
|
||||
test('a tool that ignores TERM is killed at the limit and reported, never waited for', () => {
|
||||
const r = fixture();
|
||||
branch(r);
|
||||
write(r, 'generated/digest.md', '# digest v1.2.3.0\nreworded body\n');
|
||||
const head = commit(r, 'output without a working tool');
|
||||
const started = Date.now();
|
||||
const res = regeneratedAtBase({ repoRoot: r.work, head, baseVersion: '1.2.3.0\n', tool: "trap '' TERM; echo stubborn; sleep 30", outputs: ['generated/digest.md'], timeoutMs: 2_000 });
|
||||
expect(Date.now() - started).toBeLessThan(10_000);
|
||||
expect(res.kind).toBe('error');
|
||||
expect((res as { reason: string }).reason).toContain('was killed at the 2s limit');
|
||||
expect((res as { reason: string }).reason).toContain('stubborn');
|
||||
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);
|
||||
|
||||
Reference in new issue
Block a user