From 78e4b770fa65cccc48b90871f5ad6dee417a6299 Mon Sep 17 00:00:00 2001
From: Garry Tan <garrytan@gmail.com>
Date: Thu, 30 Apr 2026 22:27:33 -0700
Subject: [PATCH] test(harness): fix detection order + whitespace-tolerant
 pattern matching
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two bugs surfaced when validating the v1.21 fix end-to-end:

1. PlanSkillObservation outcome detection ran 'asked' (any numbered
   options list) BEFORE 'plan_ready'. Plan-mode's "Ready to execute?"
   confirmation IS a numbered options list (1=auto, 2=manual, ...), so
   any skill that successfully reached the native confirmation got
   misclassified as 'asked'. Reorder: 'auto_decided' (most specific,
   requires AUTO_DECIDE annotation) > 'plan_ready' (next, requires the
   "ready to execute" stem) > 'asked' (any remaining numbered list).

2. isPlanReadyVisible and isAutoDecidedVisible regexes only matched
   spaced forms ("ready to execute", "(your preference)"). stripAnsi
   removes cursor-positioning escapes (`\x1b[40C`) entirely instead of
   replacing them with spaces, so the same text can render as
   "readytoexecute" or "(yourpreference)". Both detectors now test the
   spaced form first, fall through to a whitespace-collapsed comparison.
   Inline unit smoke confirms both forms match.

Updates to the 5 strict 'asked' regression test cases (plan-ceo,
plan-eng, plan-devex, autoplan, office-hours): with the detection order
corrected, the model's plan-file fallback flow legitimately lands at
'plan_ready' instead of 'asked'. Pass envelope expanded to ['asked',
'plan_ready'] (matching plan-design-review's existing pattern). Failure
signals tightened to include 'auto_decided' (catches AUTO_DECIDE without
opt-in) plus the standard silent_write/exited/timeout. plan-design was
already on this contract from v1.21's first commit, no change needed.

The expanded envelope is correct: under --disallowedTools AskUserQuestion
the Tool resolution preamble routes the question through plan-mode's
native "Ready to execute?" surface — the user still sees the decision,
just via the plan-file flow rather than a numbered prompt.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 test/helpers/claude-pty-runner.ts             | 58 +++++++++++++------
 test/skill-e2e-autoplan-auto-mode.test.ts     | 15 ++++-
 test/skill-e2e-office-hours-auto-mode.test.ts | 11 +++-
 test/skill-e2e-plan-ceo-plan-mode.test.ts     | 28 +++++++--
 test/skill-e2e-plan-devex-plan-mode.test.ts   | 15 +++--
 test/skill-e2e-plan-eng-plan-mode.test.ts     | 15 +++--
 6 files changed, 103 insertions(+), 39 deletions(-)

diff --git a/test/helpers/claude-pty-runner.ts b/test/helpers/claude-pty-runner.ts
index 3b56fb07..ac9c7f07 100644
--- a/test/helpers/claude-pty-runner.ts
+++ b/test/helpers/claude-pty-runner.ts
@@ -133,22 +133,34 @@ export function isTrustDialogVisible(visible: string): boolean {
   return visible.includes('trust this folder');
 }
 
-/** Detect plan-mode's native "ready to execute" confirmation. */
+/**
+ * Detect plan-mode's native "ready to execute" confirmation. Tests both the
+ * spaced and whitespace-collapsed forms because stripAnsi removes cursor-
+ * positioning escapes (e.g. `\x1b[40C`) that render visually as spaces but
+ * leave no character behind — so "ready to execute" can come through as
+ * "readytoexecute" depending on the rendering path.
+ */
 export function isPlanReadyVisible(visible: string): boolean {
-  return /ready to execute|Would you like to proceed/i.test(visible);
+  if (/ready to execute|Would you like to proceed/i.test(visible)) return true;
+  const collapsed = visible.replace(/\s+/g, '');
+  return /readytoexecute|Wouldyouliketoproceed/i.test(collapsed);
 }
 
 /**
  * Detect the AUTO_DECIDE preamble template firing. The model prints
  * "Auto-decided <summary> → <option> (your preference). Change with /plan-tune."
  * when it short-circuits an AskUserQuestion via the question-tuning resolver
- * (`scripts/resolvers/question-tuning.ts:26`). We detect any of those phrases
- * — the wording can drift slightly between model invocations, so each cue is
- * checked independently. The arrow + "(your preference)" combination is the
- * tightest signal.
+ * (`scripts/resolvers/question-tuning.ts:26`). The "Auto-decided ..." stem +
+ * "(your preference)" tail combination is the tightest signal. Whitespace-
+ * collapsed forms covered for the same TTY-rendering reason as
+ * isPlanReadyVisible.
  */
 export function isAutoDecidedVisible(visible: string): boolean {
-  return /Auto-decided\b/i.test(visible) && /\(your preference\)/i.test(visible);
+  const stemMatch =
+    /Auto-decided\b/i.test(visible) || /Auto-decided/i.test(visible.replace(/\s+/g, ''));
+  if (!stemMatch) return false;
+  if (/\(your preference\)/i.test(visible)) return true;
+  return /\(yourpreference\)/i.test(visible.replace(/\s+/g, ''));
 }
 
 /**
@@ -651,18 +663,18 @@ export async function runPlanSkillObservation(opts: {
           };
         }
       }
-      // Order: 'asked' first (rendered numbered list = user being asked),
-      // then 'auto_decided' (auto-decide text fired upstream of plan_ready
-      // — surfacing this distinguishes the auto-mode regression from a
-      // legitimate plan_ready outcome), then 'plan_ready'.
-      if (isNumberedOptionListVisible(visible)) {
-        return {
-          outcome: 'asked',
-          summary: 'skill fired a numbered-option prompt (AskUserQuestion or routing-injection)',
-          evidence: visible.slice(-2000),
-          elapsedMs: Date.now() - startedAt,
-        };
-      }
+      // Detection order is most-specific first:
+      //   1. 'auto_decided' — requires "Auto-decided" + "(your preference)";
+      //      the strongest signal that AUTO_DECIDE fired regardless of what
+      //      else is on screen.
+      //   2. 'plan_ready'   — "ready to execute" / "Would you like to proceed";
+      //      the plan-mode native confirmation. MUST be checked before
+      //      'asked' because the confirmation itself renders as a numbered
+      //      options list ("1. Yes, ... / 2. Manual ... / 3. ..."), and a
+      //      naive numbered-list check would mis-classify it as 'asked'.
+      //   3. 'asked'        — any numbered options list that wasn't already
+      //      classified as plan_ready. Real AskUserQuestion prompts AND
+      //      fallback-flow prose with numbered options both land here.
       if (isAutoDecidedVisible(visible)) {
         return {
           outcome: 'auto_decided',
@@ -679,6 +691,14 @@ export async function runPlanSkillObservation(opts: {
           elapsedMs: Date.now() - startedAt,
         };
       }
+      if (isNumberedOptionListVisible(visible)) {
+        return {
+          outcome: 'asked',
+          summary: 'skill fired a numbered-option prompt (AskUserQuestion or routing-injection)',
+          evidence: visible.slice(-2000),
+          elapsedMs: Date.now() - startedAt,
+        };
+      }
     }
 
     return {
diff --git a/test/skill-e2e-autoplan-auto-mode.test.ts b/test/skill-e2e-autoplan-auto-mode.test.ts
index ecf13852..d64e1f0b 100644
--- a/test/skill-e2e-autoplan-auto-mode.test.ts
+++ b/test/skill-e2e-autoplan-auto-mode.test.ts
@@ -27,6 +27,12 @@ const shouldRun = !!process.env.EVALS && process.env.EVALS_TIER === 'gate';
 const describeE2E = shouldRun ? describe : describe.skip;
 
 describeE2E('autoplan AskUserQuestion-blocked smoke (gate)', () => {
+  // Pass envelope is ['asked', 'plan_ready']: model either renders the
+  // first non-auto-decided gate (Phase 1 premise confirmation) as numbered
+  // prose or surfaces it through the plan file + ExitPlanMode flow.
+  // Autoplan auto-decides intermediate questions BY DESIGN; the failure
+  // signal we care about is the AUTO_DECIDE preamble firing on a gate it
+  // shouldn't (caught explicitly via the 'auto_decided' outcome).
   test('a non-auto-decided gate surfaces when AskUserQuestion is --disallowedTools', async () => {
     const obs = await runPlanSkillObservation({
       skillName: 'autoplan',
@@ -35,7 +41,12 @@ describeE2E('autoplan AskUserQuestion-blocked smoke (gate)', () => {
       timeoutMs: 300_000,
     });
 
-    if (obs.outcome !== 'asked') {
+    if (
+      obs.outcome === 'auto_decided' ||
+      obs.outcome === 'silent_write' ||
+      obs.outcome === 'exited' ||
+      obs.outcome === 'timeout'
+    ) {
       throw new Error(
         `autoplan AskUserQuestion-blocked regression: outcome=${obs.outcome}\n` +
           `summary: ${obs.summary}\n` +
@@ -43,6 +54,6 @@ describeE2E('autoplan AskUserQuestion-blocked smoke (gate)', () => {
           `--- evidence (last 2KB visible) ---\n${obs.evidence}`,
       );
     }
-    expect(obs.outcome).toEqual('asked');
+    expect(['asked', 'plan_ready']).toContain(obs.outcome);
   }, 360_000);
 });
diff --git a/test/skill-e2e-office-hours-auto-mode.test.ts b/test/skill-e2e-office-hours-auto-mode.test.ts
index 27ab4b03..b841d525 100644
--- a/test/skill-e2e-office-hours-auto-mode.test.ts
+++ b/test/skill-e2e-office-hours-auto-mode.test.ts
@@ -23,6 +23,8 @@ const shouldRun = !!process.env.EVALS && process.env.EVALS_TIER === 'gate';
 const describeE2E = shouldRun ? describe : describe.skip;
 
 describeE2E('office-hours AskUserQuestion-blocked smoke (gate)', () => {
+  // Pass envelope is ['asked', 'plan_ready']; failure signals are
+  // 'auto_decided' + silent_write/exited/timeout.
   test('AskUserQuestion surfaces when --disallowedTools AskUserQuestion is set', async () => {
     const obs = await runPlanSkillObservation({
       skillName: 'office-hours',
@@ -31,7 +33,12 @@ describeE2E('office-hours AskUserQuestion-blocked smoke (gate)', () => {
       timeoutMs: 300_000,
     });
 
-    if (obs.outcome !== 'asked') {
+    if (
+      obs.outcome === 'auto_decided' ||
+      obs.outcome === 'silent_write' ||
+      obs.outcome === 'exited' ||
+      obs.outcome === 'timeout'
+    ) {
       throw new Error(
         `office-hours AskUserQuestion-blocked regression: outcome=${obs.outcome}\n` +
           `summary: ${obs.summary}\n` +
@@ -39,6 +46,6 @@ describeE2E('office-hours AskUserQuestion-blocked smoke (gate)', () => {
           `--- evidence (last 2KB visible) ---\n${obs.evidence}`,
       );
     }
-    expect(obs.outcome).toEqual('asked');
+    expect(['asked', 'plan_ready']).toContain(obs.outcome);
   }, 360_000);
 });
diff --git a/test/skill-e2e-plan-ceo-plan-mode.test.ts b/test/skill-e2e-plan-ceo-plan-mode.test.ts
index 279d25ca..6e6e0d8f 100644
--- a/test/skill-e2e-plan-ceo-plan-mode.test.ts
+++ b/test/skill-e2e-plan-ceo-plan-mode.test.ts
@@ -50,10 +50,21 @@ describeE2E('plan-ceo-review plan-mode smoke (gate)', () => {
   // `--disallowedTools AskUserQuestion --permission-mode default` (verified
   // via `ps` on the live Conductor claude process). Native AskUserQuestion
   // is removed from the model's tool registry; without fallback guidance
-  // the model can't ask and silently proceeds. plan-ceo-review's Step 0
-  // always asks a scope-mode question, so 'asked' is the only pass — the
-  // fix must route through mcp__conductor__AskUserQuestion (when present)
-  // or the plan-file + ExitPlanMode flow.
+  // the model can't ask and silently proceeds.
+  //
+  // The fix (Tool resolution preamble) accepts two surface paths under
+  // --disallowedTools:
+  //   - 'asked'      — model emits a numbered-option prompt as prose (with
+  //                     the same D<N> + Pros/cons format as a real AUQ)
+  //   - 'plan_ready' — model writes the question into the plan file as a
+  //                     "## Decisions to confirm" section + ExitPlanMode;
+  //                     the native plan-mode "Ready to execute?" surfaces
+  //                     it through the TTY confirmation
+  //
+  // Both let the user see the decision. Failure signals are
+  // silent_write/exited/timeout (model never surfaced the question) and
+  // 'auto_decided' (the AUTO_DECIDE preamble fired without a /plan-tune
+  // opt-in — caught explicitly).
   test('AskUserQuestion surfaces when --disallowedTools AskUserQuestion is set', async () => {
     const obs = await runPlanSkillObservation({
       skillName: 'plan-ceo-review',
@@ -62,7 +73,12 @@ describeE2E('plan-ceo-review plan-mode smoke (gate)', () => {
       timeoutMs: 300_000,
     });
 
-    if (obs.outcome !== 'asked') {
+    if (
+      obs.outcome === 'auto_decided' ||
+      obs.outcome === 'silent_write' ||
+      obs.outcome === 'exited' ||
+      obs.outcome === 'timeout'
+    ) {
       throw new Error(
         `plan-ceo-review AskUserQuestion-blocked regression: outcome=${obs.outcome}\n` +
           `summary: ${obs.summary}\n` +
@@ -70,6 +86,6 @@ describeE2E('plan-ceo-review plan-mode smoke (gate)', () => {
           `--- evidence (last 2KB visible) ---\n${obs.evidence}`,
       );
     }
-    expect(obs.outcome).toEqual('asked');
+    expect(['asked', 'plan_ready']).toContain(obs.outcome);
   }, 360_000);
 });
diff --git a/test/skill-e2e-plan-devex-plan-mode.test.ts b/test/skill-e2e-plan-devex-plan-mode.test.ts
index 5b51f843..2ab1b2be 100644
--- a/test/skill-e2e-plan-devex-plan-mode.test.ts
+++ b/test/skill-e2e-plan-devex-plan-mode.test.ts
@@ -31,9 +31,9 @@ describeE2E('plan-devex-review plan-mode smoke (gate)', () => {
   }, 360_000);
 
   // v1.21+ regression: see skill-e2e-plan-ceo-plan-mode.test.ts for the
-  // contract. plan-devex-review's Step 0 always issues a persona-mode
-  // AskUserQuestion before scoring, so 'asked' is the only pass when
-  // AskUserQuestion is --disallowedTools.
+  // contract. Pass envelope is ['asked', 'plan_ready']; failure signals
+  // are 'auto_decided' (AUTO_DECIDE without opt-in) plus the standard
+  // silent_write/exited/timeout.
   test('AskUserQuestion surfaces when --disallowedTools AskUserQuestion is set', async () => {
     const obs = await runPlanSkillObservation({
       skillName: 'plan-devex-review',
@@ -42,7 +42,12 @@ describeE2E('plan-devex-review plan-mode smoke (gate)', () => {
       timeoutMs: 300_000,
     });
 
-    if (obs.outcome !== 'asked') {
+    if (
+      obs.outcome === 'auto_decided' ||
+      obs.outcome === 'silent_write' ||
+      obs.outcome === 'exited' ||
+      obs.outcome === 'timeout'
+    ) {
       throw new Error(
         `plan-devex-review AskUserQuestion-blocked regression: outcome=${obs.outcome}\n` +
           `summary: ${obs.summary}\n` +
@@ -50,6 +55,6 @@ describeE2E('plan-devex-review plan-mode smoke (gate)', () => {
           `--- evidence (last 2KB visible) ---\n${obs.evidence}`,
       );
     }
-    expect(obs.outcome).toEqual('asked');
+    expect(['asked', 'plan_ready']).toContain(obs.outcome);
   }, 360_000);
 });
diff --git a/test/skill-e2e-plan-eng-plan-mode.test.ts b/test/skill-e2e-plan-eng-plan-mode.test.ts
index 903aa824..35d6c918 100644
--- a/test/skill-e2e-plan-eng-plan-mode.test.ts
+++ b/test/skill-e2e-plan-eng-plan-mode.test.ts
@@ -31,9 +31,9 @@ describeE2E('plan-eng-review plan-mode smoke (gate)', () => {
   }, 360_000);
 
   // v1.21+ regression: see skill-e2e-plan-ceo-plan-mode.test.ts for the
-  // contract. plan-eng-review's Step 0 always issues a scope-challenge
-  // AskUserQuestion (and per-section STOPs after that), so 'asked' is the
-  // only pass when AskUserQuestion is --disallowedTools.
+  // contract. Pass envelope is ['asked', 'plan_ready']; failure signals
+  // are 'auto_decided' (AUTO_DECIDE without opt-in) plus the standard
+  // silent_write/exited/timeout.
   test('AskUserQuestion surfaces when --disallowedTools AskUserQuestion is set', async () => {
     const obs = await runPlanSkillObservation({
       skillName: 'plan-eng-review',
@@ -42,7 +42,12 @@ describeE2E('plan-eng-review plan-mode smoke (gate)', () => {
       timeoutMs: 300_000,
     });
 
-    if (obs.outcome !== 'asked') {
+    if (
+      obs.outcome === 'auto_decided' ||
+      obs.outcome === 'silent_write' ||
+      obs.outcome === 'exited' ||
+      obs.outcome === 'timeout'
+    ) {
       throw new Error(
         `plan-eng-review AskUserQuestion-blocked regression: outcome=${obs.outcome}\n` +
           `summary: ${obs.summary}\n` +
@@ -50,6 +55,6 @@ describeE2E('plan-eng-review plan-mode smoke (gate)', () => {
           `--- evidence (last 2KB visible) ---\n${obs.evidence}`,
       );
     }
-    expect(obs.outcome).toEqual('asked');
+    expect(['asked', 'plan_ready']).toContain(obs.outcome);
   }, 360_000);
 });