From b24bb7a49cb4a525fe097e58081f0ffe97f92483 Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 2 Oct 2026 22:00:34 +0000 Subject: [PATCH] fix(evals): qa protocol names defined for the judge, async PTY judge so overlapping sessions keep their cleanup deadline, bws setup cases use the standard JUDGE_MS tier --- qa-only/sections/exploratory.md | 16 ++++++----- qa/SKILL.md | 5 ++-- qa/SKILL.md.tmpl | 5 ++-- qa/sections/exploratory.md | 16 ++++++----- qa/sections/qa-patterns.md | 1 + scripts/resolvers/qa.ts | 16 ++++++----- scripts/resolvers/utility.ts | 1 + test/helpers/pty/judge.ts | 36 +++++++++++++++---------- test/helpers/pty/runners/observation.ts | 2 +- test/pty-screen-supervision.test.ts | 22 +++++++++++++++ test/skill-e2e-bws.test.ts | 6 ++--- 11 files changed, 86 insertions(+), 40 deletions(-) diff --git a/qa-only/sections/exploratory.md b/qa-only/sections/exploratory.md index 10d4df26d..63e18a692 100644 --- a/qa-only/sections/exploratory.md +++ b/qa-only/sections/exploratory.md @@ -38,13 +38,17 @@ Set SECONDS to the shorter mode/caller limit; an unlimited mode uses the caller' Without a total time limit, do not start D; announce finite command timeouts. Stop when scoped contracts are tested or blocked. Clocks/checkpoints use REPORT_DIR; mixed standalone runs use REPORT_DIR/browser and REPORT_DIR/functional, with one final report at REPORT_DIR. Caller paths win. -R = owned probe directory; D = R/deadline.json. Quote paths. -G = `$HOME/.claude/skills/gstack/bin/gstack-qa-deadline`; Q = `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence`. +Names used below (quote every path): +- R: this surface's owned probe directory, chosen above. D: `R/deadline.json`, which exists only for a bounded run. +- SECONDS: the total probe budget set above. NNN: a fresh three-digit ID (001, 002, ...) for each capture or checkpoint; never reuse one. +- G = `$HOME/.claude/skills/gstack/bin/gstack-qa-deadline`, the deadline guard: `start` creates D, `status` prints `remainingMs` and `expired`, and `run` executes one command, stopping it at the deadline. +- Q = `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence`, the functional evidence recorder: `capture` runs one command, stores its exit code, stdout and stderr under `R/.qa-evidence/NNN/`, and prints status `complete`, `incomplete` or `sensitive`. + Start once before baseline: `bun G start D SECONDS [EARLIER_UTC]` if bounded. EARLIER_UTC = caller's absolute deadline, if set. Functional: `bun Q capture R NNN [--public] --deadline D -- COMMAND ARGS`. -Unbounded: use `--timeout-ms MS` instead. Use fresh three-digit IDs. ---public requires approved public/synthetic output; Q screens credentials. For complete private captures, await a safe Read of `R/.qa-evidence/NNN/observation.json`. Sensitive/incomplete captures cannot anchor checkpoints. +Unbounded: use `--timeout-ms MS` instead of `--deadline D`. +--public requires approved public/synthetic output; Q screens credentials. For complete private captures, await a safe Read of `R/.qa-evidence/NNN/observation.json` (the decoded stdout). Sensitive/incomplete captures cannot anchor checkpoints. Bounded browsers: `bun G run D -- COMMAND ARGS`. No detached probes. Never reset D/bypass G. Expiry or invalid/missing D stops probes; report unfinished coverage. QA_DEADLINE receipts are not observations. @@ -74,8 +78,8 @@ Never batch probes. Preserve every safe program-JSON key/value and identity hash unchanged. Withhold unsafe values, disclose limits and stop that chain. Check fields before publication. No drafts/placeholders or invented safe-path redactions; corrections cannot repair published notes. - Functional: the next capture publishes it: `... --after PREV --hypothesis 'why' -- CMD` (PREV: last complete capture). Q supplies observed; never transcribe it. - Browser checkpoints use Write. + Functional: do not write this file; the next capture publishes it: `bun Q capture R NNN --deadline D --after PREV --hypothesis 'why' -- CMD` (PREV: the last complete capture's ID). Q supplies observed; never transcribe it. + Browser checkpoints use Write: create the four-field file yourself. Wait for successful checkpoint publication before dispatch. Never backfill or overwrite notes. 3. Run that exact probe; G enforces the deadline when bounded. diff --git a/qa/SKILL.md b/qa/SKILL.md index 100998a47..9ae3b1d9f 100644 --- a/qa/SKILL.md +++ b/qa/SKILL.md @@ -569,7 +569,8 @@ Prefer the richer of recent project test plans and plans in conversation over gi ## Phases 1-6: QA Baseline Follow the shared section's ordered preparation, then run its probe loop. -The numbered browser phases label techniques, not another workflow. +Inside that loop, browser runs apply qa-patterns.md's numbered Phases 1-6 as techniques +and functional runs apply system-functional.md; neither is a separate workflow. > **STOP.** Before running the selected target's QA baseline and exploratory probes, with caller-owned authority, Read `sections/exploratory.md` relative to the installed `qa`/`gstack-qa` SKILL.md directory in full and follow it. > Use this host's installed path, never the product working directory or another host's assets. @@ -632,7 +633,7 @@ Example: Value: protects=refundPayment rejects an empty reason; fails_when=the r Rejected (covered_elsewhere): "checkout renders"; checkout.e2e.ts:15 covers it, so extend that test. Extend an existing table or fixture when one covers the boundary; never add a production -seam for the test. Match 2-3 nearby tests' naming, imports, assertions and fixtures. Reproduce the failure +seam for the test. Match 2-3 nearby tests' naming, imports, assertions and fixtures, and detect the command that runs them. Reproduce the failure in a new native test. Run its detected command before repair; prove the defect caused its failure, not a bad fixture, import or service. Attribute it in the language's comment syntax: diff --git a/qa/SKILL.md.tmpl b/qa/SKILL.md.tmpl index ac3a41cfc..57971a61b 100644 --- a/qa/SKILL.md.tmpl +++ b/qa/SKILL.md.tmpl @@ -126,7 +126,8 @@ Prefer the richer of recent project test plans and plans in conversation over gi ## Phases 1-6: QA Baseline Follow the shared section's ordered preparation, then run its probe loop. -The numbered browser phases label techniques, not another workflow. +Inside that loop, browser runs apply qa-patterns.md's numbered Phases 1-6 as techniques +and functional runs apply system-functional.md; neither is a separate workflow. {{SECTION:exploratory}} @@ -179,7 +180,7 @@ and unclear contracts never authorize repair. {{TEST_VALUE_BAR:qa}} Extend an existing table or fixture when one covers the boundary; never add a production -seam for the test. Match 2-3 nearby tests' naming, imports, assertions and fixtures. Reproduce the failure +seam for the test. Match 2-3 nearby tests' naming, imports, assertions and fixtures, and detect the command that runs them. Reproduce the failure in a new native test. Run its detected command before repair; prove the defect caused its failure, not a bad fixture, import or service. Attribute it in the language's comment syntax: diff --git a/qa/sections/exploratory.md b/qa/sections/exploratory.md index 7a1c330c7..a3f4baa94 100644 --- a/qa/sections/exploratory.md +++ b/qa/sections/exploratory.md @@ -32,13 +32,17 @@ Set SECONDS to the shorter mode/caller limit; an unlimited mode uses the caller' Without a total time limit, do not start D; announce finite command timeouts. Stop when scoped contracts are tested or blocked. Clocks/checkpoints use REPORT_DIR; mixed standalone runs use REPORT_DIR/browser and REPORT_DIR/functional, with one final report at REPORT_DIR. Caller paths win. -R = owned probe directory; D = R/deadline.json. Quote paths. -G = `$HOME/.claude/skills/gstack/bin/gstack-qa-deadline`; Q = `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence`. +Names used below (quote every path): +- R: this surface's owned probe directory, chosen above. D: `R/deadline.json`, which exists only for a bounded run. +- SECONDS: the total probe budget set above. NNN: a fresh three-digit ID (001, 002, ...) for each capture or checkpoint; never reuse one. +- G = `$HOME/.claude/skills/gstack/bin/gstack-qa-deadline`, the deadline guard: `start` creates D, `status` prints `remainingMs` and `expired`, and `run` executes one command, stopping it at the deadline. +- Q = `$HOME/.claude/skills/gstack/bin/gstack-qa-evidence`, the functional evidence recorder: `capture` runs one command, stores its exit code, stdout and stderr under `R/.qa-evidence/NNN/`, and prints status `complete`, `incomplete` or `sensitive`. + Start once before baseline: `bun G start D SECONDS [EARLIER_UTC]` if bounded. EARLIER_UTC = caller's absolute deadline, if set. Functional: `bun Q capture R NNN [--public] --deadline D -- COMMAND ARGS`. -Unbounded: use `--timeout-ms MS` instead. Use fresh three-digit IDs. ---public requires approved public/synthetic output; Q screens credentials. For complete private captures, await a safe Read of `R/.qa-evidence/NNN/observation.json`. Sensitive/incomplete captures cannot anchor checkpoints. +Unbounded: use `--timeout-ms MS` instead of `--deadline D`. +--public requires approved public/synthetic output; Q screens credentials. For complete private captures, await a safe Read of `R/.qa-evidence/NNN/observation.json` (the decoded stdout). Sensitive/incomplete captures cannot anchor checkpoints. Bounded browsers: `bun G run D -- COMMAND ARGS`. No detached probes. Never reset D/bypass G. Expiry or invalid/missing D stops probes; report unfinished coverage. QA_DEADLINE receipts are not observations. @@ -57,8 +61,8 @@ Never batch probes. Preserve every safe program-JSON key/value and identity hash unchanged. Withhold unsafe values, disclose limits and stop that chain. Check fields before publication. No drafts/placeholders or invented safe-path redactions; corrections cannot repair published notes. - Functional: the next capture publishes it: `... --after PREV --hypothesis 'why' -- CMD` (PREV: last complete capture). Q supplies observed; never transcribe it. - Browser checkpoints use Write. + Functional: do not write this file; the next capture publishes it: `bun Q capture R NNN --deadline D --after PREV --hypothesis 'why' -- CMD` (PREV: the last complete capture's ID). Q supplies observed; never transcribe it. + Browser checkpoints use Write: create the four-field file yourself. Wait for successful checkpoint publication before dispatch. Never backfill or overwrite notes. 3. Run that exact probe; G enforces the deadline when bounded. diff --git a/qa/sections/qa-patterns.md b/qa/sections/qa-patterns.md index 96fd728c1..d55e5bd6e 100644 --- a/qa/sections/qa-patterns.md +++ b/qa/sections/qa-patterns.md @@ -54,6 +54,7 @@ Visit every reachable page (5-15 minutes). Score health; document 5-10 evidenced ### Regression (`--regression `) Run Full; append fixed/new issues and score delta. Preserve the supplied prior baseline. +A missing or unreadable baseline is a missing prerequisite: it blocks the comparison, not the Full run. ## Workflow diff --git a/scripts/resolvers/qa.ts b/scripts/resolvers/qa.ts index 9aad2f8eb..89a6fd27a 100644 --- a/scripts/resolvers/qa.ts +++ b/scripts/resolvers/qa.ts @@ -79,13 +79,17 @@ Set SECONDS to the shorter mode/caller limit; an unlimited mode uses the caller' Without a total time limit, do not start D; announce finite command timeouts. Stop when scoped contracts are tested or blocked. Clocks/checkpoints use REPORT_DIR; mixed standalone runs use REPORT_DIR/browser and REPORT_DIR/functional, with one final report at REPORT_DIR. Caller paths win. -R = owned probe directory; D = R/deadline.json. Quote paths. -G = \`${quoteSafePath(ctx.paths.binDir)}/gstack-qa-deadline\`; Q = \`${quoteSafePath(ctx.paths.binDir)}/gstack-qa-evidence\`. +Names used below (quote every path): +- R: this surface's owned probe directory, chosen above. D: \`R/deadline.json\`, which exists only for a bounded run. +- SECONDS: the total probe budget set above. NNN: a fresh three-digit ID (001, 002, ...) for each capture or checkpoint; never reuse one. +- G = \`${quoteSafePath(ctx.paths.binDir)}/gstack-qa-deadline\`, the deadline guard: \`start\` creates D, \`status\` prints \`remainingMs\` and \`expired\`, and \`run\` executes one command, stopping it at the deadline. +- Q = \`${quoteSafePath(ctx.paths.binDir)}/gstack-qa-evidence\`, the functional evidence recorder: \`capture\` runs one command, stores its exit code, stdout and stderr under \`R/.qa-evidence/NNN/\`, and prints status \`complete\`, \`incomplete\` or \`sensitive\`. + Start once before baseline: \`bun G start D SECONDS [EARLIER_UTC]\` if bounded. EARLIER_UTC = caller's absolute deadline, if set. Functional: \`bun Q capture R NNN [--public] --deadline D -- COMMAND ARGS\`. -Unbounded: use \`--timeout-ms MS\` instead. Use fresh three-digit IDs. ---public requires approved public/synthetic output; Q screens credentials. For complete private captures, await a safe Read of \`R/.qa-evidence/NNN/observation.json\`. Sensitive/incomplete captures cannot anchor checkpoints. +Unbounded: use \`--timeout-ms MS\` instead of \`--deadline D\`. +--public requires approved public/synthetic output; Q screens credentials. For complete private captures, await a safe Read of \`R/.qa-evidence/NNN/observation.json\` (the decoded stdout). Sensitive/incomplete captures cannot anchor checkpoints. Bounded browsers: \`bun G run D -- COMMAND ARGS\`. No detached probes. Never reset D/bypass G. Expiry or invalid/missing D stops probes; report unfinished coverage. QA_DEADLINE receipts are not observations. @@ -115,8 +119,8 @@ ${reportOnly ? ` For guarded text, copy the complete span between the guard's Preserve every safe program-JSON key/value and identity hash unchanged. Withhold unsafe values, disclose limits and stop that chain. Check fields before publication. No drafts/placeholders or invented safe-path redactions; corrections cannot repair published notes. - Functional: the next capture publishes it: \`... --after PREV --hypothesis 'why' -- CMD\` (PREV: last complete capture). Q supplies observed; never transcribe it. - Browser checkpoints use Write. + Functional: do not write this file; the next capture publishes it: \`bun Q capture R NNN --deadline D --after PREV --hypothesis 'why' -- CMD\` (PREV: the last complete capture's ID). Q supplies observed; never transcribe it. + Browser checkpoints use Write: create the four-field file yourself. Wait for successful checkpoint publication before dispatch. Never backfill or overwrite notes. 3. Run that exact probe; G enforces the deadline when bounded. diff --git a/scripts/resolvers/utility.ts b/scripts/resolvers/utility.ts index 564948ef0..5b2c1d24f 100644 --- a/scripts/resolvers/utility.ts +++ b/scripts/resolvers/utility.ts @@ -173,6 +173,7 @@ Visit every reachable page (5-15 minutes). Score health; document 5-10 evidenced ### Regression (\`--regression \`) Run Full; append fixed/new issues and score delta. Preserve the supplied prior baseline. +A missing or unreadable baseline is a missing prerequisite: it blocks the comparison, not the Full run. ## Workflow diff --git a/test/helpers/pty/judge.ts b/test/helpers/pty/judge.ts index cdc05e9e0..81c5bb4c4 100644 --- a/test/helpers/pty/judge.ts +++ b/test/helpers/pty/judge.ts @@ -5,7 +5,7 @@ import * as fs from 'node:fs'; import { resolveEvalModel } from '../../../lib/eval-model'; import { createHash } from 'node:crypto'; -import { spawnSync as nodeSpawnSync } from 'node:child_process'; +import { spawn } from 'node:child_process'; import { resolveClaudeBinary } from './binary'; // ──────────────────────────────────────────────────────────────────────────── @@ -85,15 +85,17 @@ export function logPtySnapshot(visible: string, ctx: { testName: string; elapsed * Implementation: spawns `claude -p --model claude-haiku-4-5` synchronously * with the prompt piped via stdin. Uses subscription auth (no API key env * required). 30-second timeout; returns 'unknown' on any failure mode - * (timeout, malformed JSON, missing claude binary). + * (timeout, malformed JSON, missing claude binary). Asynchronous: a PTY + * session sharing this process (bun --concurrent) keeps reading its terminal + * and running its close deadlines while the judge waits. * * Cache: identical snapshot hashes return the cached verdict without * re-calling. Cache lives in-process; resets between test runs. */ -export function judgePtyState( +export async function judgePtyState( visible: string, ctx?: { testName?: string }, -): PtyStateVerdict { +): Promise { // Normalize: strip trailing whitespace lines + take last 4KB. Hash the // normalized form so spinner-frame-only diffs (which all look "working") // don't bust the cache and rack up cost. @@ -132,16 +134,22 @@ ${tail} // Use the same binary resolution as every PTY launch in this file — // judgePtyState previously hardcoded bare 'claude' three definitions // below resolveClaudeBinary(), breaking under hermetic PATHs. - const result = nodeSpawnSync( - resolveClaudeBinary() ?? 'claude', - ['-p', '--model', resolveEvalModel('warmup'), '--max-turns', '1'], - { - input: prompt, - stdio: ['pipe', 'pipe', 'pipe'], - timeout: 30_000, - encoding: 'utf-8', - }, - ); + const result = await new Promise<{ status: number | null; stdout: string; stderr: string }>((resolve, reject) => { + const child = spawn( + resolveClaudeBinary() ?? 'claude', + ['-p', '--model', resolveEvalModel('warmup'), '--max-turns', '1'], + { stdio: ['pipe', 'pipe', 'pipe'] }, + ); + let stdout = ''; + let stderr = ''; + const timer = setTimeout(() => child.kill('SIGTERM'), 30_000); + child.stdout.setEncoding('utf-8').on('data', chunk => { stdout += chunk; }); + child.stderr.setEncoding('utf-8').on('data', chunk => { stderr += chunk; }); + child.stdin.on('error', () => {}); + child.on('error', error => { clearTimeout(timer); reject(error); }); + child.on('close', status => { clearTimeout(timer); resolve({ status, stdout, stderr }); }); + child.stdin.end(prompt); + }); const elapsedMs = Date.now() - judgeStart; if (result.status === 0 && result.stdout) { // Pull the first {...} JSON object out of stdout. Haiku occasionally diff --git a/test/helpers/pty/runners/observation.ts b/test/helpers/pty/runners/observation.ts index d1141df8e..b07f22c53 100644 --- a/test/helpers/pty/runners/observation.ts +++ b/test/helpers/pty/runners/observation.ts @@ -399,7 +399,7 @@ async function observationTick(run: ObservationRun, session: ClaudePtySession): if (elapsed > JUDGE_AFTER_MS && driver.now() - run.lastJudgeAt > JUDGE_INTERVAL_MS) { run.lastJudgeAt = driver.now(); logPtySnapshot(visible, { testName: opts.skillName, elapsedMs: elapsed, tag: 'judge-tick' }); - run.lastJudgeVerdict = judgePtyState(visible, { testName: opts.skillName }); + run.lastJudgeVerdict = await judgePtyState(visible, { testName: opts.skillName }); if (run.lastJudgeVerdict.state === 'waiting' && !pendingSeededCompletion) { run.waitingEverObserved = true; if (opts.requireProseEvidence && !run.proseAUQEverObserved) return 'continue'; diff --git a/test/pty-screen-supervision.test.ts b/test/pty-screen-supervision.test.ts index fc5a63968..eecab882b 100644 --- a/test/pty-screen-supervision.test.ts +++ b/test/pty-screen-supervision.test.ts @@ -218,3 +218,25 @@ console.log(JSON.stringify({attempts,total:clock,fileWall:1800000})); expect(proof.total).toBeLessThan(proof.fileWall); } finally { fs.rmSync(a.dir, { recursive: true, force: true }); } }, 25_000); + +test('the PTY state judge leaves the event loop to concurrent sessions while it waits', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'pty-judge-')); + const claude = path.join(dir, 'claude'); + const worker = path.join(dir, 'worker.ts'); + try { + fs.writeFileSync(claude, '#!/bin/sh\ncat >/dev/null\nsleep 1\necho \'{"state":"waiting","reasoning":"fake judge"}\'\n', { mode: 0o755 }); + fs.writeFileSync(worker, `import {judgePtyState} from ${JSON.stringify(pathToFileURL(path.join(ROOT, 'test/helpers/claude-pty-runner.ts')).href)}; +const ticks=[];const timer=setInterval(()=>ticks.push(performance.now()),50); +const verdict=await judgePtyState('Which layout? 1. One panel 2. Two panels',{testName:'nonblocking'}); +clearInterval(timer); +console.log(JSON.stringify({state:verdict.state,elapsedMs:verdict.elapsedMs,ticks:ticks.length})); +`); + const result = spawnSync(process.execPath, [worker], { cwd: ROOT, encoding: 'utf8', timeout: 15_000, + env: { ...process.env, BROWSE_TERMINAL_BINARY: claude, HOME: dir } }); + expect(result.status, result.stdout + result.stderr).toBe(0); + const proof = JSON.parse(result.stdout.trim().split('\n').at(-1)!); + expect(proof.state).toBe('waiting'); + expect(proof.elapsedMs).toBeGreaterThanOrEqual(1000); + expect(proof.ticks).toBeGreaterThanOrEqual(10); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } +}, 20_000); diff --git a/test/skill-e2e-bws.test.ts b/test/skill-e2e-bws.test.ts index 4a8dbfdaf..19222e93a 100644 --- a/test/skill-e2e-bws.test.ts +++ b/test/skill-e2e-bws.test.ts @@ -142,7 +142,7 @@ ${setupBlock} Report the exact output. Do NOT try to fix or install anything — just report what you see.`, workingDirectory: emptyDir, maxTurns: 5, - timeout: 30_000, + timeout: JUDGE_MS, testName: 'skillmd-no-local-binary', runId, }); @@ -183,7 +183,7 @@ ${setupBlock} Report the exact output — either "READY: " or "NEEDS_SETUP".`, workingDirectory: nonGitDir, maxTurns: 5, - timeout: 30_000, + timeout: JUDGE_MS, testName: 'skillmd-outside-git', runId, }); @@ -259,7 +259,7 @@ Replace N with a confidence score 1-10. Log the operational learning now. Then say what you logged.`, workingDirectory: opDir, maxTurns: 5, - timeout: 30_000, + timeout: JUDGE_MS, testName: 'operational-learning', runId, });