mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-08 09:48:20 +01:00
fix(flaky): xvfb welcome marker written atomically; ship-publication-gates kills a stuck block's whole tree before cleanup; hook spawns name a kill instead of reading it as exit 1
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
ad8e12dff7
commit
d6fa0d8cc3
3 files changed
+83
-31
No files matched your search
@@ -373,6 +373,7 @@ describe.skipIf(process.platform !== 'linux')('daemon-owned display lifecycle',
|
||||
fs.writeFileSync(preload, `
|
||||
import * as fs from 'node:fs';
|
||||
import { BrowserManager } from ${JSON.stringify(path.resolve(import.meta.dir, '../src/browser-manager.ts'))};
|
||||
import { atomicWriteSync } from ${JSON.stringify(path.resolve(import.meta.dir, '../../lib/fs-atomic.ts'))};
|
||||
const getPage = BrowserManager.prototype.getPage;
|
||||
const wrapped = new WeakSet();
|
||||
let rejectWelcome;
|
||||
@@ -383,7 +384,7 @@ describe.skipIf(process.platform !== 'linux')('daemon-owned display lifecycle',
|
||||
const goto = page.goto.bind(page);
|
||||
page.goto = async (url, options) => {
|
||||
if (new URL(url).pathname === '/welcome') {
|
||||
fs.writeFileSync(${JSON.stringify(entered)}, JSON.stringify(this.getXvfbHandle()));
|
||||
atomicWriteSync(${JSON.stringify(entered)}, JSON.stringify(this.getXvfbHandle()));
|
||||
if (${JSON.stringify(mode)} === 'shutdown') {
|
||||
return new Promise((resolve, reject) => {
|
||||
rejectWelcome = () => reject(new Error('Welcome interrupted during shutdown'));
|
||||
|
||||
+20
-15
@@ -11,14 +11,26 @@ usePrivateStateRoot();
|
||||
const CAREFUL_SCRIPT = path.join(ROOT, 'careful', 'bin', 'check-careful.sh');
|
||||
const FREEZE_SCRIPT = path.join(ROOT, 'freeze', 'bin', 'check-freeze.sh');
|
||||
|
||||
function runHook(scriptPath: string, input: object, env?: Record<string, string>, cwd?: string): { exitCode: number; output: any; raw: string } {
|
||||
// A hook spawn's kill sits under the runner's 30s per-test timeout, not at 5s.
|
||||
// Windows Free Tests, 39 runs: every check-careful test p99.9 1.8s, max 3.3s
|
||||
// (n=8736), except the file's first hook spawn (p50 1.5s, max 3.0s passing),
|
||||
// which the old 5s kill cut down three times on 2026-10-29 and reported as
|
||||
// "exited 1". A killed or unspawnable hook now throws with its elapsed time.
|
||||
const HOOK_SPAWN_TIMEOUT_MS = 25_000;
|
||||
|
||||
function spawnHook(scriptPath: string, rawInput: string, env?: Record<string, string>, cwd?: string): { exitCode: number; output: any; raw: string } {
|
||||
const started = performance.now();
|
||||
const result = spawnSync('bash', [scriptPath], {
|
||||
input: JSON.stringify(input),
|
||||
input: rawInput,
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
env: { ...process.env, ...env },
|
||||
cwd,
|
||||
timeout: 5000,
|
||||
timeout: HOOK_SPAWN_TIMEOUT_MS,
|
||||
});
|
||||
if (result.error || result.signal) {
|
||||
const elapsed = Math.round(performance.now() - started);
|
||||
throw new Error(`${path.basename(scriptPath)} did not exit on its own: ${result.signal ?? result.error?.message} after ${elapsed}ms (kill at ${HOOK_SPAWN_TIMEOUT_MS}ms); stderr: ${result.stderr?.toString().trim() || '(empty)'}`);
|
||||
}
|
||||
const raw = result.stdout.toString().trim();
|
||||
let output: any = {};
|
||||
try {
|
||||
@@ -27,6 +39,10 @@ function runHook(scriptPath: string, input: object, env?: Record<string, string>
|
||||
return { exitCode: result.status ?? 1, output, raw };
|
||||
}
|
||||
|
||||
function runHook(scriptPath: string, input: object, env?: Record<string, string>, cwd?: string): { exitCode: number; output: any; raw: string } {
|
||||
return spawnHook(scriptPath, JSON.stringify(input), env, cwd);
|
||||
}
|
||||
|
||||
// Scratch git repo with a resolvable origin default branch — the HIGH-tier
|
||||
// force-push check reads `git symbolic-ref refs/remotes/origin/HEAD` from the
|
||||
// hook's cwd, and Conductor worktrees don't reliably carry that ref.
|
||||
@@ -46,18 +62,7 @@ function withGitRepo(defaultBranch: string, currentBranch: string, fn: (repoDir:
|
||||
}
|
||||
|
||||
function runHookRaw(scriptPath: string, rawInput: string, env?: Record<string, string>): { exitCode: number; output: any; raw: string } {
|
||||
const result = spawnSync('bash', [scriptPath], {
|
||||
input: rawInput,
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
env: { ...process.env, ...env },
|
||||
timeout: 5000,
|
||||
});
|
||||
const raw = result.stdout.toString().trim();
|
||||
let output: any = {};
|
||||
try {
|
||||
output = JSON.parse(raw);
|
||||
} catch {}
|
||||
return { exitCode: result.status ?? 1, output, raw };
|
||||
return spawnHook(scriptPath, rawInput, env);
|
||||
}
|
||||
|
||||
function carefulInput(command: string) {
|
||||
|
||||
@@ -2,6 +2,7 @@ import { expect, test } from 'bun:test';
|
||||
import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join, resolve } from 'node:path';
|
||||
import { killGroup } from '../lib/pregate/supervise';
|
||||
import { expectMentions } from './helpers/prompt-structure';
|
||||
|
||||
const template = readFileSync(new URL('../ship/sections/pr-body.md.tmpl', import.meta.url), 'utf8');
|
||||
@@ -34,11 +35,23 @@ const composeBlock = template.match(/```bash\n(: "\$\{DOCS_SECTION_FILE:[\s\S]*?
|
||||
const editBlock = fenceAfter('**Existing open PR/MR**');
|
||||
const createBlock = fenceAfter('**No open PR/MR:**');
|
||||
|
||||
/**
|
||||
* One deadline per case, under the runner's 30s per-test timeout. On Windows Free
|
||||
* Tests the first posting case in the file took 2.4-25.2s (40 runs) and four times
|
||||
* hit bun's timeout, which killed only the bash child: its descendants kept the root
|
||||
* as their working directory, so cleanup failed with EBUSY and hid the timeout. A
|
||||
* block still running at the deadline is killed with its whole tree, and the error
|
||||
* names the stage and what the earlier stages took.
|
||||
*/
|
||||
const CASE_DEADLINE_MS = 27_000;
|
||||
|
||||
/**
|
||||
* A repo whose origin is GitHub, a stub gh that records argv and stdin, and
|
||||
* the real gstack-post and gstack-prose-lint at the installed path the blocks call.
|
||||
*/
|
||||
function publicationRepo(root: string) {
|
||||
function publicationRepo(root: string, deadlineMs = CASE_DEADLINE_MS) {
|
||||
const started = performance.now();
|
||||
const deadline = started + deadlineMs;
|
||||
const log = join(root, 'log');
|
||||
const stubs = join(root, 'stubs');
|
||||
const bin = join(root, '.claude/skills/gstack/bin');
|
||||
@@ -61,10 +74,25 @@ for a in "$@"; do case "$a" in --title=*) printf '%s\\n' "\${a#--title=}" > "${l
|
||||
[ "$1 $2" = "pr create" ] && echo "https://github.com/o/r/pull/9"
|
||||
exit 0
|
||||
`, { mode: 0o755 });
|
||||
const run = (script: string) => Bun.spawnSync(['bash', '-c', script], {
|
||||
cwd: root, env: { ...process.env, ...gitEnv, HOME: root, PATH: `${stubs}:${process.env.PATH}`, DOCS_SECTION_FILE: join(root, 'section.md'), TMPDIR: root },
|
||||
stdout: 'pipe', stderr: 'pipe', timeout: 30_000,
|
||||
});
|
||||
const stages = [`setup ${Math.round(performance.now() - started)}ms`];
|
||||
const run = async (stage: string, script: string) => {
|
||||
const stageStart = performance.now();
|
||||
const child = Bun.spawn(['bash', '-c', script], {
|
||||
cwd: root, env: { ...process.env, ...gitEnv, HOME: root, PATH: `${stubs}:${process.env.PATH}`, DOCS_SECTION_FILE: join(root, 'section.md'), TMPDIR: root },
|
||||
stdout: 'pipe', stderr: 'pipe', detached: process.platform !== 'win32',
|
||||
});
|
||||
const output = Promise.all([new Response(child.stdout).text(), new Response(child.stderr).text()]);
|
||||
const expired = await Promise.race([child.exited.then(() => false), Bun.sleep(Math.max(0, deadline - performance.now())).then(() => true)]);
|
||||
if (expired) {
|
||||
killGroup(child.pid, 'SIGKILL');
|
||||
await child.exited;
|
||||
await output;
|
||||
throw new Error(`${stage} still running at the ${deadlineMs}ms case deadline after ${Math.round(performance.now() - stageStart)}ms; process tree killed; earlier stages: ${stages.join(', ')}`);
|
||||
}
|
||||
stages.push(`${stage} ${Math.round(performance.now() - stageStart)}ms`);
|
||||
const [stdout, stderr] = await output;
|
||||
return { exitCode: child.exitCode, stdout, stderr };
|
||||
};
|
||||
const calls = () => readdirSync(log).filter(f => f.endsWith('.args')).sort().map(f => ({
|
||||
args: readFileSync(join(log, f), 'utf8').split('\0').slice(0, -1),
|
||||
stdin: readFileSync(join(log, f.replace('.args', '.stdin')), 'utf8'),
|
||||
@@ -73,16 +101,16 @@ exit 0
|
||||
}
|
||||
|
||||
/** Compose the body with the real compose block and write the Step 18 title file. */
|
||||
function composed(root: string, opts: { title?: string; top?: string } = {}) {
|
||||
async function composed(root: string, opts: { title?: string; top?: string } = {}) {
|
||||
const repo = publicationRepo(root);
|
||||
writeFileSync(join(root, 'section.md'), '**Status:** current — no edits.');
|
||||
const compose = repo.run(writeDrafts(root, opts.top)(composeBlock));
|
||||
const compose = await repo.run('compose', writeDrafts(root, opts.top)(composeBlock));
|
||||
expect(compose.exitCode, compose.stderr.toString()).toBe(0);
|
||||
const bodyFile = /^PR_BODY_FILE: (\S+)$/m.exec(compose.stdout.toString())![1]!;
|
||||
writeFileSync(join(root, '.gstack/tmp/pr-title.abc123'), (opts.title ?? 'v1.2.3.4 fix: publication checks') + '\n');
|
||||
const fill = (block: string) => `PR_BODY_FILE=${JSON.stringify(bodyFile)}\n` +
|
||||
block.replace('<title-file-name>', 'pr-title.abc123').replaceAll('<pr-number>', '9').replace('<base>', 'main');
|
||||
return { ...repo, bodyFile, publish: (block: string) => repo.run(fill(block)) };
|
||||
return { ...repo, bodyFile, publish: (block: string) => repo.run('publish', fill(block)) };
|
||||
}
|
||||
|
||||
// The posting cases drive a POSIX stub gh that gstack-post spawns; Windows CreateProcess cannot
|
||||
@@ -90,15 +118,15 @@ function composed(root: string, opts: { title?: string; top?: string } = {}) {
|
||||
// test/gstack-post.test.ts.
|
||||
const postsThroughStubGh = (failure: string) => process.platform === 'win32' && failure === 'none';
|
||||
for (const failure of ['none', 'body', 'title'] as const) {
|
||||
test.skipIf(postsThroughStubGh(failure))(`publication posts only scanned bytes through gstack-post: ${failure}`, () => {
|
||||
test.skipIf(postsThroughStubGh(failure))(`publication posts only scanned bytes through gstack-post: ${failure}`, async () => {
|
||||
const root = mkdtempSync(join(tmpdir(), 'ship-post-'));
|
||||
try {
|
||||
const secret = 'AKIA' + '1234567890ABCDEF';
|
||||
const c = composed(root, {
|
||||
const c = await composed(root, {
|
||||
top: failure === 'body' ? `## Summary\nkey ${secret}\n\n## Documentation` : undefined,
|
||||
title: failure === 'title' ? `v1.2.3.4 fix: ${secret}` : undefined,
|
||||
});
|
||||
const result = c.publish(createBlock);
|
||||
const result = await c.publish(createBlock);
|
||||
if (failure === 'none') {
|
||||
expect(result.exitCode, result.stderr.toString()).toBe(0);
|
||||
expect(result.stdout.toString()).toContain('https://github.com/o/r/pull/9');
|
||||
@@ -114,18 +142,36 @@ for (const failure of ['none', 'body', 'title'] as const) {
|
||||
});
|
||||
}
|
||||
|
||||
test.skipIf(process.platform === 'win32')('MEDIUM findings publish nothing until the block reruns with the printed token', () => {
|
||||
test('a block still running at the case deadline is killed with its whole tree before cleanup', async () => {
|
||||
const root = mkdtempSync(join(tmpdir(), 'ship-post-deadline-'));
|
||||
const pidFile = join(root, 'holder.pid');
|
||||
const repo = publicationRepo(root, 1_500);
|
||||
await expect(repo.run('publish', `sh -c 'echo $$ > "${pidFile}"; exec sleep 30'\necho unreachable`))
|
||||
.rejects.toThrow(/^publish still running at the 1500ms case deadline after \d+ms; process tree killed; earlier stages: setup \d+ms$/);
|
||||
if (process.platform !== 'win32') {
|
||||
const holder = Number(readFileSync(pidFile, 'utf8'));
|
||||
let alive = true;
|
||||
for (let i = 0; alive && i < 50; i++) {
|
||||
try { process.kill(holder, 0); await Bun.sleep(20); } catch { alive = false; }
|
||||
}
|
||||
expect(alive).toBe(false);
|
||||
}
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
expect(existsSync(root)).toBe(false);
|
||||
});
|
||||
|
||||
test.skipIf(process.platform === 'win32')('MEDIUM findings publish nothing until the block reruns with the printed token', async () => {
|
||||
const root = mkdtempSync(join(tmpdir(), 'ship-post-medium-'));
|
||||
try {
|
||||
const c = composed(root, { top: '## Summary\nThanks jane.roe@acme-corp.io\n\n## Documentation' });
|
||||
const first = c.publish(editBlock);
|
||||
const c = await composed(root, { top: '## Summary\nThanks jane.roe@acme-corp.io\n\n## Documentation' });
|
||||
const first = await c.publish(editBlock);
|
||||
expect(first.exitCode).toBe(2);
|
||||
expect(first.stdout.toString()).toContain('RULE: pii.email LINE: 2 PART: body');
|
||||
expect(c.calls()).toEqual([]);
|
||||
const token = /^TOKEN: (\S+)$/m.exec(first.stdout.toString())![1]!;
|
||||
const flagged = 'pr-body <pr-number> --body-file "${PR_BODY_FILE:?restore the composed body path}"';
|
||||
expect(editBlock).toContain(flagged);
|
||||
const then = c.publish(editBlock.replace(flagged, `${flagged} --confirm ${token}`));
|
||||
const then = await c.publish(editBlock.replace(flagged, `${flagged} --confirm ${token}`));
|
||||
expect(then.exitCode, then.stderr.toString()).toBe(0);
|
||||
const calls = c.calls();
|
||||
expect(calls.map(call => call.args)).toEqual([
|
||||
|
||||
Reference in new issue
Block a user