diff --git a/scripts/typecheck-test-baseline.json b/scripts/typecheck-test-baseline.json index a1477b4ef..ee19e3b9f 100644 --- a/scripts/typecheck-test-baseline.json +++ b/scripts/typecheck-test-baseline.json @@ -485,7 +485,6 @@ "test/skill-e2e-shared-libs-paths.test.ts\tTS2769\tNo overload matches this call. The last overload gave the following error. Argument of type 'unknown' is not assignable to parameter of type 'string'.": 1, "test/skill-e2e-ship-docsync.test.ts\tTS2345\tArgument of type 'EvalCollector | null' is not assignable to parameter of type 'EvalCollector'. Type 'null' is not assignable to type 'EvalCollector'.": 9, "test/skill-e2e-third-party-actions.test.ts\tTS2345\tArgument of type '{ maxTurns: 8; allowedTools: readonly ['Read', 'Bash']; timeout: 240000; runId: string; env: { PATH: string; }; prompt: string; workingDirectory: string; testName: string; }' is not assignable to parameter of type '{ prompt: string; workingDirectory: string; maxTurns?: number | undefined; appendSystemPrompt?: string | undefined; completionReserveMs?: number | undefined; allowedTools?: string[] | undefined; ... 9 more ...; nativeLifecycle?: { ...; } | undefined; }'. Types of property 'allowedTools' are incompatible. The type 'readonly [\"Read\", \"Bash\"]' is 'readonly' and cannot be assigned to the mutable type 'string[]'.": 5, - "test/skill-e2e-triage.test.ts\tTS2353\tObject literal may only specify known properties, and 'has_in_branch_classification' does not exist in type 'Partial'.": 1, "test/skill-routing-e2e.test.ts\tTS2345\tArgument of type '\"e2e-routing\"' is not assignable to parameter of type '\"e2e\" | \"llm-judge\"'.": 1, "test/strict-output.test.ts\tTS2739\tType '{ failedTests: number; unhandledBetweenTests: number; terminalFileCounts: number[]; }' is missing the following properties from type 'BunTestOutputSummary': terminalTestCounts, skippedTests, passedTests": 5, "test/test-free-shards.test.ts\tTS2345\tArgument of type 'number | ReadableStream> | undefined' is not assignable to parameter of type 'BodyInit | null | undefined'. Type 'number' is not assignable to type 'BodyInit | null | undefined'.": 2, diff --git a/test/fixtures/ship-triage/known-bad-one-label.txt b/test/fixtures/ship-triage/known-bad-one-label.txt new file mode 100644 index 000000000..22ee0da9e --- /dev/null +++ b/test/fixtures/ship-triage/known-bad-one-label.txt @@ -0,0 +1 @@ +{"test/string.test.js": "in-branch", "test/math.test.js": "in-branch"} diff --git a/test/fixtures/ship-triage/known-bad-swapped.txt b/test/fixtures/ship-triage/known-bad-swapped.txt new file mode 100644 index 000000000..912a3353a --- /dev/null +++ b/test/fixtures/ship-triage/known-bad-swapped.txt @@ -0,0 +1,2 @@ +- string.test.js (truncate): PRE-EXISTING +- math.test.js (divide): IN-BRANCH diff --git a/test/fixtures/ship-triage/known-bad-words-only.txt b/test/fixtures/ship-triage/known-bad-words-only.txt new file mode 100644 index 000000000..6863e9b28 --- /dev/null +++ b/test/fixtures/ship-triage/known-bad-words-only.txt @@ -0,0 +1,2 @@ +I ran math.test and string.test. Some failures are in-branch and some are pre-existing; +FAIL: divide and FAIL: truncate both appeared. diff --git a/test/fixtures/ship-triage/known-good-json.txt b/test/fixtures/ship-triage/known-good-json.txt new file mode 100644 index 000000000..5a9dcaec5 --- /dev/null +++ b/test/fixtures/ship-triage/known-good-json.txt @@ -0,0 +1,4 @@ +Both test files ran. `test/string.test.js` fails because truncate(null) throws; src/string.js is new on this branch. +`test/math.test.js` fails on divide-by-zero; src/math.js is unchanged since main. + +{"test/string.test.js": "in-branch", "test/math.test.js": "pre-existing"} diff --git a/test/fixtures/ship-triage/known-good-lines.txt b/test/fixtures/ship-triage/known-good-lines.txt new file mode 100644 index 000000000..d80ddf06a --- /dev/null +++ b/test/fixtures/ship-triage/known-good-lines.txt @@ -0,0 +1,4 @@ +## Triage +- FAIL: truncate(null, 5) — **IN-BRANCH**: src/string.js was added on feature/string-utils. +- FAIL: divide — **PRE-EXISTING**: src/math.js is identical on main. +Recommendation (solo repo): fix the pre-existing failure now as well. diff --git a/test/helpers/ship-triage-labels.ts b/test/helpers/ship-triage-labels.ts new file mode 100644 index 000000000..af1572dca --- /dev/null +++ b/test/helpers/ship-triage-labels.ts @@ -0,0 +1,36 @@ +export type TriageLabel = 'in-branch' | 'pre-existing'; + +const LABEL = /\b(in[- ]branch|pre[- ]existing)\b/gi; +const normalize = (label: string): TriageLabel => (/^in/i.test(label) ? 'in-branch' : 'pre-existing'); + +/** + * Classify each seeded ship-triage failure from the agent's final message. + * Prefers the requested JSON object ({"": "in-branch" | "pre-existing"}); + * otherwise uses the lines that name exactly one failure and exactly one label. + * A failure labelled both ways, or not at all, stays undefined. + */ +export function triageLabels(output: string): { string?: TriageLabel; math?: TriageLabel } { + const result: { string?: TriageLabel; math?: TriageLabel } = {}; + for (const block of output.match(/\{[^{}]*\}/g) ?? []) { + let parsed: unknown; + try { parsed = JSON.parse(block); } catch { continue; } + if (!parsed || typeof parsed !== 'object') continue; + for (const [key, value] of Object.entries(parsed as Record)) { + if (typeof value !== 'string' || !/^(in[- ]branch|pre[- ]existing)$/i.test(value.trim())) continue; + if (/string|truncate/i.test(key)) result.string = normalize(value.trim()); + if (/math|divide/i.test(key)) result.math = normalize(value.trim()); + } + } + if (result.string && result.math) return result; + const seen: Record<'string' | 'math', Set> = { string: new Set(), math: new Set() }; + for (const line of output.split('\n')) { + const names = (['string', 'math'] as const).filter(name => + (name === 'string' ? /string|truncate/i : /math|divide/i).test(line)); + const labels = [...new Set([...line.matchAll(LABEL)].map(match => normalize(match[1]!)))]; + if (names.length === 1 && labels.length === 1) seen[names[0]!].add(labels[0]!); + } + for (const name of ['string', 'math'] as const) { + if (!result[name] && seen[name].size === 1) result[name] = [...seen[name]][0]; + } + return result; +} diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index b4cfa7d82..d8654523b 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -793,7 +793,7 @@ export const E2E_TOUCHFILES: Record = { "scripts/resolvers/preamble/generate-preamble-bash.ts", "scripts/resolvers/preamble/generate-completion-status.ts", 'test/helpers/coverage-audit.ts', 'test/helpers/office-hours-attempt.ts', 'scripts/resolvers/testing.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/resolvers/review-dashboard.ts', 'scripts/resolvers/plan-gates.ts', 'scripts/resolvers/spec-review.ts', 'scripts/resolvers/outside-voice-steps.ts', 'scripts/resolvers/review-scope.ts', 'scripts/resolvers/outside-voice.ts' ], - 'ship-triage': ['bin/gstack-state-root.sh', 'lib/state-root.ts', 'ship/**', 'bin/gstack-repo-mode', 'test/skill-e2e-triage.test.ts', + 'ship-triage': ['bin/gstack-state-root.sh', 'lib/state-root.ts', 'ship/**', 'bin/gstack-repo-mode', 'test/skill-e2e-triage.test.ts', 'test/helpers/ship-triage-labels.ts', 'scripts/resolvers/testing.ts' ], 'ship-docsync-completion': ['bin/gstack-state-root.sh', 'lib/state-root.ts', 'ship/**', 'document-release/**', 'test/skill-e2e-ship-docsync.test.ts', 'test/helpers/docsync-*.ts', 'test/helpers/session-runner.ts', 'test/helpers/hermetic-env.ts', 'bin/gstack-skill-start', 'bin/gstack-session-kind', 'scripts/resolvers/sections.ts', 'scripts/resolvers/preamble/generate-ask-user-format.ts', 'scripts/gen-skill-docs.ts', 'scripts/resolvers/testing.ts', 'test/helpers/qa-checkpoint-evidence.ts', 'test/helpers/qa-functional-observer.ts', 'test/helpers/e2e-gate.ts', 'test/helpers/qa-evidence-producer.ts', 'test/helpers/qa-functional-fixture.ts'], diff --git a/test/ship-triage-labels.test.ts b/test/ship-triage-labels.test.ts new file mode 100644 index 000000000..f812f2af4 --- /dev/null +++ b/test/ship-triage-labels.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, test } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; +import { triageLabels } from './helpers/ship-triage-labels'; + +const DIR = path.join(import.meta.dir, 'fixtures', 'ship-triage'); +const read = (name: string) => fs.readFileSync(path.join(DIR, name), 'utf8'); +const correct = (output: string) => { + const labels = triageLabels(output); + return labels.string === 'in-branch' && labels.math === 'pre-existing'; +}; + +describe('ship triage label grader', () => { + test.each(['known-good-json.txt', 'known-good-lines.txt'])('accepts %s', (name) => { + expect(correct(read(name))).toBe(true); + }); + test.each(['known-bad-swapped.txt', 'known-bad-words-only.txt', 'known-bad-one-label.txt'])('rejects %s', (name) => { + expect(correct(read(name))).toBe(false); + }); + test('the previous word-presence check passed the words-only and swapped outputs', () => { + const previous = (output: string) => { + const lower = output.toLowerCase(); + return /in-branch|in branch|introduced/.test(lower) && /pre-existing|pre existing|existed before/.test(lower) + && /truncate|string/.test(lower) && /divide|math/.test(lower); + }; + expect(previous(read('known-bad-words-only.txt'))).toBe(true); + expect(previous(read('known-bad-swapped.txt'))).toBe(true); + }); +}); diff --git a/test/skill-e2e-triage.test.ts b/test/skill-e2e-triage.test.ts index 89f75db18..a20e86bfa 100644 --- a/test/skill-e2e-triage.test.ts +++ b/test/skill-e2e-triage.test.ts @@ -32,6 +32,7 @@ import { spawnSync } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; +import { triageLabels } from './helpers/ship-triage-labels'; const evalCollector = createEvalCollector('e2e-triage'); @@ -175,6 +176,7 @@ For each failing test, classify it as: Use git diff origin/main...HEAD (or git diff main...HEAD since there's no remote) to determine which files changed on this branch. Output your classification for each failure clearly, labeling each as "IN-BRANCH" or "PRE-EXISTING" with your reasoning. +End with one JSON object mapping each failing test file to "in-branch" or "pre-existing". This is a solo repo (REPO_MODE=solo). For pre-existing failures, recommend fixing now.`, workingDirectory: triageDir, @@ -188,49 +190,20 @@ This is a solo repo (REPO_MODE=solo). For pre-existing failures, recommend fixin logCost('/ship triage', result); const output = result.output || ''; - const outputLower = output.toLowerCase(); - - // The triage should identify the string/truncate failure as in-branch - const hasInBranch = outputLower.includes('in-branch') || outputLower.includes('in branch') || outputLower.includes('introduced'); - // The triage should identify the math/divide failure as pre-existing - const hasPreExisting = outputLower.includes('pre-existing') || outputLower.includes('pre existing') || outputLower.includes('existed before'); - - console.log(`Output identifies IN-BRANCH failures: ${hasInBranch}`); - console.log(`Output identifies PRE-EXISTING failures: ${hasPreExisting}`); - - // Check that the string/truncate bug is classified as in-branch - const mentionsTruncate = outputLower.includes('truncate') || outputLower.includes('string'); - const mentionsDivide = outputLower.includes('divide') || outputLower.includes('math'); - - console.log(`Mentions truncate/string (in-branch bug): ${mentionsTruncate}`); - console.log(`Mentions divide/math (pre-existing bug): ${mentionsDivide}`); - - // Verify BOTH failure classes are exercised (not just detected): - // The test runner must have actually run both test files - const ranMathTest = output.includes('math.test') || output.includes('FAIL: divide'); - const ranStringTest = output.includes('string.test') || output.includes('FAIL: truncate'); - console.log(`Ran math test file (pre-existing failure): ${ranMathTest}`); - console.log(`Ran string test file (in-branch failure): ${ranStringTest}`); + // Outcome: each seeded failure carries the right label, and the suite actually ran. + const labels = triageLabels(output); + const ranTests = result.toolCalls.some(call => call.tool === 'Bash' + && /test\/run\.js|test\/(?:math|string)\.test\.js/.test(String(call.input?.command ?? ''))); + console.log(`Triage labels: ${JSON.stringify(labels)}; ran tests: ${ranTests}`); recordE2E(evalCollector, '/ship triage', 'Test Failure Triage E2E', result, { - passed: result.exitReason === 'success' && hasInBranch && hasPreExisting, - has_in_branch_classification: hasInBranch, - has_pre_existing_classification: hasPreExisting, - mentions_truncate: mentionsTruncate, - mentions_divide: mentionsDivide, - ran_both_test_files: ranMathTest && ranStringTest, + passed: result.exitReason === 'success' && ranTests && labels.string === 'in-branch' && labels.math === 'pre-existing', }); expect(result.exitReason).toBe('success'); - // Must classify at least one failure as in-branch AND one as pre-existing - expect(hasInBranch).toBe(true); - expect(hasPreExisting).toBe(true); - // Must mention the specific bugs - expect(mentionsTruncate).toBe(true); - expect(mentionsDivide).toBe(true); - // Must have actually run both test files (exercises both failure classes) - expect(ranMathTest).toBe(true); - expect(ranStringTest).toBe(true); + expect(ranTests, 'the agent must run the test suite').toBe(true); + expect(labels.string, 'truncate/string failure is in-branch').toBe('in-branch'); + expect(labels.math, 'divide/math failure is pre-existing').toBe('pre-existing'); }, CAPTURE_MS); });