From 119ed92f9cd9fb425d87bbc892363fcaa331d529 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 9 Nov 2026 07:54:27 -0800 Subject: [PATCH] Fix seven test and eval checks that misread correct work or raced their neighbours (GST-2131) (#4223) * Fix seven test and eval checks that misread correct work or raced their neighbours (GST-2131) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Record the harness decision for the flaky-pass helpers (GST-2131) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Register the captured scrape fixture as free (GST-2131) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Point the whole codex-review excerpt at the checkout and stop counting refused or conditional defers (GST-2131) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Make the flaky-pass changelog entry read plainly and name lib paths (GST-2131) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- browse/test/handoff.test.ts | 6 +- .../capy-fix-flaky-checks-2026-11-09.md | 1 + .../test/helpers/session-runner.ts.json | 2 +- .../test/helpers/skill-fixture.ts.json | 2 +- scripts/lib/windows-curation.ts | 4 + .../scrape-prototype-goto-42571575838.json | 115 ++++++++++++++++++ test/helpers/auq-sdk-capture.ts | 2 + test/helpers/free-fixtures-data.ts | 1 + test/helpers/session-runner.test.ts | 29 ++++- test/helpers/session-runner.ts | 9 ++ test/helpers/skill-fixture.ts | 33 +++++ test/helpers/test-value-fixture.ts | 45 +++++++ test/helpers/third-party-actions.ts | 14 ++- test/skill-e2e-skillify.test.ts | 4 +- test/skill-e2e-test-value.test.ts | 19 +-- test/skill-e2e-workflow.test.ts | 29 +---- test/skill-fixture.test.ts | 28 +++++ test/test-value-bar.test.ts | 29 ++++- test/third-party-actions.test.ts | 28 +++++ 19 files changed, 346 insertions(+), 54 deletions(-) create mode 100644 changelog.d/capy-fix-flaky-checks-2026-11-09.md create mode 100644 test/fixtures/scrape-prototype-goto-42571575838.json diff --git a/browse/test/handoff.test.ts b/browse/test/handoff.test.ts index 793a40b45..d0bfeb86a 100644 --- a/browse/test/handoff.test.ts +++ b/browse/test/handoff.test.ts @@ -13,7 +13,7 @@ import { startTestServer } from './test-server'; import { BrowserManager, type BrowserState } from '../src/browser-manager'; import { handleWriteCommand as _handleWriteCommand } from '../src/write-commands'; import { handleMetaCommand } from '../src/meta-commands'; -import { spawnXvfb, pickFreeDisplay, isOurXvfb, type XvfbHandle } from '../src/xvfb'; +import { spawnFreeXvfb, isOurXvfb, type XvfbHandle } from '../src/xvfb'; // Per-FILE Chromium profile: this file launches an in-process persistent // context (BrowserManager.launch()), and sharing a profile dir with the @@ -210,9 +210,7 @@ const HEADED_BROKEN_ON_DARWIN = process.platform === 'darwin'; describe('handoff integration', () => { test.skipIf(process.platform !== 'linux')('restore failure after candidate assignment leaves the original manager and pages usable', async () => { - const displayNum = pickFreeDisplay(); - expect(displayNum).not.toBeNull(); - const display = await spawnXvfb(displayNum!); + const display = await spawnFreeXvfb(); const originalDisplay = process.env.DISPLAY; process.env.DISPLAY = display.display; const hbm = new BrowserManager(); diff --git a/changelog.d/capy-fix-flaky-checks-2026-11-09.md b/changelog.d/capy-fix-flaky-checks-2026-11-09.md new file mode 100644 index 000000000..e7f403b9a --- /dev/null +++ b/changelog.d/capy-fix-flaky-checks-2026-11-09.md @@ -0,0 +1 @@ +- **Seven test and eval checks that misread correct work, or raced their neighbours, now read it right (2026-11-09 daily flaky pass, GST-2131).** tpa-present's consent check now accepts the defer option in any words when the option says the step will be marked or left as blocked or waiting on you (gate 42431616220: "D) Do it later. I'll mark this step as waiting on you."), and still rejects every refusal of it, such as "I will not mark this step as blocked on you" or "I won't mark this step as waiting on you". Three weekly-census reds were harness misreads. scrape-prototype-path now credits browse's own "Navigated to (2xx)" line when the model navigated through `$PWD`; codex-review's skill excerpt now points every helper path, `bin/` and `lib/` alike, at the checkout instead of an install the fixture never seeds; and ship-coverage-value resolves a `file:line` gap inside refundPayment instead of requiring the word "refund". The handoff integration test takes its X display from the race-safe `spawnFreeXvfb()`, the AUQ capture fixture turns off git's detached auto-maintenance that left a directory behind after cleanup, and the Windows curated lane skips test/ship-finish-fixed-links.test.ts, which spawns shebang bins like its excluded siblings. No threshold, trial count, case kind or retry changed. diff --git a/scripts/harness-pins/test/helpers/session-runner.ts.json b/scripts/harness-pins/test/helpers/session-runner.ts.json index 24e92def6..b1dc4ddd3 100644 --- a/scripts/harness-pins/test/helpers/session-runner.ts.json +++ b/scripts/harness-pins/test/helpers/session-runner.ts.json @@ -1 +1 @@ -{"blob":"c73b2596d9eb5377530a6bf1d284e0b6ea1556a0","since":6,"nonBehavioral":"Cost attribution only (GSTA-150 PR 2): a display-label record carries cost_case_id from runSkillTest's testName and the report binds its cost by it while the failed record and failure class still come from the named records (test/paid-report-cost.test.ts pins both); an aborted Agent SDK session logs its streamed usage as streamed_cost_usd, used only when a case has no record; a listed model-free case with no record or session is a known $0. No case's execution, outcome, failure class, rerun eligibility or panel verdict changes, so no pass-rate series resets"} +{"blob":"0d5f36f91c31bf32d4997cde0c267f96caf57c7a","since":6,"nonBehavioral":"adds helpers called only by the changed scrape-prototype-path, codex-review and ship-coverage-value case files and turns off git auto-maintenance in the AUQ capture fixture; no existing caller's behavior changes (GST-2131)"} diff --git a/scripts/harness-pins/test/helpers/skill-fixture.ts.json b/scripts/harness-pins/test/helpers/skill-fixture.ts.json index a6731990a..68acc23c6 100644 --- a/scripts/harness-pins/test/helpers/skill-fixture.ts.json +++ b/scripts/harness-pins/test/helpers/skill-fixture.ts.json @@ -1 +1 @@ -{"blob":"ce7609eacc7a8265a4a211358d1b4fb49ea85bc5","since":6,"nonBehavioral":"GSTA-224 B2 (#3122): skill-fixture.ts follows review's renamed Step 2.5 heading ('Check for review bot comments') so the review E2E cases extract the same section, now the skill's own reworded bytes; llm-judge.ts updates only the cross-skill consistency judge's intended architecture to the renamed review-bot-history.md and its record helper. No other case's input or verdict can change; the cross-skill judge's judged files changed in this PR, so its owned inputs start its own new series"} +{"blob":"9fdd82f483d8edb440156d891443ae51114a2778","since":6,"nonBehavioral":"adds helpers called only by the changed scrape-prototype-path, codex-review and ship-coverage-value case files (the codex-review excerpt now rewrites the whole install prefix, lib/ included) and turns off git auto-maintenance in the AUQ capture fixture; no existing caller's behavior changes (GST-2131)"} diff --git a/scripts/lib/windows-curation.ts b/scripts/lib/windows-curation.ts index fbd406ddd..d94c6db81 100644 --- a/scripts/lib/windows-curation.ts +++ b/scripts/lib/windows-curation.ts @@ -447,6 +447,10 @@ export const KNOWN_WINDOWS_INCOMPATIBLE: Array<{ file: string; reason: string }> file: 'test/ship-record-ci-reuse.test.ts', reason: 'drives bin/gstack-evidence, gstack-ship-preflight and gstack-ship-release (shebangs) through test/helpers/ship-bin-fixture.ts like test/ship-accept-remote-gate-lanes.test.ts; windows-latest fails every case at the first bin spawn (windows-scoped-shard, run 42382484796)', }, + { + file: 'test/ship-finish-fixed-links.test.ts', + reason: 'drives bin/gstack-ship-release, gstack-approvals, gstack-review-log, gstack-docs-candidate and gstack-wtree (shebangs) through test/helpers/ship-bin-fixture.ts like test/ship-finish-ruled-notes.test.ts; windows-latest fails every case at the first bin spawn (windows-free-shard 3, run 42593329215)', + }, ]; // Force-include overrides: files a WINDOWS_FRAGILE_PATTERNS regex excludes for diff --git a/test/fixtures/scrape-prototype-goto-42571575838.json b/test/fixtures/scrape-prototype-goto-42571575838.json new file mode 100644 index 000000000..2a1692f89 --- /dev/null +++ b/test/fixtures/scrape-prototype-goto-42571575838.json @@ -0,0 +1,115 @@ +{ + "run": "42571575838", + "sha": "b5c690ce", + "case": "scrape-prototype-path", + "trials": [ + { + "trial": 1, + "source": "paid-slice-32-a1/skill-e2e-skillify--scrape-prototype-path.t1", + "cwd": "/home/runner/.cache/gstack-paid-shard-HfCKXD/tmp/skill-e2e-skillify-prototype-rfJ8rF", + "events": [ + { + "type": "assistant", + "parent_tool_use_id": null, + "message": { + "content": [ + { + "type": "tool_use", + "id": "toolu_011iBmQ9aZEhyGr1Mg32VUd8", + "name": "Bash", + "input": { + "command": "export GSTACK_HOME=\"$PWD/.gstack-home\"; B=/__w/gstack/gstack/browse/dist/browse; $B goto \"file://$PWD/fixture.html\"; $B html 2>&1 | head -80" + } + } + ] + } + }, + { + "type": "user", + "parent_tool_use_id": null, + "message": { + "content": [ + { + "type": "tool_result", + "tool_use_id": "toolu_011iBmQ9aZEhyGr1Mg32VUd8", + "content": "Navigated to file:///home/runner/.cache/gstack-paid-shard-HfCKXD/tmp/skill-e2e-skillify-prototype-rfJ8rF/fixture.html (200)" + } + ] + } + } + ] + }, + { + "trial": 2, + "source": "paid-slice-33-a1/skill-e2e-skillify--scrape-prototype-path.t2", + "cwd": "/home/runner/.cache/gstack-paid-shard-iFDuSx/tmp/skill-e2e-skillify-prototype-JLM2yo", + "events": [ + { + "type": "assistant", + "parent_tool_use_id": null, + "message": { + "content": [ + { + "type": "tool_use", + "id": "toolu_017pVuU1H9ruNJNAKZxn1ZXZ", + "name": "Bash", + "input": { + "command": "B=/__w/gstack/gstack/browse/dist/browse; export GSTACK_HOME=\"$PWD/.gstack-home\"; $B goto \"file://$PWD/fixture.html\" && $B html" + } + } + ] + } + }, + { + "type": "user", + "parent_tool_use_id": null, + "message": { + "content": [ + { + "type": "tool_result", + "tool_use_id": "toolu_017pVuU1H9ruNJNAKZxn1ZXZ", + "content": "Navigated to file:///home/runner/.cache/gstack-paid-shard-iFDuSx/tmp/skill-e2e-skillify-prototype-JLM2yo/fixture.html (200)" + } + ] + } + } + ] + }, + { + "trial": 3, + "source": "paid-slice-34-a1/skill-e2e-skillify--scrape-prototype-path.t3", + "cwd": "/home/runner/.cache/gstack-paid-shard-Pwqhyx/tmp/skill-e2e-skillify-prototype-dwxDIM", + "events": [ + { + "type": "assistant", + "parent_tool_use_id": null, + "message": { + "content": [ + { + "type": "tool_use", + "id": "toolu_012iUcATxHXfCfkxEqungvKZ", + "name": "Bash", + "input": { + "command": "cd /home/runner/.cache/gstack-paid-shard-Pwqhyx/tmp/skill-e2e-skillify-prototype-dwxDIM\nexport GSTACK_HOME=\"$PWD/.gstack-home\"; B=/github/home/.claude/skills/gstack/browse/dist/browse\n$B goto file://$PWD/fixture.html; $B html" + } + } + ] + } + }, + { + "type": "user", + "parent_tool_use_id": null, + "message": { + "content": [ + { + "type": "tool_result", + "tool_use_id": "toolu_012iUcATxHXfCfkxEqungvKZ", + "content": "Navigated to file:///home/runner/.cache/gstack-paid-shard-Pwqhyx/tmp/skill-e2e-skillify-prototype-dwxDIM/fixture.html (200)" + } + ] + } + } + ] + } + ] +} \ No newline at end of file diff --git a/test/helpers/auq-sdk-capture.ts b/test/helpers/auq-sdk-capture.ts index 7f941ffbb..b7bf26b1e 100644 --- a/test/helpers/auq-sdk-capture.ts +++ b/test/helpers/auq-sdk-capture.ts @@ -112,6 +112,8 @@ export function setupPlanCeoDir(opts: { run('git', ['init', '-b', 'main']); run('git', ['config', 'user.email', 'test@test.com']); run('git', ['config', 'user.name', 'Test']); + run('git', ['config', 'maintenance.auto', 'false']); + run('git', ['config', 'gc.auto', '0']); fs.writeFileSync( path.join(dir, 'plan.md'), [ diff --git a/test/helpers/free-fixtures-data.ts b/test/helpers/free-fixtures-data.ts index 4bf2ee84f..3f78417b2 100644 --- a/test/helpers/free-fixtures-data.ts +++ b/test/helpers/free-fixtures-data.ts @@ -216,6 +216,7 @@ export const FREE_FIXTURES: Readonly> = { 'test/fixtures/review-design-lite-reports-ci-36633323521.json': ['test/review-finalization-budget.test.ts'], 'test/fixtures/review-gate-integrity-opus-reports.json': ['test/gate-integrity-templates.test.ts'], 'test/fixtures/review-wrong-tree/**': ['test/review-dispatch.test.ts'], + 'test/fixtures/scrape-prototype-goto-42571575838.json': ['test/helpers/session-runner.test.ts'], 'test/fixtures/screen-verdict-41662815506/**': ['crew/test/ship-queue-screen-verdict.test.ts'], 'test/fixtures/sealed-guard-off-golden.json': ['test/sealed-guard.test.ts'], 'test/fixtures/session-update-heartbeat.sh': ['test/session-update-heartbeat.test.ts'], diff --git a/test/helpers/session-runner.test.ts b/test/helpers/session-runner.test.ts index c91130337..2c67252e6 100644 --- a/test/helpers/session-runner.test.ts +++ b/test/helpers/session-runner.test.ts @@ -4,7 +4,8 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { pathToFileURL } from 'node:url'; -import { parseNDJSON } from './session-runner'; +import { browseNavigatedTo, parseNDJSON } from './session-runner'; +import captured_scrape_goto from '../fixtures/scrape-prototype-goto-42571575838.json'; test('runSkillTest launches a child without operator credentials', () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-hermetic-session-')); @@ -256,3 +257,29 @@ describe('parseNDJSON', () => { expect(parsed.turnCount).toBe(2); }); }); + +describe('browseNavigatedTo', () => { + const trials = captured_scrape_goto.trials.map(trial => ({ + url: `file://${trial.cwd}/fixture.html`, + calls: parseNDJSON(trial.events.map(event => JSON.stringify(event))).toolCalls, + })); + const bash = (command: string, output: string) => [{ tool: 'Bash', input: { command }, output }]; + + test('credits every captured scrape-prototype-path goto file://$PWD/fixture.html (run 42571575838)', () => { + expect(trials).toHaveLength(3); + for (const { url, calls } of trials) { + expect(calls.some(call => String(call.input.command).includes(url))).toBe(false); + expect(browseNavigatedTo(calls, url)).toBe(true); + } + }); + + test('a different file, an echoed URL, another path or a failed navigation is not credited', () => { + const { url, calls } = trials[0]!; + expect(browseNavigatedTo(calls, url.replace('fixture.html', 'other.html'))).toBe(false); + expect(browseNavigatedTo(bash(`echo "${url}"`, url), url)).toBe(false); + expect(browseNavigatedTo(bash(`echo "Navigated to ${url} (200)"`, `Navigated to ${url} (200)`), url)).toBe(false); + expect(browseNavigatedTo(bash('$B goto file:///tmp/elsewhere/fixture.html', 'Navigated to file:///tmp/elsewhere/fixture.html (200)'), url)).toBe(false); + expect(browseNavigatedTo(bash(`$B goto ${url}`, `Error: page.goto: net::ERR_FILE_NOT_FOUND at ${url}`), url)).toBe(false); + expect(browseNavigatedTo(bash(`$B goto ${url}`, `Navigated to ${url} (404)`), url)).toBe(false); + }); +}); diff --git a/test/helpers/session-runner.ts b/test/helpers/session-runner.ts index c73b2596d..0d5f36f91 100644 --- a/test/helpers/session-runner.ts +++ b/test/helpers/session-runner.ts @@ -92,6 +92,15 @@ export function scanBrowseErrors(text: string): string[] { }); } +/** True when a Bash `goto` printed browse's success line for exactly this URL with a 2xx status. */ +export function browseNavigatedTo(toolCalls: Array<{ tool: string; input: any; output: string }>, url: string): boolean { + return toolCalls.some(call => call.tool === 'Bash' && /\bgoto\b/.test(String(call.input?.command ?? '')) + && String(call.output ?? '').split('\n').some(line => { + const match = line.trim().match(/^Navigated to (\S+) \((\d+)\)$/); + return match?.[1] === url && match[2]!.startsWith('2'); + })); +} + // --- Testable NDJSON parser --- export interface ParsedNDJSON { diff --git a/test/helpers/skill-fixture.ts b/test/helpers/skill-fixture.ts index ce7609eac..9fdd82f48 100644 --- a/test/helpers/skill-fixture.ts +++ b/test/helpers/skill-fixture.ts @@ -316,3 +316,36 @@ export function extractDesignResearchContract(skill: string): string { 'For each Aside research call, include this prelude before invoking `_aside_exec` with the requested query:', '```bash', prelude, '```'].join('\n\n'); } + +/** + * /codex review E2E excerpt: the skeleton's intro and setup steps plus the + * review-mode section, skipping the Section index and STOP pointers whose + * install paths don't exist in a temp fixture. The fixture seeds no install, so + * every ~/.claude/skills/gstack/ path (bin/ helpers and lib/ scripts alike) + * points at this checkout, as review-design-lite does. + */ +export function extractCodexReviewExcerpt(root: string): string { + const full = fs.readFileSync(path.join(root, 'codex', 'SKILL.md'), 'utf-8'); + const introStart = full.indexOf('# /codex — Multi-AI Second Opinion'); + const introEnd = full.indexOf('## Section index', introStart); + const stepsStart = full.indexOf('## Step 0.4', introStart); + const stepsEnd = full.indexOf('> **STOP.**', stepsStart); + if (!(introStart > -1 && introEnd > introStart && stepsStart > introEnd && stepsEnd > stepsStart)) { + throw new Error('skill fixture: codex/SKILL.md review excerpt markers moved'); + } + const reviewMode = fs.readFileSync(path.join(root, 'codex', 'sections', 'review-mode.md'), 'utf-8'); + if (!reviewMode.includes('## Step 2A: Review Mode')) throw new Error('skill fixture: codex review-mode section missing'); + return [full.slice(introStart, introEnd), full.slice(stepsStart, stepsEnd), reviewMode].join('\n') + .replaceAll('~/.claude/skills/gstack/', `${root}/`); +} + +/** + * Every helper path in an excerpt that a fixture with no install cannot run: any path under an installed + * .claude/skills/gstack/ (a Capy machine or laptop may have one, a CI runner does not), and any gstack bin/ helper or + * lib/*.ts script that does not resolve to a file (bare bin/ and lib/ resolve under root). + */ +export function unresolvedBinRefs(excerpt: string, root: string): string[] { + const refs = new Set(excerpt.match(/[^\s`'"()=]*(?:\.claude\/skills\/gstack\/[^\s`'"()]*|\b(?:bin\/gstack-[\w.-]*\w|lib\/[\w./-]*\.ts)\b)/g) ?? []); + return [...refs].filter(ref => ref.includes('.claude/skills/gstack/') + || !fs.existsSync(/^(?:bin|lib)\//.test(ref) ? path.join(root, ref) : ref)); +} diff --git a/test/helpers/test-value-fixture.ts b/test/helpers/test-value-fixture.ts index 719d9bdd7..4b38afcf5 100644 --- a/test/helpers/test-value-fixture.ts +++ b/test/helpers/test-value-fixture.ts @@ -12,6 +12,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { spawnSync } from 'node:child_process'; +import { createCoverageAuditFixture } from '../fixtures/coverage-audit-fixture'; export const LOW_VALUE_TESTS = { sourceGrep: 'test/pricing-source.test.ts', @@ -128,3 +129,47 @@ export function recordsNonTestCallerSearch(finding: { evidence?: unknown }): boo const evidence = JSON.stringify(finding.evidence ?? ''); return /non_test_callers/.test(evidence) && /\bgit grep\b|\brg\b|\bgrep\b|\bGrep\b|\bag\b|\back\b/.test(evidence); } + +const REFUND_SMOKE_TEST = ` +import { refundPayment } from '../src/billing'; +test('refundPayment does not throw', () => { + expect(() => refundPayment('pay_1', 'duplicate')).not.toThrow(); +}); +`; + +/** ship-coverage-value: the coverage-audit billing fixture plus a ★-only refundPayment smoke test. */ +export function createShipCoverageValueFixture(dir: string): void { + createCoverageAuditFixture(dir); + fs.appendFileSync(path.join(dir, 'test/billing.test.ts'), REFUND_SMOKE_TEST); +} + +/** The header of the innermost function or test/it/describe call whose braces span a `file:line` reference under cwd, or null. */ +export function enclosingDeclaration(cwd: string, ref: unknown): string | null { + const match = String(ref ?? '').match(/^(.+?):(\d+)(?:-\d+)?$/); + if (!match || !fs.existsSync(path.resolve(cwd, match[1]!))) return null; + const lines = fs.readFileSync(path.resolve(cwd, match[1]!), 'utf8').split('\n'); + const target = Number(match[2]) - 1; + for (let start = Math.min(target, lines.length - 1); start >= 0; start--) { + if (!/^\s*(?:export\s+)?(?:async\s+)?function\s+\w+|^\s*(?:test|it|describe)\s*\(/.test(lines[start]!)) continue; + let depth = 0; + for (let end = start; end < lines.length; end++) { + depth += (lines[end]!.match(/\{/g) ?? []).length - (lines[end]!.match(/\}/g) ?? []).length; + if (depth > 0) continue; + if (target <= end) return lines[start]!.trim(); + break; + } + } + return null; +} + +/** + * A weak_gaps row for symbol: named in the row, or a star_one row whose path:line + * lies inside the symbol's function and whose existing_test:line lies inside a test + * naming it. The Step 7 schema has no function-name field, so a file:line row counts. + */ +export function weakGapCovers(gap: any, cwd: string, symbol: RegExp): boolean { + if (symbol.test(JSON.stringify(gap))) return true; + return gap?.reason === 'star_one' + && symbol.test(enclosingDeclaration(cwd, gap.path) ?? '') + && symbol.test(enclosingDeclaration(cwd, gap.existing_test) ?? ''); +} diff --git a/test/helpers/third-party-actions.ts b/test/helpers/third-party-actions.ts index 0eccf99d3..8220e897c 100644 --- a/test/helpers/third-party-actions.ts +++ b/test/helpers/third-party-actions.ts @@ -55,11 +55,23 @@ export const offersDriveOption = (text: string): boolean => letteredOptions(text * mark this step as blocked on you."), never as a drive option's "I wait while you sign in": the phrase must open the * option or a sentence and end it, and the option must not be a browser drive, so "I wait for now while you sign in", * "wait for now and do it yourself" and "I drive it in Aside. I wait for now." do not count. + * + * The contract's own defer outcome also counts, whatever the option calls it: an option that is neither a browser + * drive nor manual steps and says the step is marked, left or kept as blocked or waiting on the user, with that clause + * ending at "on you" (gate 42431616220: "D) Do it later. I'll mark this step as waiting on you."). A clause starts + * at the option start, after sentence punctuation, after an en or em dash, or after a spaced hyphen; a mid-word hyphen + * ("re-mark") starts none. A negation, refusal or condition anywhere in the clause before that verb or the + * stays/remains/is predicate never counts, so "I will not mark this step as blocked on you", "I won't mark this step as + * waiting on you", "I refuse to mark this step as blocked on you", "otherwise I'll mark this step as blocked on you", + * "if you skip it, I'll mark this step as blocked on you", "the step is not blocked on you" and "nothing stays blocked + * on you" do not count, and neither does "waiting on you to approve it". */ +const DEFER_OUTCOME = /(?:^|[.!?;:]\s+|\s-+\s+|[\u2013\u2014]\s*)(?:(?!\b(?:not|never|no|nothing|nobody|none|neither|nor|without|cannot|refus\w*|declin\w*|avoid\w*|rather|instead|if|unless|otherwise|else)\b|n['\u2019]t\b)[^.!?;:\u2013\u2014])*?\b(?:(?:I|we)(?:['\u2019]ll| will) |I |we )?(?:(?:mark|leave|keep)\w* (?:this|it|the)(?: [\w-]+)?(?: step)? (?:as )?|(?:stays|remains|is) )(?:blocked|waiting) on you(?=\s*(?:[.!?;:]|[-\u2013\u2014]|$))/i; export const offersDefer = (text: string): boolean => letteredOptions(text).some(option => /\b(?:defer\w*|postpone\w*|put (?:this|it)(?: step)? off|skip (?:this|it)(?: step)? for now|come back to (?:this|it)(?: step)? later|leave (?:this|it)(?: step)? for (?:now|later))\b/i.test(option) || !/drive|browse|Aside/i.test(option) - && /(?:^|[.!?;:]\s+)(?:(?:we|I|let['\u2019]s) )?(?:wait|hold off)(?: on (?:this|it)(?: step)?)? for now\b(?=\s*(?:[.!?;:]|[-\u2013\u2014]|$))/i.test(option)); + && (/(?:^|[.!?;:]\s+)(?:(?:we|I|let['\u2019]s) )?(?:wait|hold off)(?: on (?:this|it)(?: step)?)? for now\b(?=\s*(?:[.!?;:]|[-\u2013\u2014]|$))/i.test(option) + || !offersManualSteps(option) && DEFER_OUTCOME.test(option))); /** The manual option: "manual", or one sentence offering instructions or steps the user follows themselves. */ export function offersManualSteps(text: string): boolean { diff --git a/test/skill-e2e-skillify.test.ts b/test/skill-e2e-skillify.test.ts index fd9c262ed..66b46b5d8 100644 --- a/test/skill-e2e-skillify.test.ts +++ b/test/skill-e2e-skillify.test.ts @@ -30,7 +30,7 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { JUDGE_MS, CAPTURE_MS, CAPTURE_LONG_MS } from './helpers/eval-budgets'; -import { runSkillTest } from './helpers/session-runner'; +import { browseNavigatedTo, runSkillTest } from './helpers/session-runner'; import { ROOT, browseBin, runId, describeIfSelected, testConcurrentIfSelected, @@ -277,7 +277,7 @@ This run is non-interactive; AskUserQuestion is unavailable.`, logCost('scrape-prototype-path', result); const cmds = bashCommands(result); - const wentToFixture = cmds.some(c => c.includes(fileUrl)); + const wentToFixture = browseNavigatedTo(result.toolCalls, fileUrl); const fetchedHtml = cmds.some(c => /\bgoto\b|\bhtml\b|\btext\b/.test(c)); const surface = fullSurface(result); const mentionsSkillify = /skillify/i.test(surface); diff --git a/test/skill-e2e-test-value.test.ts b/test/skill-e2e-test-value.test.ts index af6d1b3a0..664d66b79 100644 --- a/test/skill-e2e-test-value.test.ts +++ b/test/skill-e2e-test-value.test.ts @@ -16,8 +16,7 @@ import { runSkillTest } from './helpers/session-runner'; import { ROOT, runId, describeIfSelected, testIfSelected, copyDirSync, logCost, createEvalCollector, finalizeEvalCollector } from './helpers/e2e-helpers'; import { extractSkillBody } from './helpers/skill-fixture'; -import { createCoverageAuditFixture } from './fixtures/coverage-audit-fixture'; -import { createTestValueFixture, lastJsonLine, jsonFindings, LOW_VALUE_TESTS, recordsNonTestCallerSearch } from './helpers/test-value-fixture'; +import { createShipCoverageValueFixture, createTestValueFixture, lastJsonLine, weakGapCovers, jsonFindings, LOW_VALUE_TESTS, recordsNonTestCallerSearch } from './helpers/test-value-fixture'; import { runRecordedOfficeHoursAttempt, OFFICE_HOURS_BUN_GRACE_MS } from './helpers/office-hours-attempt'; import { resolveEvalModel } from '../lib/eval-model'; import { RETIREMENT_FIELDS } from '../scripts/resolvers/test-value'; @@ -30,13 +29,6 @@ function trackedChanges(cwd: string): string { return spawnSync('git', ['status', '--porcelain', '--untracked-files=no'], { cwd, encoding: 'utf8', timeout: 10_000 }).stdout.trim(); } -const SMOKE_TEST = ` -import { refundPayment } from '../src/billing'; -test('refundPayment does not throw', () => { - expect(() => refundPayment('pay_1', 'duplicate')).not.toThrow(); -}); -`; - type Case = { id: string; skill: string; @@ -49,10 +41,7 @@ type Case = { const CASES: Case[] = [ { id: 'ship-coverage-value', skill: 'ship', suite: 'Ship Test Value E2E', - setup: cwd => { - createCoverageAuditFixture(cwd); - fs.appendFileSync(path.join(cwd, 'test/billing.test.ts'), SMOKE_TEST); - }, + setup: createShipCoverageValueFixture, prompt: cwd => `Read ship/SKILL.md and ship/sections/test-coverage.md for the current ship workflow. You are on the feature/billing branch. The base branch is main. There is no remote. @@ -61,13 +50,13 @@ directly to ${cwd}/src/billing.ts and its tests in ${cwd}/test/billing.test.ts ( targeted audit with no branch diff). Generation: audit-only; passes used: 0 of 2. Do not dispatch subagents, write tests or modify files. Skip every other step. End with the audit's LAST-line JSON exactly as Step 7 specifies.`, - validate: result => { + validate: (result, cwd) => { const json = lastJsonLine(result.output || ''); if (!json) throw new Error('ship coverage: no last-line JSON'); if (typeof json.coverage_pct !== 'number' || typeof json.coverage_pct_value !== 'number') throw new Error(`ship coverage: numeric coverage_pct and coverage_pct_value required, got ${JSON.stringify(json)}`); if (json.coverage_pct < json.coverage_pct_value) throw new Error(`ship coverage: coverage_pct ${json.coverage_pct} < coverage_pct_value ${json.coverage_pct_value}`); const weak = Array.isArray(json.weak_gaps) ? json.weak_gaps : []; - if (!weak.some((gap: any) => /refund/i.test(JSON.stringify(gap)))) throw new Error(`ship coverage: the ★-only refundPayment path must be in weak_gaps, got ${JSON.stringify(json.weak_gaps)}`); + if (!weak.some((gap: any) => weakGapCovers(gap, cwd, /refund/i))) throw new Error(`ship coverage: the ★-only refundPayment path must be in weak_gaps, got ${JSON.stringify(json.weak_gaps)}`); }, }, { diff --git a/test/skill-e2e-workflow.test.ts b/test/skill-e2e-workflow.test.ts index 8d566469d..da602a27c 100644 --- a/test/skill-e2e-workflow.test.ts +++ b/test/skill-e2e-workflow.test.ts @@ -13,7 +13,7 @@ import { randomUUID } from 'node:crypto'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; -import { extractSkillBody } from './helpers/skill-fixture'; +import { extractCodexReviewExcerpt, extractSkillBody } from './helpers/skill-fixture'; import { createCoverageAuditFixture } from './fixtures/coverage-audit-fixture'; import { claudeOutsideExecutions, codexReviewVerdicts } from './helpers/outside-voice-evidence'; import { validateCoverageAudit, type CoverageFile } from './helpers/coverage-audit'; @@ -413,32 +413,7 @@ describeIfSelected('Codex skill E2E', ['codex-review'], () => { run('git', ['add', 'user_controller.rb']); run('git', ['commit', '-m', 'add vulnerable controller']); - // Extract only the review-relevant content (CLAUDE.md: "extract, don't copy"). - // The codex skill is carved (T9): the skeleton carries setup + dispatch and - // STOP-points to codex/sections/*-mode.md. Build the fixture from the - // skeleton's setup slices plus the review-mode section body, SKIPPING the - // Section index and STOP pointers — their install paths don't exist in this - // temp fixture dir and would burn agent turns on failed Reads. - const full = fs.readFileSync(path.join(ROOT, 'codex', 'SKILL.md'), 'utf-8'); - const introStart = full.indexOf('# /codex — Multi-AI Second Opinion'); - const introEnd = full.indexOf('## Section index', introStart); - const stepsStart = full.indexOf('## Step 0.4', introStart); - const stepsEnd = full.indexOf('> **STOP.**', stepsStart); - expect(introStart).toBeGreaterThan(-1); - expect(introEnd).toBeGreaterThan(introStart); - expect(stepsStart).toBeGreaterThan(introEnd); - expect(stepsEnd).toBeGreaterThan(stepsStart); - const reviewMode = fs.readFileSync( - path.join(ROOT, 'codex', 'sections', 'review-mode.md'), - 'utf-8', - ); - expect(reviewMode).toContain('## Step 2A: Review Mode'); // non-empty, right section - const reviewSection = [ - full.slice(introStart, introEnd), - full.slice(stepsStart, stepsEnd), - reviewMode, - ].join('\n'); - fs.writeFileSync(path.join(codexDir, 'codex-SKILL.md'), reviewSection); + fs.writeFileSync(path.join(codexDir, 'codex-SKILL.md'), extractCodexReviewExcerpt(ROOT)); }); afterAll(() => { diff --git a/test/skill-fixture.test.ts b/test/skill-fixture.test.ts index 6308f4eae..f05ca0729 100644 --- a/test/skill-fixture.test.ts +++ b/test/skill-fixture.test.ts @@ -26,6 +26,8 @@ import { REVIEW_ARMY_E2E_SECTIONS, RETRO_E2E_SECTIONS, CODEX_REVIEW_E2E_SECTIONS, + extractCodexReviewExcerpt, + unresolvedBinRefs, } from './helpers/skill-fixture'; import { E2E_TOUCHFILES, GLOBAL_TOUCHFILES, selectTests } from './helpers/touchfiles'; import { readShippedSkillRouting } from './helpers/shipped-skill-routing'; @@ -362,6 +364,32 @@ describe('real-skill pins: section lists used by E2E fixtures', () => { expect(out).toContain('## Step 4: Critical pass (core review)'); expect(out).not.toContain('## Telemetry (run last)'); }); + + // Run 42571575838: the excerpt kept ~/.claude/skills/gstack/bin/gstack-codex-probe, + // the fixture seeds no install, and the review failed closed on HELPER_UNAVAILABLE. + test('every bin/ helper the codex-review excerpt names resolves in this checkout', () => { + const excerpt = extractCodexReviewExcerpt(ROOT); + expect(excerpt).toContain('## Step 2A: Review Mode'); + expect(excerpt).toContain(`_CODEX_PROBE=${path.join(ROOT, 'bin', 'gstack-codex-probe')}`); + expect(excerpt).toContain(`${path.join(ROOT, 'lib', 'outside-review-result.ts')} --label 'Codex review'`); + expect(excerpt).not.toContain('.claude/skills/gstack/'); + expect(unresolvedBinRefs(excerpt, ROOT)).toEqual([]); + }); + + test('an install-path or missing helper stays unresolved', () => { + expect(unresolvedBinRefs('_CODEX_PROBE=~/.claude/skills/gstack/bin/gstack-codex-probe', ROOT)) + .toEqual(['~/.claude/skills/gstack/bin/gstack-codex-probe']); + // /ship adversarial pass on #4223: the bin/-only rewrite left this lib/ call on the install path. + expect(unresolvedBinRefs("bun ~/.claude/skills/gstack/lib/outside-review-result.ts --label 'Codex review'", ROOT)) + .toEqual(['~/.claude/skills/gstack/lib/outside-review-result.ts']); + expect(unresolvedBinRefs('a global `~/.claude/skills/gstack/` install', ROOT)).toEqual(['~/.claude/skills/gstack/']); + expect(unresolvedBinRefs(`bun ${path.join(ROOT, 'lib', 'outside-review-result.ts')}`, ROOT)).toEqual([]); + expect(unresolvedBinRefs(`bun ${path.join(ROOT, 'lib', 'no-such-script.ts')}`, ROOT)) + .toEqual([path.join(ROOT, 'lib', 'no-such-script.ts')]); + const missing = path.join(ROOT, 'bin', 'gstack-no-such-helper'); + expect(unresolvedBinRefs(`X=$(${missing} probe)`, ROOT)).toEqual([missing]); + expect(unresolvedBinRefs('see `bin/gstack-no-such-helper`.', ROOT)).toEqual(['bin/gstack-no-such-helper']); + }); }); describe('real-skill pins: body/head extraction used by E2E fixtures', () => { diff --git a/test/test-value-bar.test.ts b/test/test-value-bar.test.ts index d4bdefc54..fd8226453 100644 --- a/test/test-value-bar.test.ts +++ b/test/test-value-bar.test.ts @@ -3,7 +3,7 @@ * sweeps tests. These checks prove the prompts carry the contract; the paid * cases in test/skill-e2e-test-value.test.ts carry the behavior claim. */ -import { describe, expect, test } from 'bun:test'; +import { afterAll, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -16,7 +16,7 @@ import { NO_CLAIM_EXAMPLES, VALUE_CARD_FIELDS, VALUE_CARD_OPTIONAL_FIELD, WEAK_REASONS, clampCardField, generateTestValueBar, renderValueCard, type TestValueBarMode, } from '../scripts/resolvers/test-value'; -import { recordsNonTestCallerSearch } from './helpers/test-value-fixture'; +import { createShipCoverageValueFixture, recordsNonTestCallerSearch, weakGapCovers } from './helpers/test-value-fixture'; import { expectMentions } from './helpers/prompt-structure'; const ROOT = path.resolve(import.meta.dir, '..'); @@ -297,3 +297,28 @@ describe('docs and registration', () => { }); }); } + +describe('ship-coverage-value weak_gaps grader', () => { + // Run 42571575838, codex-sol gate arm, t1: the ★-only refund path as a bare file:line row. + const captured = { path: 'src/billing.ts:11', existing_test: 'test/billing.test.ts:16', reason: 'star_one' }; + const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'ship-coverage-value-')); + createShipCoverageValueFixture(cwd); + const covers = (gap: unknown) => weakGapCovers(gap, cwd, /refund/i); + + test('a star_one row inside refundPayment with its smoke test counts, as does a row naming refund', () => { + expect(/refund/i.test(JSON.stringify(captured))).toBe(false); + expect(covers(captured)).toBe(true); + expect(covers({ path: 'src/billing.ts:refundPayment', existing_test: 'test/billing.test.ts', reason: 'star_one' })).toBe(true); + }); + + test('processPayment rows, other reasons, empty rows and the processPayment test do not count', () => { + expect(covers({ ...captured, path: 'src/billing.ts:4' })).toBe(false); + expect(covers({ ...captured, reason: 'gate_failed' })).toBe(false); + expect([].some(covers)).toBe(false); + expect(covers({})).toBe(false); + expect(covers({ ...captured, existing_test: 'test/billing.test.ts:6' })).toBe(false); + expect(covers({ ...captured, path: 'src/billing.ts:40' })).toBe(false); + }); + + afterAll(() => fs.rmSync(cwd, { recursive: true, force: true })); +}); diff --git a/test/third-party-actions.test.ts b/test/third-party-actions.test.ts index dcdc06289..c597fe4f1 100644 --- a/test/third-party-actions.test.ts +++ b/test/third-party-actions.test.ts @@ -574,6 +574,34 @@ describe('Opus consent renders (gate captures)', () => { expect(offersDefer(option), option).toBe(true); }); + test('the defer outcome counts in any words, and a refusal of it never does', () => { + // Gate 42431616220 slice 15: the red tpa-present render's D) option, missed by the spelling list. + for (const option of ["D) Do it later. I'll mark this step as waiting on you.", 'D) Not yet: the step stays blocked on you.', + 'D) Later. This step remains waiting on you.', "D) Do it later \u2014 I'll leave this step as blocked on you.", + "D) Do it later - I'll leave this step as blocked on you."]) + expect(offersDefer(option), option).toBe(true); + for (const text of ['D) Continue now. I will not mark this step as blocked on you.', + "D) Continue now. I won't mark this step as waiting on you.", 'D) Continue now. I never mark this step as blocked on you.', + 'D) Go ahead now. I cannot leave it waiting on you.', 'D) Continue now. The step is not blocked on you.', + 'D) Continue now; this step is no longer blocked on you.', 'D) Continue now, so nothing stays blocked on you.', + 'D) Continue now without leaving it waiting on you.', + 'D) Continue now. I refuse to mark this step as blocked on you.', + 'D) Continue now. I decline to leave it waiting on you.', + "D) Continue now. I'll avoid marking this step as blocked on you.", + 'D) Continue now rather than leave it waiting on you.', + "D) Continue now. Otherwise I'll mark this step as blocked on you.", + "D) Continue now. If you skip it, I'll mark this step as blocked on you.", + 'D) Continue now unless you want me to keep it waiting on you.', + "D) Continue now. I won't re-mark this step as blocked on you.", + "D) Do it later. I'll mark this step as waiting on you to approve it.", + 'A) I drive it in your Aside browser; the step stays blocked on you.', + "B) I drive it in gstack's own browser and mark this step as waiting on you.", + "C) Manual instructions; I'll mark this step as blocked on you.", + 'C) I give you the steps to do it yourself. The step stays blocked on you.', + "I'll mark this step as waiting on you.\nA) I drive it in your Aside browser\nB) Manual instructions"]) + expect(offersDefer(text), text).toBe(false); + }); + test('defer counts only as a lettered option', () => { const present = renders('tpa-present').find(render => render.run === '39157502890')!.assistantText; const withoutD = present.split('\n').filter(line => !/^\W*D\)/.test(line.replaceAll('**', ''))).join('\n');