mirror of https://github.com/garrytan/gstack.git
test(evals): harden no-op asserts, close tier-invariant fail-open holes, pin gate question strings
- no-op regression: gate-must-ask is now UNCONDITIONAL for eng/design (the outcome==='asked' conditional let a silent-bypass plan_ready run sail through); eng/design cases force --disallowedTools so the pinned prose shape is contractual rather than hoping native AUQ renders match; the named-target case uses trackTokens for consumption and lists wrote_findings_before_asking in its diagnostic throw branch. - tier-alignment invariant: both quote styles matched; zero-self-gate, mixed-tier, and owning-keys-without-E2E_TIERS-entries are all REPORTED instead of silently skipped (the fail-open holes three reviewers found). - drift-guard: the generated gate menus must carry the exact question/option strings the PTY question detector anchors on — free CI fails before the paid smokes can go vacuous on a menu reword. - touchfiles: corrected the no-op cost note for CI concurrency + retry semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
4e61233021
commit
3ddbee5952
|
|
@ -22,7 +22,10 @@ import * as path from 'path';
|
|||
import { E2E_TOUCHFILES, E2E_TIERS, LLM_JUDGE_TOUCHFILES } from './helpers/touchfiles';
|
||||
|
||||
const TEST_DIR = import.meta.dir;
|
||||
const SELF_GATE_RE = /EVALS_TIER\s*===\s*'(gate|periodic)'/g;
|
||||
// Both quote styles — a mechanical refactor to double quotes must not
|
||||
// silently drop a file from the invariant (fail-open is the defect class
|
||||
// this test exists to kill).
|
||||
const SELF_GATE_RE = /EVALS_TIER\s*===\s*['"](gate|periodic)['"]/g;
|
||||
|
||||
describe('E2E tier alignment (touchfiles declaration vs test self-gate)', () => {
|
||||
const testFiles = readdirSync(TEST_DIR)
|
||||
|
|
@ -33,24 +36,40 @@ describe('E2E tier alignment (touchfiles declaration vs test self-gate)', () =>
|
|||
|
||||
test('every self-gated test file named in a dep list matches its declared tier', () => {
|
||||
const misaligned: string[] = [];
|
||||
const unmapped: string[] = [];
|
||||
const reported: string[] = [];
|
||||
|
||||
for (const file of testFiles) {
|
||||
const content = readFileSync(path.join(TEST_DIR, file), 'utf-8');
|
||||
const tiers = new Set<string>();
|
||||
for (const m of content.matchAll(SELF_GATE_RE)) tiers.add(m[1]);
|
||||
if (tiers.size !== 1) continue; // no self-gate, or mixed-tier file — out of scope
|
||||
const repoPath = `test/${file}`;
|
||||
if (tiers.size === 0) {
|
||||
// Every skill-e2e file is expected to self-gate; zero matches means
|
||||
// either a genuinely ungated file or a gate shape the regex can't
|
||||
// see — both worth a visible report, never a silent skip.
|
||||
reported.push(`${repoPath}: no detectable EVALS_TIER self-gate`);
|
||||
continue;
|
||||
}
|
||||
if (tiers.size > 1) {
|
||||
reported.push(`${repoPath}: mixed-tier self-gates (${[...tiers].join(', ')}) — not tier-checked`);
|
||||
continue;
|
||||
}
|
||||
const selfTier = [...tiers][0];
|
||||
|
||||
const repoPath = `test/${file}`;
|
||||
const owningKeys = Object.keys(allDeps).filter((k) => allDeps[k].includes(repoPath));
|
||||
if (owningKeys.length === 0) {
|
||||
unmapped.push(`${repoPath} (self-gates '${selfTier}')`);
|
||||
reported.push(`${repoPath} (self-gates '${selfTier}'): not named in any touchfiles dep list`);
|
||||
continue;
|
||||
}
|
||||
for (const k of owningKeys) {
|
||||
const declared = E2E_TIERS[k];
|
||||
if (declared && declared !== selfTier) {
|
||||
if (!declared) {
|
||||
// A dep-list key with no E2E_TIERS entry (e.g. an LLM-judge key)
|
||||
// can't tier-check this file — report instead of silently passing.
|
||||
reported.push(`${repoPath}: matched key '${k}' which has no E2E_TIERS entry`);
|
||||
continue;
|
||||
}
|
||||
if (declared !== selfTier) {
|
||||
misaligned.push(
|
||||
`${repoPath}: self-gates on '${selfTier}' but E2E_TIERS['${k}'] declares '${declared}' — the declaration is inert`,
|
||||
);
|
||||
|
|
@ -58,13 +77,12 @@ describe('E2E tier alignment (touchfiles declaration vs test self-gate)', () =>
|
|||
}
|
||||
}
|
||||
|
||||
// Reported, not asserted: these files' evals can't be tier-checked until
|
||||
// their dep lists name the test file. Add `test/<file>` to the eval's
|
||||
// touchfiles entry to bring them under the invariant.
|
||||
if (unmapped.length > 0) {
|
||||
// Reported, not asserted: coverage holes the invariant can see but not
|
||||
// arbitrate. Add the test file to its eval's dep list (or a tier entry
|
||||
// for the key) to bring it under the invariant.
|
||||
if (reported.length > 0) {
|
||||
console.warn(
|
||||
`[tier-alignment] ${unmapped.length} self-gated test file(s) not named in any touchfiles dep list:\n ` +
|
||||
unmapped.join('\n '),
|
||||
`[tier-alignment] ${reported.length} file(s) outside the invariant:\n ` + reported.join('\n '),
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -3256,7 +3256,7 @@ describe('scope-gate exceptions drift-guard', () => {
|
|||
// instead of drifting. The real fix (shared {{SCOPE_GATE}} resolver) is a
|
||||
// filed TODO — this guard is the stopgap that makes the duplication safe.
|
||||
const START_MARKER = '**Exceptions — check in this order, BEFORE asking:**';
|
||||
const END_MARKER = 'nothing changes: this gate is a hard STOP.';
|
||||
const END_MARKER = 'in any mode — it is a hard STOP.';
|
||||
|
||||
function extractExceptionsBlock(skill: string): string {
|
||||
const md = fs.readFileSync(path.join(ROOT, skill, 'SKILL.md'), 'utf-8');
|
||||
|
|
@ -3290,6 +3290,18 @@ describe('scope-gate exceptions drift-guard', () => {
|
|||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('gate menu carries the question strings the PTY question detector pins', () => {
|
||||
// isScopeGateQuestionVisible (claude-pty-runner.ts) anchors on the
|
||||
// question text + option A's body. If the menu is reworded without
|
||||
// updating the detector, the paid smokes' must-stay-false assertions go
|
||||
// vacuous — this free pin fails first.
|
||||
for (const skill of ['plan-eng-review', 'plan-design-review']) {
|
||||
const md = fs.readFileSync(path.join(ROOT, skill, 'SKILL.md'), 'utf-8');
|
||||
expect(md, `${skill}: gate question text`).toContain('What should I review?');
|
||||
expect(md, `${skill}: option A body text`).toContain('The current branch diff');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('GSTACK REVIEW REPORT mandatory unresolved-decisions status', () => {
|
||||
|
|
|
|||
|
|
@ -103,8 +103,11 @@ export const E2E_TOUCHFILES: Record<string, string[]> = {
|
|||
'plan-design-review-plan-mode': ['plan-design-review/**', 'scripts/resolvers/preamble/generate-completion-status.ts', 'scripts/resolvers/question-tuning.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/review.ts', 'test/helpers/claude-pty-runner.ts', 'test/skill-e2e-plan-design-plan-mode.test.ts'],
|
||||
'plan-devex-review-plan-mode': ['plan-devex-review/**', 'scripts/resolvers/preamble/generate-completion-status.ts', 'scripts/resolvers/question-tuning.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/preamble.ts', 'scripts/resolvers/review.ts', 'test/helpers/claude-pty-runner.ts'],
|
||||
// Covers ceo (preamble misfire) + eng/design (scope-gate bypass must not
|
||||
// fire outside plan mode) + the named-target exception case. 4 sequential
|
||||
// PTY runs (~+10 min, ~+$2 vs the pre-bypass single run).
|
||||
// fire outside plan mode) + the named-target exception case. 4 PTY runs;
|
||||
// in CI these run CONCURRENT with the rest of the pty-plan-smoke suite
|
||||
// (--max-concurrency + --retry 2), so worst-case cost is ~3x a single
|
||||
// pass of each, sharing the API budget with sibling tests — not the
|
||||
// sequential ~+10min a local read suggests.
|
||||
'plan-mode-no-op': ['plan-ceo-review/**', 'plan-eng-review/**', 'plan-design-review/**', 'scripts/resolvers/preamble/generate-completion-status.ts', 'scripts/resolvers/preamble.ts', 'test/helpers/claude-pty-runner.ts', 'test/skill-e2e-plan-mode-no-op.test.ts'],
|
||||
|
||||
// v1.21+ AskUserQuestion-blocked regression tests — Conductor launches
|
||||
|
|
|
|||
|
|
@ -31,7 +31,6 @@
|
|||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { existsSync, readFileSync } from 'node:fs';
|
||||
import { runPlanSkillObservation } from './helpers/claude-pty-runner';
|
||||
|
||||
const shouldRun = !!process.env.EVALS && process.env.EVALS_TIER === 'gate';
|
||||
|
|
@ -64,6 +63,15 @@ describeE2E('plan-mode-info no-op outside plan mode (gate regression)', () => {
|
|||
skillName,
|
||||
inPlanMode: false,
|
||||
timeoutMs: 300_000,
|
||||
// eng/design: force the prose-fallback path. The unconditional
|
||||
// gate-must-ask assert below pins the render shape the detector
|
||||
// anchors on, and only the --disallowedTools prose fallback makes
|
||||
// that shape CONTRACTUAL ("use exactly this shape" in the template);
|
||||
// native AskUserQuestion could render terse option labels that a
|
||||
// correct run would fail on (red-team finding).
|
||||
...(skillName === 'plan-ceo-review'
|
||||
? {}
|
||||
: { extraArgs: ['--disallowedTools', 'AskUserQuestion'] }),
|
||||
});
|
||||
|
||||
if (obs.outcome === 'silent_write' || obs.outcome === 'exited' || obs.outcome === 'timeout') {
|
||||
|
|
@ -85,12 +93,13 @@ describeE2E('plan-mode-info no-op outside plan mode (gate regression)', () => {
|
|||
// Scope-gate bypass must not misfire: no auto-select announcement
|
||||
// outside plan mode.
|
||||
expect(obs.scopeGateAutoSelectObserved ?? false).toBe(false);
|
||||
// And when a question fired, it must have been the scope gate —
|
||||
// outside plan mode with no named target, the gate is the FIRST
|
||||
// question by contract (hard STOP before any tool call).
|
||||
if (obs.outcome === 'asked') {
|
||||
expect(obs.scopeGateQuestionObserved ?? false).toBe(true);
|
||||
}
|
||||
// UNCONDITIONAL: outside plan mode with no named target, the gate is
|
||||
// a hard STOP before any tool call, so the gate question must have
|
||||
// rendered no matter which terminal outcome fired. Gating this on
|
||||
// outcome === 'asked' would let a silent-bypass run that reaches
|
||||
// plan_ready (isPlanReadyVisible also matches common prose) sail
|
||||
// through — the exact regression this test exists to catch.
|
||||
expect(obs.scopeGateQuestionObserved ?? false).toBe(true);
|
||||
}
|
||||
}, 360_000);
|
||||
}
|
||||
|
|
@ -106,10 +115,16 @@ describeE2E('plan-mode-info no-op outside plan mode (gate regression)', () => {
|
|||
skillName: 'plan-eng-review',
|
||||
inPlanMode: false,
|
||||
initialPlanContent: NAMED_TARGET_SEED,
|
||||
trackTokens: [SEED_TOKEN],
|
||||
timeoutMs: 300_000,
|
||||
});
|
||||
|
||||
if (obs.outcome === 'silent_write' || obs.outcome === 'exited' || obs.outcome === 'timeout') {
|
||||
if (
|
||||
obs.outcome === 'wrote_findings_before_asking' ||
|
||||
obs.outcome === 'silent_write' ||
|
||||
obs.outcome === 'exited' ||
|
||||
obs.outcome === 'timeout'
|
||||
) {
|
||||
throw new Error(
|
||||
`named-target no-op FAILED: outcome=${obs.outcome}\n` +
|
||||
`summary: ${obs.summary}\n` +
|
||||
|
|
@ -125,14 +140,10 @@ describeE2E('plan-mode-info no-op outside plan mode (gate regression)', () => {
|
|||
expect(obs.scopeGateQuestionObserved ?? false).toBe(false);
|
||||
expect(obs.scopeGateAutoSelectObserved ?? false).toBe(false);
|
||||
|
||||
// Target consumption: the review must reference the seeded feature.
|
||||
// evidence is the lossy 2KB tail, so also accept the token appearing in
|
||||
// the plan file the review wrote — the token names the component, route,
|
||||
// and test file every finding has to discuss, so absence from BOTH means
|
||||
// the pasted target was not actually reviewed.
|
||||
const inEvidence = obs.evidence.includes(SEED_TOKEN);
|
||||
const inPlanFile =
|
||||
!!obs.planFile && existsSync(obs.planFile) && readFileSync(obs.planFile, 'utf-8').includes(SEED_TOKEN);
|
||||
expect(inEvidence || inPlanFile).toBe(true);
|
||||
// Target consumption via high-water token tracking over the CUMULATIVE
|
||||
// buffer — the 2KB evidence tail is lossy and the plan-file fallback is
|
||||
// unreachable outside plan mode (extractPlanFilePath only matches
|
||||
// plan-mode save renders).
|
||||
expect(obs.tokensObserved?.[SEED_TOKEN] ?? false).toBe(true);
|
||||
}, 360_000);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue