mirror of https://github.com/garrytan/gstack.git
test(pty): pin spawned claude to EVALS model chain (default claude-sonnet-4-6)
launchClaudePty spawned the interactive `claude` TUI with no --model flag, so the child inherited the operator's ~/.claude/settings.json model. On a slow-thinking model that meant 5+ min of extended thinking on empty plan-mode context, timing out the plan-mode smoke tests regardless of contention. Pin the model via opts.model ?? EVALS_MODEL ?? 'claude-sonnet-4-6' — byte-identical to session-runner.ts:144, so PTY and `claude -p` evals always agree. Pushed before extraArgs (last flag wins, so a per-test --model still overrides). Placement leaves the spawn region byte-stable for a clean merge with the in-flight hermetic-env branch. Plumbed model through the three plan-skill wrappers. Static-grep tripwires guard the pin, its fallback chain, the before-extraArgs ordering, and all three wrapper forwards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
51e9351ed7
commit
6216c3e326
|
|
@ -70,6 +70,15 @@ export interface ClaudePtyOptions {
|
|||
permissionMode?: 'plan' | 'default' | 'acceptEdits' | 'bypassPermissions' | 'auto' | 'dontAsk' | null;
|
||||
/** Extra args after the permission-mode flag. */
|
||||
extraArgs?: string[];
|
||||
/**
|
||||
* Model for the spawned interactive `claude`. Without an explicit --model the
|
||||
* child inherits the operator's ~/.claude/settings.json model (e.g.
|
||||
* claude-fable-5[1m]), which can spend 5+ min in extended thinking on an empty
|
||||
* plan-mode context and blow every smoke budget. Resolution mirrors
|
||||
* session-runner.ts:144 exactly: opts.model ?? EVALS_MODEL ?? 'claude-sonnet-4-6'.
|
||||
* Pushed BEFORE extraArgs so a test-supplied --model still wins (last flag wins).
|
||||
*/
|
||||
model?: string;
|
||||
/** Terminal size. Default 120x40. Plan-mode UI lays out cleanly at this size. */
|
||||
cols?: number;
|
||||
rows?: number;
|
||||
|
|
@ -1137,9 +1146,15 @@ export async function launchClaudePty(
|
|||
let exited = false;
|
||||
let exitCodeCaptured: number | null = null;
|
||||
|
||||
const args: string[] = [];
|
||||
// Pin the model so smokes don't inherit the operator's settings.json model
|
||||
// (see ClaudePtyOptions.model). Chain mirrors session-runner.ts:144 so PTY and
|
||||
// `claude -p` evals always agree. Pushed before extraArgs => a test-supplied
|
||||
// --model wins (last flag wins).
|
||||
const model = opts.model ?? process.env.EVALS_MODEL ?? 'claude-sonnet-4-6';
|
||||
args.push('--model', model);
|
||||
// Permission mode: 'plan' default, null => omit flag entirely.
|
||||
const permissionMode = opts.permissionMode === undefined ? 'plan' : opts.permissionMode;
|
||||
const args: string[] = [];
|
||||
if (permissionMode !== null) {
|
||||
args.push('--permission-mode', permissionMode);
|
||||
}
|
||||
|
|
@ -1480,6 +1495,9 @@ export async function runPlanSkillObservation(opts: {
|
|||
* Step 0 reads the prior conversation context so it sees the draft.
|
||||
*/
|
||||
initialPlanContent?: string;
|
||||
/** Override the spawned model. Defaults via launchClaudePty's chain
|
||||
* (opts.model ?? EVALS_MODEL ?? 'claude-sonnet-4-6'). */
|
||||
model?: string;
|
||||
}): Promise<PlanSkillObservation> {
|
||||
const startedAt = Date.now();
|
||||
const session = await launchClaudePty({
|
||||
|
|
@ -1488,6 +1506,7 @@ export async function runPlanSkillObservation(opts: {
|
|||
timeoutMs: (opts.timeoutMs ?? 180_000) + 30_000,
|
||||
extraArgs: opts.extraArgs,
|
||||
env: opts.env,
|
||||
model: opts.model,
|
||||
});
|
||||
|
||||
try {
|
||||
|
|
@ -1744,6 +1763,8 @@ export async function runPlanSkillCounting(opts: {
|
|||
timeoutMs?: number;
|
||||
/** Extra env merged into the spawned `claude` process. */
|
||||
env?: Record<string, string>;
|
||||
/** Override the spawned model. Defaults via launchClaudePty's chain. */
|
||||
model?: string;
|
||||
}): Promise<PlanSkillCountObservation> {
|
||||
const startedAt = Date.now();
|
||||
const defaultPick = opts.defaultPick ?? 1;
|
||||
|
|
@ -1754,6 +1775,7 @@ export async function runPlanSkillCounting(opts: {
|
|||
cwd: opts.cwd,
|
||||
timeoutMs: timeoutMs + 60_000,
|
||||
env: opts.env,
|
||||
model: opts.model,
|
||||
});
|
||||
|
||||
const fingerprints: AskUserQuestionFingerprint[] = [];
|
||||
|
|
@ -1975,6 +1997,8 @@ export async function runPlanSkillFloorCheck(opts: {
|
|||
timeoutMs?: number;
|
||||
/** Extra env merged into the spawned `claude` process. */
|
||||
env?: Record<string, string>;
|
||||
/** Override the spawned model. Defaults via launchClaudePty's chain. */
|
||||
model?: string;
|
||||
}): Promise<PlanSkillFloorObservation> {
|
||||
const startedAt = Date.now();
|
||||
const timeoutMs = opts.timeoutMs ?? 600_000;
|
||||
|
|
@ -1984,6 +2008,7 @@ export async function runPlanSkillFloorCheck(opts: {
|
|||
cwd: opts.cwd,
|
||||
timeoutMs: timeoutMs + 60_000,
|
||||
env: opts.env,
|
||||
model: opts.model,
|
||||
});
|
||||
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@
|
|||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import {
|
||||
isPermissionDialogVisible,
|
||||
isNumberedOptionListVisible,
|
||||
|
|
@ -552,6 +553,45 @@ describe('runPlanSkillObservation env passthrough surface', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('launchClaudePty model pin (static tripwire)', () => {
|
||||
// Why static-grep, not a behavioral assert: the spawn fires immediately
|
||||
// inside launchClaudePty, so asserting the built args array would require
|
||||
// extracting an arg-builder seam — which rewrites the exact region kyoto-v5's
|
||||
// hermetic --strict-mcp-config insertion edits, reintroducing a merge
|
||||
// conflict the placement deliberately avoids. The end-to-end behavioral proof
|
||||
// is the live PTY smoke (skill-e2e-plan-*-plan-mode.test.ts) running under the
|
||||
// pinned model. These grep-level guards stop a refactor from silently
|
||||
// dropping the pin or reordering it past extraArgs.
|
||||
const src = readFileSync(new URL('./claude-pty-runner.ts', import.meta.url), 'utf-8');
|
||||
|
||||
test('ClaudePtyOptions exposes model?: string', () => {
|
||||
const opts: ClaudePtyOptions = { model: 'claude-sonnet-4-6' };
|
||||
expect(opts.model).toBe('claude-sonnet-4-6');
|
||||
});
|
||||
|
||||
test('spawn args push --model from the EVALS_MODEL fallback chain', () => {
|
||||
expect(src).toContain("args.push('--model', model)");
|
||||
// opts.model -> EVALS_MODEL -> 'claude-sonnet-4-6' (mirrors session-runner.ts:144)
|
||||
expect(src).toMatch(
|
||||
/opts\.model\s*\?\?\s*process\.env\.EVALS_MODEL\s*\?\?\s*'claude-sonnet-4-6'/,
|
||||
);
|
||||
});
|
||||
|
||||
test('--model is pushed BEFORE extraArgs so a per-test --model override wins', () => {
|
||||
const modelPush = src.indexOf("args.push('--model', model)");
|
||||
const extraArgsPush = src.indexOf('if (opts.extraArgs) args.push(...opts.extraArgs)');
|
||||
expect(modelPush).toBeGreaterThan(-1);
|
||||
expect(extraArgsPush).toBeGreaterThan(-1);
|
||||
expect(modelPush).toBeLessThan(extraArgsPush);
|
||||
});
|
||||
|
||||
test('all three plan-skill wrappers forward model to launchClaudePty', () => {
|
||||
// Count must match the number of wrappers (observation, counting, floor).
|
||||
const forwards = src.match(/^\s*model: opts\.model,$/gm) ?? [];
|
||||
expect(forwards.length).toBe(3);
|
||||
});
|
||||
});
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────────
|
||||
// Per-finding count primitives — Section 3 unit tests #1–#5, #7, #12.
|
||||
// ────────────────────────────────────────────────────────────────────────────
|
||||
|
|
|
|||
Loading…
Reference in New Issue