mirror of https://github.com/garrytan/gstack.git
fix(pty-runner): positional floor exclusion, flag builder, outcome union, token tracking
Review-army + adversarial findings on the scope-gate observability work, all verified before fixing: - Floor check: acceptance scanned the CUMULATIVE buffer while the scope-gate exclusion scanned only the 1500-byte tail, so an early gate render satisfied the floor vacuously once ~1.5KB of output accumulated (found independently by 4 review passes; predicate reproduced). Acceptance now scans only content APPENDED after the first gate render (positional anchor), and the LLM-judge 'waiting' shortcut no longer fires while the gate menu is the pending render. - High-water flags are built once and spread at every return path — the hand-spread pattern had already drifted (judge-waiting return omitted two flags), which made must-stay-false asserts vacuous on those paths. - isScopeGateAutoSelectVisible: tense-tolerant selected/selecting/selects token (must-be-TRUE asserts shouldn't fail semantically-perfect paraphrases) and quoted-occurrence rejection (a model verbatim-quoting the announcement while declining must not trip must-stay-FALSE asserts). Fixtures added for both directions. - PlanSkillObservation outcome union gains 'wrote_findings_before_asking' (returned at runtime via classifyVisible but missing from the type). - trackTokens/tokensObserved: cumulative-buffer token high-water for consumption asserts (the 2KB evidence tail is lossy and the plan-file fallback is unreachable outside plan mode). - New scope-gate-floor unit pins (from the ship coverage audit): both gate render forms trip acceptance and exclusion; a genuine finding AUQ is not excluded; tail-scoping semantics pinned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
parent
57c53e7913
commit
4e61233021
|
|
@ -0,0 +1,122 @@
|
|||
/**
|
||||
* Scope-gate floor-exclusion regression pins (free, static).
|
||||
*
|
||||
* runPlanSkillFloorCheck's acceptance condition changed with the plan-mode
|
||||
* auto-select-B work: a render only satisfies the finding floor when
|
||||
*
|
||||
* (isNumberedOptionListVisible(visible) || isProseAUQVisible(visible))
|
||||
* && !isPermissionDialogVisible(tail)
|
||||
* && !isScopeGateQuestionVisible(tail) // <- new exclusion
|
||||
*
|
||||
* where tail = visible.slice(-TAIL_SCAN_BYTES). The composition lives inline
|
||||
* in the paid PTY loop, so these tests pin the load-bearing behavior of each
|
||||
* detector on the exact render shapes the floor passes them:
|
||||
*
|
||||
* 1. Both scope-gate render forms (native numbered UI, prose lettered
|
||||
* fallback) trip the acceptance detectors — WITHOUT the exclusion the
|
||||
* gate would trivially satisfy the floor inside the 3s pre-target
|
||||
* window. The exclusion must catch both forms.
|
||||
* 2. A genuine finding-driven AskUserQuestion must NOT trip the exclusion,
|
||||
* or the floor becomes unsatisfiable.
|
||||
* 3. The exclusion is TAIL-scoped by design: an early gate render that has
|
||||
* scrolled past TAIL_SCAN_BYTES must not suppress a later real finding
|
||||
* AskUserQuestion.
|
||||
*
|
||||
* Also closes the untested OR-branch of isScopeGateAutoSelectVisible: the
|
||||
* fully-collapsed hyphen-less 'autoselectedb' form.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import {
|
||||
TAIL_SCAN_BYTES,
|
||||
isNumberedOptionListVisible,
|
||||
isProseAUQVisible,
|
||||
isPermissionDialogVisible,
|
||||
isScopeGateQuestionVisible,
|
||||
isScopeGateAutoSelectVisible,
|
||||
} from './claude-pty-runner';
|
||||
|
||||
// The gate's native AskUserQuestion render (numbered options + cursor) —
|
||||
// what fires inside the floor check's 3s window before the seed arrives.
|
||||
const GATE_NATIVE_RENDER = `
|
||||
What should I review?
|
||||
|
||||
❯ 1. The current branch diff — the work in progress on this branch.
|
||||
2. A plan or design doc I'll paste or point you to.
|
||||
3. A specific file, directory, or path.
|
||||
`;
|
||||
|
||||
// The gate's prose fallback render (lettered options under --disallowedTools).
|
||||
const GATE_PROSE_RENDER = `
|
||||
What should I review?
|
||||
A) The current branch diff — the work in progress on this branch.
|
||||
B) A plan or design doc I'll paste or point you to.
|
||||
C) A specific file, directory, or path.
|
||||
Recommendation: A when a branch diff exists, otherwise B.
|
||||
`;
|
||||
|
||||
// A genuine finding-driven AskUserQuestion — the render the floor MEASURES.
|
||||
const FINDING_AUQ_RENDER = `
|
||||
Finding 1: the plan reimplements test sharding that Bun provides natively.
|
||||
|
||||
❯ 1. Use Bun's native --shard flag (recommended)
|
||||
2. Keep the custom scheduler as planned
|
||||
3. Defer this decision to implementation
|
||||
`;
|
||||
|
||||
describe('floor-check scope-gate exclusion (acceptance-condition regression)', () => {
|
||||
test('native gate render trips the acceptance detector — the exclusion is load-bearing', () => {
|
||||
// Pre-exclusion, this render satisfied the floor by itself.
|
||||
expect(isNumberedOptionListVisible(GATE_NATIVE_RENDER)).toBe(true);
|
||||
expect(isPermissionDialogVisible(GATE_NATIVE_RENDER)).toBe(false);
|
||||
// The new exclusion catches it.
|
||||
expect(isScopeGateQuestionVisible(GATE_NATIVE_RENDER)).toBe(true);
|
||||
});
|
||||
|
||||
test('prose gate render trips the prose-AUQ arm — the exclusion catches that form too', () => {
|
||||
expect(isProseAUQVisible(GATE_PROSE_RENDER)).toBe(true);
|
||||
expect(isPermissionDialogVisible(GATE_PROSE_RENDER)).toBe(false);
|
||||
expect(isScopeGateQuestionVisible(GATE_PROSE_RENDER)).toBe(true);
|
||||
});
|
||||
|
||||
test('a genuine finding AskUserQuestion is NOT excluded — the floor stays satisfiable', () => {
|
||||
expect(isNumberedOptionListVisible(FINDING_AUQ_RENDER)).toBe(true);
|
||||
expect(isPermissionDialogVisible(FINDING_AUQ_RENDER)).toBe(false);
|
||||
expect(isScopeGateQuestionVisible(FINDING_AUQ_RENDER)).toBe(false);
|
||||
});
|
||||
|
||||
test('tail-scoping: an early gate render scrolled out of the tail does not suppress a later finding AUQ', () => {
|
||||
// Gate render, then >TAIL_SCAN_BYTES of review output, then the real
|
||||
// finding AskUserQuestion — the shape the TAIL-scoped exclusion exists for.
|
||||
const filler = 'Reading the plan and auditing the design system.\n'.repeat(
|
||||
Math.ceil(TAIL_SCAN_BYTES / 48) + 4,
|
||||
);
|
||||
const visible = GATE_NATIVE_RENDER + filler + FINDING_AUQ_RENDER;
|
||||
const tail = visible.slice(-TAIL_SCAN_BYTES);
|
||||
|
||||
// Full buffer still remembers the gate (scrollback)…
|
||||
expect(isScopeGateQuestionVisible(visible)).toBe(true);
|
||||
// …but the floor's exclusion looks only at the tail, which is clean:
|
||||
expect(isScopeGateQuestionVisible(tail)).toBe(false);
|
||||
// and the acceptance arm (full-buffer scan) sees the finding AUQ.
|
||||
expect(isNumberedOptionListVisible(visible)).toBe(true);
|
||||
expect(isPermissionDialogVisible(tail)).toBe(false);
|
||||
});
|
||||
|
||||
test('a gate render inside the tail IS suppressed (no false floor pass)', () => {
|
||||
const tail = GATE_NATIVE_RENDER.slice(-TAIL_SCAN_BYTES);
|
||||
expect(isScopeGateQuestionVisible(tail)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isScopeGateAutoSelectVisible collapsed hyphen-less branch', () => {
|
||||
test("matches the fully-collapsed 'autoselectedb' form (hyphen lost in TTY reflow)", () => {
|
||||
const sample = 'Scopegate:planmode—autoselectedB(reviewingPLAN.md).';
|
||||
expect(isScopeGateAutoSelectVisible(sample)).toBe(true);
|
||||
});
|
||||
|
||||
test('hyphen-less token without the announcement prefix stays false', () => {
|
||||
const sample = 'The agent autoselectedB from the menu without announcing a scope gate decision.';
|
||||
expect(isScopeGateAutoSelectVisible(sample)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -644,15 +644,27 @@ export function isScopeGateQuestionVisible(visible: string): boolean {
|
|||
/**
|
||||
* True when the plan-mode auto-select announcement is rendered:
|
||||
* "Scope gate: plan mode — auto-selected B (reviewing <target>)."
|
||||
* Requires BOTH the announcement prefix and the selected-B token so
|
||||
* narration ("in plan mode I'd auto-select B") stays false.
|
||||
* Requires BOTH the announcement prefix and an auto-select-B token so
|
||||
* narration ("in plan mode I'd auto-select B") stays false. The token is
|
||||
* tense-tolerant (selected/selecting/selects) because the smokes assert
|
||||
* must-be-TRUE on it — a semantically-perfect paraphrase must not fail a
|
||||
* paid run — while the prefix stays exact so paraphrase narration without
|
||||
* the announcement frame stays false. A prefix immediately preceded by a
|
||||
* quote character is a QUOTATION (e.g. the model explaining why it is NOT
|
||||
* announcing), not a render — the announcement line itself never renders
|
||||
* quoted.
|
||||
*/
|
||||
export function isScopeGateAutoSelectVisible(visible: string): boolean {
|
||||
const squished = visible.replace(/\s+/g, '').toLowerCase();
|
||||
return (
|
||||
squished.includes('scopegate:planmode') &&
|
||||
(squished.includes('auto-selectedb') || squished.includes('autoselectedb'))
|
||||
);
|
||||
const QUOTES = ['"', "'", '`', '“', '‘'];
|
||||
const re = /scopegate:planmode/g;
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = re.exec(squished)) !== null) {
|
||||
const before = m.index > 0 ? squished[m.index - 1]! : '';
|
||||
if (QUOTES.includes(before)) continue; // quoted occurrence — narration, keep scanning
|
||||
if (/auto-?select(?:ed|ing|s)?b/.test(squished.slice(m.index))) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -1513,10 +1525,20 @@ export interface PlanSkillObservation {
|
|||
* "Ready to execute" confirmation
|
||||
* - 'silent_write' — a Write/Edit landed BEFORE any prompt, to a path
|
||||
* outside the sanctioned plan/project directories
|
||||
* - 'wrote_findings_before_asking' — strictPlanWrites only (seeded runs):
|
||||
* the plan file was rewritten with findings before any
|
||||
* AskUserQuestion render (the May-2026 transcript bug)
|
||||
* - 'exited' — claude process died before any of the above
|
||||
* - 'timeout' — none of the above within budget
|
||||
*/
|
||||
outcome: 'asked' | 'auto_decided' | 'plan_ready' | 'silent_write' | 'exited' | 'timeout';
|
||||
outcome:
|
||||
| 'asked'
|
||||
| 'auto_decided'
|
||||
| 'plan_ready'
|
||||
| 'silent_write'
|
||||
| 'wrote_findings_before_asking'
|
||||
| 'exited'
|
||||
| 'timeout';
|
||||
/** Human-readable summary. */
|
||||
summary: string;
|
||||
/** Visible terminal text since the slash command was sent (last 2KB). */
|
||||
|
|
@ -1567,6 +1589,14 @@ export interface PlanSkillObservation {
|
|||
* plan-mode smokes assert true; the no-op regression asserts false.
|
||||
*/
|
||||
scopeGateAutoSelectObserved?: boolean;
|
||||
/**
|
||||
* High-water map for opts.trackTokens: token → did it EVER appear in the
|
||||
* cumulative visible buffer? Consumption asserts (e.g. "the pasted target's
|
||||
* distinctive token shows up in the review output") must not depend on the
|
||||
* lossy 2KB evidence tail — plan-file fallbacks are unreachable outside
|
||||
* plan mode (extractPlanFilePath only matches plan-mode save renders).
|
||||
*/
|
||||
tokensObserved?: Record<string, boolean>;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -1627,6 +1657,10 @@ export async function runPlanSkillObservation(opts: {
|
|||
/** Override the spawned model. Defaults via launchClaudePty's chain
|
||||
* (opts.model ?? EVALS_MODEL ?? 'claude-sonnet-4-6'). */
|
||||
model?: string;
|
||||
/** Literal tokens to track as high-water marks over the CUMULATIVE visible
|
||||
* buffer (case-sensitive). Results land in obs.tokensObserved. Use for
|
||||
* consumption asserts that must survive the 2KB evidence tail. */
|
||||
trackTokens?: string[];
|
||||
}): Promise<PlanSkillObservation> {
|
||||
const startedAt = Date.now();
|
||||
const session = await launchClaudePty({
|
||||
|
|
@ -1672,6 +1706,19 @@ export async function runPlanSkillObservation(opts: {
|
|||
let waitingEverObserved = false;
|
||||
let scopeGateQuestionObserved = false;
|
||||
let scopeGateAutoSelectObserved = false;
|
||||
const tokensObserved: Record<string, boolean> = {};
|
||||
for (const t of opts.trackTokens ?? []) tokensObserved[t] = false;
|
||||
// Single source for the high-water flags at EVERY return site. Hand-
|
||||
// spreading them per-site already drifted once (the judge-waiting return
|
||||
// omitted the prose/waiting flags); a site that forgets a must-stay-false
|
||||
// flag makes `obs.flag ?? false` negative assertions pass vacuously.
|
||||
const highWaterFlags = () => ({
|
||||
proseAUQEverObserved,
|
||||
waitingEverObserved,
|
||||
scopeGateQuestionObserved,
|
||||
scopeGateAutoSelectObserved,
|
||||
...(opts.trackTokens?.length ? { tokensObserved } : {}),
|
||||
});
|
||||
const JUDGE_AFTER_MS = 60_000;
|
||||
const JUDGE_INTERVAL_MS = 30_000;
|
||||
while (Date.now() - start < budgetMs) {
|
||||
|
|
@ -1684,8 +1731,7 @@ export async function runPlanSkillObservation(opts: {
|
|||
summary: `claude exited (code=${session.exitCode()}) before reaching a terminal outcome`,
|
||||
evidence: visible.slice(-2000),
|
||||
elapsedMs: Date.now() - startedAt,
|
||||
scopeGateQuestionObserved,
|
||||
scopeGateAutoSelectObserved,
|
||||
...highWaterFlags(),
|
||||
};
|
||||
}
|
||||
if (visible.includes('Unknown command:')) {
|
||||
|
|
@ -1694,8 +1740,7 @@ export async function runPlanSkillObservation(opts: {
|
|||
summary: `claude rejected /${opts.skillName} as unknown command (skill not registered in this cwd)`,
|
||||
evidence: visible.slice(-2000),
|
||||
elapsedMs: Date.now() - startedAt,
|
||||
scopeGateQuestionObserved,
|
||||
scopeGateAutoSelectObserved,
|
||||
...highWaterFlags(),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -1718,6 +1763,9 @@ export async function runPlanSkillObservation(opts: {
|
|||
if (!scopeGateAutoSelectObserved && isScopeGateAutoSelectVisible(visible)) {
|
||||
scopeGateAutoSelectObserved = true;
|
||||
}
|
||||
for (const t of opts.trackTokens ?? []) {
|
||||
if (!tokensObserved[t] && visible.includes(t)) tokensObserved[t] = true;
|
||||
}
|
||||
|
||||
const classified = classifyVisible(visible, {
|
||||
strictPlanWrites: !!opts.initialPlanContent,
|
||||
|
|
@ -1727,10 +1775,7 @@ export async function runPlanSkillObservation(opts: {
|
|||
...classified,
|
||||
evidence: visible.slice(-2000),
|
||||
elapsedMs: Date.now() - startedAt,
|
||||
proseAUQEverObserved,
|
||||
waitingEverObserved,
|
||||
scopeGateQuestionObserved,
|
||||
scopeGateAutoSelectObserved,
|
||||
...highWaterFlags(),
|
||||
};
|
||||
// Capture the plan file path on any outcome where one may have been
|
||||
// written. Gating only on 'plan_ready' missed two cases: (1) the
|
||||
|
|
@ -1761,8 +1806,7 @@ export async function runPlanSkillObservation(opts: {
|
|||
summary: `LLM judge: ${lastJudgeVerdict.reasoning} (state=waiting after ${Math.round(elapsed / 1000)}s)`,
|
||||
evidence: visible.slice(-2000),
|
||||
elapsedMs: Date.now() - startedAt,
|
||||
scopeGateQuestionObserved,
|
||||
scopeGateAutoSelectObserved,
|
||||
...highWaterFlags(),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
@ -1784,10 +1828,7 @@ export async function runPlanSkillObservation(opts: {
|
|||
: ''),
|
||||
evidence: finalVisible.slice(-2000),
|
||||
elapsedMs: Date.now() - startedAt,
|
||||
proseAUQEverObserved,
|
||||
waitingEverObserved,
|
||||
scopeGateQuestionObserved,
|
||||
scopeGateAutoSelectObserved,
|
||||
...highWaterFlags(),
|
||||
};
|
||||
}
|
||||
return {
|
||||
|
|
@ -1799,10 +1840,7 @@ export async function runPlanSkillObservation(opts: {
|
|||
: ''),
|
||||
evidence: finalVisible.slice(-2000),
|
||||
elapsedMs: Date.now() - startedAt,
|
||||
proseAUQEverObserved,
|
||||
waitingEverObserved,
|
||||
scopeGateQuestionObserved,
|
||||
scopeGateAutoSelectObserved,
|
||||
...highWaterFlags(),
|
||||
};
|
||||
} finally {
|
||||
await session.close();
|
||||
|
|
@ -2173,11 +2211,23 @@ export async function runPlanSkillFloorCheck(opts: {
|
|||
const start = Date.now();
|
||||
let lastJudgeAt = 0;
|
||||
let lastJudgeVerdict: PtyStateVerdict | null = null;
|
||||
// Positional anchor for the scope-gate exclusion. The visible buffer is
|
||||
// append-only (old renders never leave scrollback), so a gate question
|
||||
// rendered in the 3s pre-target window would keep satisfying the
|
||||
// full-buffer acceptance checks forever while a tail-only exclusion
|
||||
// stops seeing it after ~TAIL_SCAN_BYTES of output — a vacuous
|
||||
// auq_observed (found independently by 4 review passes). Once the gate
|
||||
// render is seen, acceptance only counts AUQ renders in content APPENDED
|
||||
// after that point.
|
||||
let gateSeenIdx = -1;
|
||||
const JUDGE_AFTER_MS = 60_000;
|
||||
const JUDGE_INTERVAL_MS = 30_000;
|
||||
while (Date.now() - start < timeoutMs) {
|
||||
await Bun.sleep(2000);
|
||||
const visible = session.visibleSince(since);
|
||||
if (gateSeenIdx === -1 && isScopeGateQuestionVisible(visible)) {
|
||||
gateSeenIdx = visible.length;
|
||||
}
|
||||
|
||||
if (session.exited()) {
|
||||
return {
|
||||
|
|
@ -2206,12 +2256,16 @@ export async function runPlanSkillFloorCheck(opts: {
|
|||
//
|
||||
// Scope-gate renders do NOT count: the gate's "What should I review?"
|
||||
// can fire inside the 3s pre-target window and would trivially satisfy
|
||||
// the floor, but the floor measures FINDING-driven questions. The
|
||||
// exclusion is TAIL-scoped so an early gate render that has scrolled
|
||||
// out doesn't suppress a later, real finding AUQ.
|
||||
// the floor, but the floor measures FINDING-driven questions. Once a
|
||||
// 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.
|
||||
const tail = visible.slice(-TAIL_SCAN_BYTES);
|
||||
const acceptWindow = gateSeenIdx === -1 ? visible : visible.slice(gateSeenIdx);
|
||||
if (
|
||||
(isNumberedOptionListVisible(visible) || isProseAUQVisible(visible)) &&
|
||||
(isNumberedOptionListVisible(acceptWindow) || isProseAUQVisible(acceptWindow)) &&
|
||||
!isPermissionDialogVisible(tail) &&
|
||||
!isScopeGateQuestionVisible(tail)
|
||||
) {
|
||||
|
|
@ -2235,7 +2289,10 @@ export async function runPlanSkillFloorCheck(opts: {
|
|||
lastJudgeAt = Date.now();
|
||||
logPtySnapshot(visible, { testName: opts.skillName, elapsedMs: elapsed, tag: 'floor-judge-tick' });
|
||||
lastJudgeVerdict = judgePtyState(visible, { testName: opts.skillName });
|
||||
if (lastJudgeVerdict.state === 'waiting') {
|
||||
// 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)) {
|
||||
return {
|
||||
auqObserved: true,
|
||||
outcome: 'auq_observed',
|
||||
|
|
|
|||
|
|
@ -264,6 +264,33 @@ Recommendation: A when a branch diff exists, otherwise B.
|
|||
expect(isScopeGateAutoSelectVisible(sample)).toBe(false);
|
||||
});
|
||||
|
||||
test('stays false on a VERBATIM QUOTE of the announcement (negation narration)', () => {
|
||||
// The exact announcement line sits quoted in the skill context, so a
|
||||
// model explaining why it is NOT firing it can reproduce it byte-exact
|
||||
// inside quotes — that must not trip a must-stay-false assert.
|
||||
const sample =
|
||||
'Not in plan mode, so I won\'t announce "Scope gate: plan mode — auto-selected B (reviewing <target>)." and will ask instead.';
|
||||
expect(isScopeGateAutoSelectVisible(sample)).toBe(false);
|
||||
});
|
||||
|
||||
test('a later real render still matches after an earlier quoted mention', () => {
|
||||
const sample =
|
||||
'Earlier I said I would render "Scope gate: plan mode — auto-selected B (…)" and now:\n' +
|
||||
'Scope gate: plan mode — auto-selected B (reviewing PLAN.md).';
|
||||
expect(isScopeGateAutoSelectVisible(sample)).toBe(true);
|
||||
});
|
||||
|
||||
test('matches tense paraphrases WITH the announcement prefix (auto-selecting / auto-selects)', () => {
|
||||
expect(
|
||||
isScopeGateAutoSelectVisible('Scope gate: plan mode — auto-selecting B (reviewing the drafted plan).'),
|
||||
).toBe(true);
|
||||
expect(isScopeGateAutoSelectVisible('Scope gate: plan mode — auto-selects B.')).toBe(true);
|
||||
});
|
||||
|
||||
test('stays false on tense paraphrases WITHOUT the announcement prefix', () => {
|
||||
expect(isScopeGateAutoSelectVisible('Auto-selecting B since we are in plan mode.')).toBe(false);
|
||||
});
|
||||
|
||||
test('stays false on AUTO_DECIDE preamble output', () => {
|
||||
const sample = 'Auto-decided scope question → B (your preference). Change with /plan-tune.';
|
||||
expect(isScopeGateAutoSelectVisible(sample)).toBe(false);
|
||||
|
|
|
|||
Loading…
Reference in New Issue