mirror of
https://github.com/garrytan/gstack.git
synced 2026-06-10 20:07:49 +02:00
v1.57.2.0 feat: AskUserQuestion prose fallback when the tool fails at runtime (#1908)
* feat(auq): add gstack-session-kind + echo SESSION_KIND in preamble Classifies the session as spawned | headless | interactive from env markers (OPENCLAW_SESSION / GSTACK_HEADLESS / CONDUCTOR_* / CLAUDE_CODE_ENTRYPOINT / CI), defaulting to interactive. Echoed once at skill start alongside BRANCH/REPO_MODE so the AskUserQuestion-failure fallback can branch without a shell-out at failure time. Degrade-safe: empty/error => interactive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(auq): prose fallback when AskUserQuestion fails (interactive sessions) On a genuine AUQ failure (tool absent, or present-but-erroring like Conductor's flaky MCP returning '[Tool result missing due to internal error]'): retry once, then branch on SESSION_KIND — spawned auto-chooses, headless BLOCKs, interactive renders a prose decision brief the user answers by typing a letter. The prose fallback MUST surface the triad: a clear ELI10 of the issue, a per-choice Completeness score, and a recommendation+why (one paragraph per choice). Carves out the [plan-tune auto-decide] denial as NOT a failure, and qualifies the former 'tool_use, not prose' assertions so the rule isn't self-contradicting. Tests pin the triad, the SESSION_KIND branch, the OV2 collision guard, the always-loaded guarantee, and a cross-file invariant on the auto-decide prefix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(auq): default GSTACK_HEADLESS=1 in eval/E2E runners Headless harness runs classify as headless (BLOCK on AUQ failure rather than emit a prose question no one reads). SDK runner uses ambient mutation, not the Options.env object, to avoid breaking the SDK auth pipeline. Interactive-path suites opt out by overriding the env per-run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(auq): defensive PostToolUse error-fallback hook (OV3:B) When an AskUserQuestion call returns an error/missing result, this hook injects additionalContext reminding the model to run the prose fallback for the current SESSION_KIND. It does not render prose itself — it guarantees the reminder fires at the moment of failure instead of relying on the model recalling SESSION_KIND. Inert on success and inert if the platform never invokes PostToolUse on tool errors (unverified — could not force the Conductor MCP error in a harness; see the spike doc). The prompt-level fallback covers the case regardless. Decision logic is unit-tested deterministically; registered in setup beside the existing AUQ hooks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(auq): regenerate SKILL.md for all hosts + refresh ship goldens Regenerated from the resolver changes (gen:skill-docs --host all). Refreshes the byte-exact ship golden fixtures (claude/codex/factory). Spec prose tightened so the cross-cutting preamble addition stays under the 5% per-skill parity ceiling (investigate 4.8%) — guard unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(test): kebab testNames for section-loading E2Es to match TOUCHFILES keys The two section-loading E2E tests used display-form testNames ('/ship section-loading', '/plan-ceo-review section-loading') while every other E2E testName and their E2E_TOUCHFILES keys are kebab. The completeness gate does an exact `name in E2E_TOUCHFILES` check, so it failed (pre-existing on main); diff- based selection also couldn't match them. Align to ship-section-loading / plan-ceo-section-loading. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(test): make external-host freshness checks deterministic The parameterized host smoke + --host all freshness tests assumed an external `gen:skill-docs --host all` had run first (it never does in `bun test`), so which host reported STALE varied by sibling-test timing — flaky. Regenerate the gitignored external host dirs in a beforeAll so the --dry-run check is deterministic. It still catches non-deterministic generation (the real bug class for regenerated outputs); the tracked-claude freshness test runs earlier and is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(parity): headroom for AUQ cross-cutting addition on carved document-release Merging main brought the carve of document-release (smaller skeleton); the AUQ prose-fallback adds ~2KB to every skill's always-loaded preamble, landing document-release at ~5.9% over the pre-carve v1.53.0.0 baseline. Add a per-carve maxSizeRatio override (CARVE_GUARDS single source of truth) and bump only this skill to 1.08. All other skills keep the strict 1.05 ceiling. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(auq): harden error-fallback hook + harness per adversarial review Codex pre-landing review found three real issues: - The PostToolUse fallback hook shared source 'plan-tune-cathedral' with the question-log hook (same event+matcher); gstack-settings-hook replaces the entry, so it would have clobbered plan-tune capture. Give it its own 'auq-error-fallback' source (separate entry, both run); ALREADY_INSTALLED now requires both sources. - isErrorResponse triggered on any string containing 'internal error'/'is_error', so a real answer or a {"is_error": false} payload could fire the fallback after a successful question. Narrow it to the missing-result sentinel + boolean is_error. - The SDK runner mutated process.env.GSTACK_HEADLESS process-wide (leaked headless into later tests). Removed; GSTACK_HEADLESS=1 now lives in the eval package.json scripts, scoped to the invocation and inherited by the SDK child. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v1.57.2.0) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
@@ -0,0 +1,131 @@
|
||||
/**
|
||||
* auq-error-fallback-hook — the OV3:B runtime reliability layer.
|
||||
*
|
||||
* Two layers of testing:
|
||||
* - PURE functions (isErrorResponse, directiveFor): deterministic, the core logic.
|
||||
* - INTEGRATION: spawn the hook as a PostToolUse process with synthetic stdin and
|
||||
* a controlled env, assert it injects the right directive on an error result and
|
||||
* stays inert on a real answer.
|
||||
*
|
||||
* NOTE: whether the Claude Code PLATFORM invokes PostToolUse on an MCP
|
||||
* transport/missing-result error is unverified (could not force the Conductor
|
||||
* bug in a harness — see docs/spikes/claude-code-hook-mutation.md). These tests
|
||||
* pin the hook's BEHAVIOR given it is invoked; the platform trigger is the
|
||||
* documented residual risk. The hook is inert if never invoked.
|
||||
*/
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { spawnSync } from 'child_process';
|
||||
import * as path from 'path';
|
||||
import { isErrorResponse, directiveFor } from '../hosts/claude/hooks/auq-error-fallback-hook.ts';
|
||||
|
||||
const HOOK = path.resolve(__dirname, '..', 'hosts', 'claude', 'hooks', 'auq-error-fallback-hook.ts');
|
||||
|
||||
describe('isErrorResponse — only clear failures, never a real answer', () => {
|
||||
test('null / undefined / empty string are failures', () => {
|
||||
expect(isErrorResponse(null)).toBe(true);
|
||||
expect(isErrorResponse(undefined)).toBe(true);
|
||||
expect(isErrorResponse('')).toBe(true);
|
||||
expect(isErrorResponse(' ')).toBe(true);
|
||||
});
|
||||
|
||||
test('the Conductor missing-result string is a failure', () => {
|
||||
expect(isErrorResponse('[Tool result missing due to internal error]')).toBe(true);
|
||||
});
|
||||
|
||||
test('is_error: true / error-field / sentinel-in-content are failures', () => {
|
||||
expect(isErrorResponse({ is_error: true })).toBe(true);
|
||||
expect(isErrorResponse({ isError: true })).toBe(true);
|
||||
expect(isErrorResponse({ error: 'boom' })).toBe(true);
|
||||
expect(isErrorResponse({ content: 'Tool result missing due to internal error' })).toBe(true);
|
||||
});
|
||||
|
||||
test('a real answer is NOT a failure (no false trigger)', () => {
|
||||
expect(isErrorResponse({ answers: [{ option_label: 'A' }] })).toBe(false);
|
||||
expect(isErrorResponse('A')).toBe(false);
|
||||
// a choice that coincidentally contains "error" must not trip it
|
||||
expect(isErrorResponse({ answers: [{ option_label: 'Fix the error' }] })).toBe(false);
|
||||
expect(isErrorResponse('Investigate the login error')).toBe(false);
|
||||
});
|
||||
|
||||
test('Codex review: narrow detection — generic "error"/"is_error" substrings do NOT trigger', () => {
|
||||
// A real answer mentioning "internal error" must not be read as a failure.
|
||||
expect(isErrorResponse('Investigate the internal error')).toBe(false);
|
||||
// A serialized success payload containing the substring is_error:false must not trigger.
|
||||
expect(isErrorResponse('{"is_error": false, "answer": "A"}')).toBe(false);
|
||||
expect(isErrorResponse({ is_error: false })).toBe(false);
|
||||
expect(isErrorResponse({ content: 'The page had an internal error we fixed' })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('directiveFor — per-session-kind instruction', () => {
|
||||
test('interactive directive demands the prose triad', () => {
|
||||
const d = directiveFor('interactive');
|
||||
expect(d).toMatch(/ELI10/);
|
||||
expect(d).toMatch(/Completeness: X\/10/);
|
||||
expect(d).toMatch(/\(recommended\)/);
|
||||
expect(d).toMatch(/reply with a letter/i);
|
||||
expect(d).toMatch(/STOP/);
|
||||
});
|
||||
|
||||
test('headless directive BLOCKs', () => {
|
||||
expect(directiveFor('headless')).toMatch(/BLOCKED — AskUserQuestion unavailable/);
|
||||
});
|
||||
|
||||
test('spawned directive auto-chooses', () => {
|
||||
expect(directiveFor('spawned')).toMatch(/auto-choose/i);
|
||||
});
|
||||
});
|
||||
|
||||
/** Spawn the hook with synthetic stdin + controlled env; parse its JSON stdout. */
|
||||
function runHook(stdin: object, env: Record<string, string>): { additionalContext?: string } {
|
||||
const res = spawnSync('bun', [HOOK], {
|
||||
input: JSON.stringify(stdin),
|
||||
encoding: 'utf-8',
|
||||
env: { PATH: process.env.PATH ?? '/usr/bin:/bin', ...env },
|
||||
});
|
||||
const parsed = JSON.parse(res.stdout || '{}');
|
||||
return parsed.hookSpecificOutput ?? {};
|
||||
}
|
||||
|
||||
describe('hook integration — invoked as PostToolUse', () => {
|
||||
test('error result + headless env → injects BLOCK directive', () => {
|
||||
const out = runHook(
|
||||
{ tool_name: 'mcp__conductor__AskUserQuestion', tool_response: '[Tool result missing due to internal error]' },
|
||||
{ GSTACK_HEADLESS: '1' },
|
||||
);
|
||||
expect(out.additionalContext).toMatch(/BLOCKED — AskUserQuestion unavailable/);
|
||||
});
|
||||
|
||||
test('error result + interactive env → injects prose-triad directive', () => {
|
||||
const out = runHook(
|
||||
{ tool_name: 'AskUserQuestion', tool_response: null },
|
||||
{ CONDUCTOR_PORT: '55010' },
|
||||
);
|
||||
expect(out.additionalContext).toMatch(/render the decision as a PROSE message/i);
|
||||
expect(out.additionalContext).toMatch(/Completeness: X\/10/);
|
||||
});
|
||||
|
||||
test('error result + spawned env → injects auto-choose directive', () => {
|
||||
const out = runHook(
|
||||
{ tool_name: 'AskUserQuestion', tool_response: { is_error: true } },
|
||||
{ OPENCLAW_SESSION: '1' },
|
||||
);
|
||||
expect(out.additionalContext).toMatch(/auto-choose/i);
|
||||
});
|
||||
|
||||
test('SUCCESSFUL answer → no injection (inert on real answers)', () => {
|
||||
const out = runHook(
|
||||
{ tool_name: 'AskUserQuestion', tool_response: { answers: [{ option_label: 'A' }] } },
|
||||
{ GSTACK_HEADLESS: '1' },
|
||||
);
|
||||
expect(out.additionalContext).toBeUndefined();
|
||||
});
|
||||
|
||||
test('non-AUQ tool → defers (no injection)', () => {
|
||||
const out = runHook(
|
||||
{ tool_name: 'Bash', tool_response: null },
|
||||
{ GSTACK_HEADLESS: '1' },
|
||||
);
|
||||
expect(out.additionalContext).toBeUndefined();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user