From 3d2dd4a33f6711128dd05f523536695712822572 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 12 Aug 2026 09:19:00 -0700 Subject: [PATCH] fix(pty-runner): active-render gate veto in the floor check + honest periodic-wiring docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex re-review P2s on the fix wave, both verified: - A finding AUQ rendering within TAIL_SCAN_BYTES of the gate (model waiting, no further output) was vetoed by the blanket tail exclusion until timeout. The veto is now ACTIVE-RENDER-aware: parseNumberedOptions anchors the last cursor menu, so only a pending GATE menu vetoes; the judge fallback shares the same check. Residual (documented): prose gate + prose finding inside one tail — floors run the native-menu path in practice. - The four demoted periodic tests are not in evals-periodic.yml's explicit matrix (a named instance of the pre-existing periodic-orphans TODO), so they run locally/manually until the PTY-capable periodic job lands. CHANGELOG claim softened accordingly; TODO filed with the wiring recipe. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 4 ++-- TODOS.md | 17 +++++++++++++ ...e-pty-runner.scope-gate-floor.unit.test.ts | 21 ++++++++++++++++ test/helpers/claude-pty-runner.ts | 24 +++++++++++++++---- 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 826d6ebe5..30a5ad203 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ Source: this branch's live PTY eval runs on 2026-08-11 (logs in ~/.gstack-dev/ev | Finding-floor runs with the gate excluded from the count | trivially satisfiable | 2/2 pass, gate renders don't count | | Stochastic smokes wrongly blocking the CI gate lane | 4 | 0 | -That last row is a repair: four plan-mode/finding-floor smokes were demoted to the weekly tier months ago, but the demotion never took effect — the test files still gated on the blocking lane. They now run where they were declared to run, and a new free invariant test makes that class of drift impossible to reintroduce silently. +That last row is a repair: four plan-mode/finding-floor smokes were demoted to the weekly tier months ago, but the demotion never took effect — the test files still gated on the blocking lane. They no longer block the gate lane; they run via `bun run test:periodic` (weekly-cron wiring for PTY tests is tracked in TODOS). A new free invariant test makes the declared-vs-actual tier drift impossible to reintroduce silently. ### What this means for you @@ -41,7 +41,7 @@ The plan → review → ship loop loses its most pointless click. Draft a plan, - The finding-floor harness no longer counts a scope-gate render toward its question floor (positional anchoring, judge-fallback exclusion) — the floor now genuinely measures finding-driven questions. ### Fixed -- Four stochastic plan-mode/finding-floor smokes declared `periodic` were still self-gating on the blocking `gate` tier — the demotion is now real, and the invariant test above prevents recurrence. +- Four stochastic plan-mode/finding-floor smokes declared `periodic` were still self-gating on the blocking `gate` tier — they no longer run in (or block) the gate lane, and the invariant test above prevents declared-vs-actual tier drift from recurring. Weekly-cron wiring for PTY-driven periodic tests is tracked in TODOS. - CI eval containers now register `plan-eng-review` and `plan-design-review` as discoverable skills (registration loops, dangling-target checks, and frontmatter verification all extended) — previously only two skills were registered. - The no-op regression suite covers all three plan-review skills outside plan mode, asserts the gate question actually rendered (unconditionally), and proves a pasted named target is consumed via cumulative-buffer token tracking. diff --git a/TODOS.md b/TODOS.md index 631ce234b..cddd7c9be 100644 --- a/TODOS.md +++ b/TODOS.md @@ -233,6 +233,23 @@ SKILL.md untouched). `bun test` is green again. ## Scope-gate follow-ups (filed via /plan-eng-review on the plan-mode auto-select-B change) +### P2: Wire the four demoted plan-mode/finding-floor PTY tests into periodic CI + +**What:** `evals-periodic.yml` runs an explicit 9-file matrix; the four tests +demoted to `periodic` in v1.62.0.0 (`skill-e2e-plan-eng-plan-mode`, +`skill-e2e-plan-design-plan-mode`, `skill-e2e-plan-eng-finding-floor`, +`skill-e2e-plan-design-finding-floor`) are not in it, so they currently run +only locally/manually (`bun run test:periodic` or `eval:bg:periodic`). Wiring +them needs a PTY-capable periodic job: the container skill-registration setup +from evals.yml's `e2e-pty-plan-smoke` job (real-file SKILL.md copies for the +TUI's cross-mount symlink bug) with `EVALS_TIER=periodic`. + +**Why:** Codex re-review P2 on the v1.62.0.0 ship. This is a named instance of +the existing periodic-orphans problem (see "P1/P2 periodic coverage" TODO in +Test infrastructure) — solve it there or here, once. + +**Depends on / blocked by:** none; sibling of the periodic-orphans TODO above. + ### P3: Extract the whole scope gate to a shared `{{SCOPE_GATE}}` resolver **What:** Move the duplicated scope-gate prose (heading, intro sentence, the diff --git a/test/helpers/claude-pty-runner.scope-gate-floor.unit.test.ts b/test/helpers/claude-pty-runner.scope-gate-floor.unit.test.ts index 66bf75d53..873846bb4 100644 --- a/test/helpers/claude-pty-runner.scope-gate-floor.unit.test.ts +++ b/test/helpers/claude-pty-runner.scope-gate-floor.unit.test.ts @@ -34,6 +34,7 @@ import { isPermissionDialogVisible, isScopeGateQuestionVisible, isScopeGateAutoSelectVisible, + parseNumberedOptions, } from './claude-pty-runner'; // The gate's native AskUserQuestion render (numbered options + cursor) — @@ -107,6 +108,26 @@ describe('floor-check scope-gate exclusion (acceptance-condition regression)', ( const tail = GATE_NATIVE_RENDER.slice(-TAIL_SCAN_BYTES); expect(isScopeGateQuestionVisible(tail)).toBe(true); }); + + test('active-render veto: a finding AUQ close after the gate is NOT vetoed (codex P2 re-review)', () => { + // The finding menu renders /current\s*branch\s*diff/i.test(o.label)); + expect(gateIsActiveRender).toBe(false); + }); + + test('active-render veto: the gate as the pending menu IS vetoed', () => { + const visible = 'booting…\n' + GATE_NATIVE_RENDER; + const activeMenu = parseNumberedOptions(visible); + expect(activeMenu.length).toBeGreaterThan(0); + const gateIsActiveRender = activeMenu.some((o) => /current\s*branch\s*diff/i.test(o.label)); + expect(gateIsActiveRender).toBe(true); + }); }); describe('isScopeGateAutoSelectVisible collapsed hyphen-less branch', () => { diff --git a/test/helpers/claude-pty-runner.ts b/test/helpers/claude-pty-runner.ts index dd44b9671..516840126 100644 --- a/test/helpers/claude-pty-runner.ts +++ b/test/helpers/claude-pty-runner.ts @@ -2260,14 +2260,27 @@ export async function runPlanSkillFloorCheck(opts: { // gate render has been seen, acceptance scans only the content APPENDED // after it (positional anchor above) — the buffer is append-only, so a // whole-buffer acceptance would keep matching the stale gate render - // forever. The tail exclusion additionally covers the window where the - // gate menu is still the active render. + // forever. + // + // The gate veto is ACTIVE-RENDER-aware, not blanket-tail: when a + // numbered menu is up, parseNumberedOptions anchors on the LAST cursor + // line, so we veto only when the pending menu IS the gate — a finding + // AUQ that renders within TAIL_SCAN_BYTES of the gate (model waiting, + // no further output) still satisfies the floor. Prose renders have no + // cursor anchor, so the prose path falls back to the tail check + // (accepted residual: prose gate + prose finding inside one tail can + // suppress until timeout; floors run the native-menu path in practice). const tail = visible.slice(-TAIL_SCAN_BYTES); const acceptWindow = gateSeenIdx === -1 ? visible : visible.slice(gateSeenIdx); + const activeMenu = parseNumberedOptions(visible); + const gateIsActiveRender = + activeMenu.length > 0 + ? activeMenu.some((o) => /current\s*branch\s*diff/i.test(o.label)) + : isScopeGateQuestionVisible(tail); if ( (isNumberedOptionListVisible(acceptWindow) || isProseAUQVisible(acceptWindow)) && !isPermissionDialogVisible(tail) && - !isScopeGateQuestionVisible(tail) + !gateIsActiveRender ) { return { auqObserved: true, @@ -2291,8 +2304,9 @@ export async function runPlanSkillFloorCheck(opts: { lastJudgeVerdict = judgePtyState(visible, { testName: opts.skillName }); // The judge can't tell a scope-gate question from a finding question, // so a 'waiting' verdict while the gate menu is the pending render - // must NOT satisfy the floor — same exclusion as the regex path. - if (lastJudgeVerdict.state === 'waiting' && !isScopeGateQuestionVisible(tail)) { + // must NOT satisfy the floor — same active-render exclusion as the + // regex path. + if (lastJudgeVerdict.state === 'waiting' && !gateIsActiveRender) { return { auqObserved: true, outcome: 'auq_observed',