test(harness): fix detection order + whitespace-tolerant pattern matching

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>
This commit is contained in:
Garry Tan
2026-04-30 22:27:33 -07:00
parent a7cfbeba4c
commit 78e4b770fa
6 changed files with 103 additions and 39 deletions
+39 -19
View File
@@ -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 {