diff --git a/autoplan/SKILL.md b/autoplan/SKILL.md index f6e4c6452..9e54f745d 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -789,7 +789,7 @@ Branch on the echoed `CODEX_MODE`: - **`under_codex`** — stale artifact selected its own harness. Print: "Codex outside review unavailable: harness mismatch; no outside process started. Missing coverage. Repair: setup --host codex." Skip the outside invocation and follow the workflow's native-review instructions below. Conflicting inherited harness markers are not grounds to guess another provider. - **`not_authed`** — installed but no credentials. Print: "Codex not authenticated; outside coverage unavailable. Run `codex login` or set `$CODEX_API_KEY`." Keep the required Claude adversarial pass; do not dispatch a duplicate. - **`broken_install`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: `npm install -g @openai/codex`." Relay the probe's HINT lines. Keep the required Claude adversarial pass; do not dispatch a duplicate. -- **`model_unusable`** — the selected model (see `CODEX_MODEL:`) is invalid or unavailable to the account (HTTP 400 on every call). Relay the probe's HINT lines and the fix (`GSTACK_CODEX_MODEL=` or config.toml `model`); never substitute a model. Keep the required Claude adversarial pass; do not dispatch a duplicate. The ~10s round trip is cached for 1h. +- **`model_unusable`** — the role invocation rejected policy, auth or model selection. Relay its reason and source-specific Repair/HINT lines, not a lower-priority setting. `AUTH_FAILED` needs `codex login`; never substitute a model. Keep the required Claude adversarial pass; do not dispatch a duplicate. - **`quota_exhausted`** — Codex usage limit: relay the probe's lines verbatim (reset time, retry); no more Codex calls this run. Keep the required Claude adversarial pass; do not dispatch a duplicate. - **`sandbox_unavailable`** — Codex's sandbox cannot start here (containers without user namespaces); the probe printed the reason and fix. No paid call ran; outside coverage is unavailable. Keep the required Claude adversarial pass; do not dispatch a duplicate. - **`ready`** or **`unverified`** — run the Codex pass below. `unverified` means the model check timed out or, with `(rate_limited)`, hit a 429; say so, and let the pass's own verdict decide. diff --git a/docs/model-policy.md b/docs/model-policy.md index ce2ec7022..f5f50befc 100644 --- a/docs/model-policy.md +++ b/docs/model-policy.md @@ -195,6 +195,10 @@ fresh. A catalog rollback reopens historical findings for a model put back in use. Only current default-branch runs may update that issue; copied markers in user-owned issues are not trusted state. +A documented retain decision preserves the current default, but does not make a +vendor mismatch `current`: the advisory issue stays open, or reopens, while that +candidate remains. Keep the rationale outside the tool-owned report region. + Thirty-day staleness means an absence of trustworthy freshness evidence, not that a model was released thirty days ago. An unchanged recommendation does not require a date-only catalog commit or a monthly paid benchmark. The recorded diff --git a/lib/model-policy-notice.ts b/lib/model-policy-notice.ts index 9b3fd8c9f..f4dc48d89 100644 --- a/lib/model-policy-notice.ts +++ b/lib/model-policy-notice.ts @@ -11,7 +11,7 @@ export function modelPolicyNoticeMarker(env: StateRootEnv = process.env, platfor export function modelPolicyNoticeText(selection: ModelSelection): string { const commands = modelPolicyCommands(); return [ - 'NOTICE: gstack plan reviews now use an independent plan-review model (default: frontier tier), not your general coding-model setting.', + 'NOTICE: gstack plan reviews now use an independent plan-review model by default (frontier tier), rather than general coding-model settings. Explicit overrides and host mode still apply.', `This review: ${describeSelection(selection)}.`, `Explicit choices still win, in order: a model named for the request, ${PROVIDER_ENV_OVERRIDES[selection.provider]}, then the per-tier config override.`, `Use the smart tier: ${commands.useSmart}. Keep host model settings: ${commands.useHost}. Shown once. Docs: https://github.com/garrytan/gstack/blob/main/docs/model-policy.md`, diff --git a/scripts/resolvers/constants.ts b/scripts/resolvers/constants.ts index 43c082d73..12b826d56 100644 --- a/scripts/resolvers/constants.ts +++ b/scripts/resolvers/constants.ts @@ -215,7 +215,7 @@ Branch on the echoed \`CODEX_MODE\`: - **\`under_codex\`** — stale artifact selected its own harness. Print: "Codex outside review unavailable: harness mismatch; no outside process started. Missing coverage. Repair: setup --host codex." Skip the outside invocation and follow the workflow's native-review instructions below. Conflicting inherited harness markers are not grounds to guess another provider. - **\`not_authed\`** — installed but no credentials. Print: "Codex not authenticated; outside coverage unavailable. Run \`codex login\` or set \`$CODEX_API_KEY\`." ${nativeRoute} - **\`broken_install\`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: \`npm install -g @openai/codex\`." Relay the probe's HINT lines. ${nativeRoute} -- **\`model_unusable\`** — the selected model (see \`CODEX_MODEL:\`) is invalid or unavailable to the account (HTTP 400 on every call). Relay the probe's HINT lines and the fix (\`GSTACK_CODEX_MODEL=\` or config.toml \`model\`); never substitute a model. ${nativeRoute} The ~10s round trip is cached for 1h. +- **\`model_unusable\`** — ${opts.role ? 'the role invocation rejected policy, auth or model selection. Relay its reason and source-specific Repair/HINT lines, not a lower-priority setting. `AUTH_FAILED` needs `codex login`; never substitute a model.' : "the selected model (see `CODEX_MODEL:`) is invalid or unavailable to the account (HTTP 400 on every call). Relay the probe's HINT lines and the fix (`GSTACK_CODEX_MODEL=` or config.toml `model`); never substitute a model."} ${nativeRoute}${opts.role ? '' : ' The ~10s round trip is cached for 1h.'} - **\`quota_exhausted\`** — Codex usage limit: relay the probe's lines verbatim (reset time, retry); no more Codex calls this run. ${nativeRoute} - **\`sandbox_unavailable\`** — Codex's sandbox cannot start here (containers without user namespaces); the probe printed the reason and fix. No paid call ran; outside coverage is unavailable. ${nativeRoute} - **\`ready\`** or **\`unverified\`** — run the Codex pass below. \`unverified\` means the model check timed out or, with \`(rate_limited)\`, hit a 429; say so, and let the pass's own verdict decide.${opts.role ? ` diff --git a/scripts/resolvers/outside-voice.ts b/scripts/resolvers/outside-voice.ts index 7c5eaead9..ddf1e45c4 100644 --- a/scripts/resolvers/outside-voice.ts +++ b/scripts/resolvers/outside-voice.ts @@ -124,12 +124,11 @@ _OUTSIDE_EXIT=0 "$_CODEX_PROBE" run-with-timeout ${Math.ceil(opts.timeoutMs / 1000)} ${codex} >"$_OUTSIDE_TMP/${opts.structuredBase ? 'text' : 'events'}" 2>"$_OUTSIDE_TMP/stderr" || _OUTSIDE_EXIT=$? ${opts.structuredBase ? 'cat "$_OUTSIDE_TMP/text"' : 'cat "$_OUTSIDE_TMP/text" 2>/dev/null || tail -n 20 "$_OUTSIDE_TMP/events"'}` : `_OUTSIDE_EXIT=0 -"${bin}/gstack-claude-code" --cwd "$_REPO_ROOT" --access ${opts.access ?? 'none'} --timeout-ms ${opts.timeoutMs}${opts.role ? ` --role ${opts.role}` : ''} <"$_OUTSIDE_INPUT" >"$_OUTSIDE_TMP/result.json" 2>"$_OUTSIDE_TMP/stderr" || _OUTSIDE_EXIT=$? +: >"$_OUTSIDE_TMP/stderr" || exit 1 +"${bin}/gstack-claude-code" --cwd "$_REPO_ROOT" --access ${opts.access ?? 'none'} --timeout-ms ${opts.timeoutMs}${opts.role ? ` --role ${opts.role}` : ''} <"$_OUTSIDE_INPUT" >"$_OUTSIDE_TMP/result.json" || _OUTSIDE_EXIT=$? # Preserve session/usage/modelUsage from this JSON; multiple models have no invented primary. cat "$_OUTSIDE_TMP/result.json" || { [ "$_OUTSIDE_EXIT" -ne 0 ] || _OUTSIDE_EXIT=1; } -if [ "$_OUTSIDE_EXIT" -eq 0 ]; then - bun -e 'const r=await Bun.file(process.argv[1]).json(); if(r.status!=="completed" || typeof r.result!=="string" || !r.result.trim()) process.exit(1); await Bun.write(process.argv[2],r.result)' "$_OUTSIDE_TMP/result.json" "$_OUTSIDE_TMP/text" || _OUTSIDE_EXIT=1 -fi`; +bun -e 'const r=await Bun.file(process.argv[1]).json(); await Bun.write(process.argv[3],typeof r.stderr==="string"?r.stderr:""); if(r.status!=="completed" || typeof r.result!=="string" || !r.result.trim()) process.exit(1); await Bun.write(process.argv[2],r.result)' "$_OUTSIDE_TMP/result.json" "$_OUTSIDE_TMP/text" "$_OUTSIDE_TMP/stderr" || { [ "$_OUTSIDE_EXIT" -ne 0 ] || _OUTSIDE_EXIT=1; }`; return `${outsideVoiceGuard(ctx)} ${runtimeRootPrelude(ctx)} _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo 'ERROR: not in a git repo' >&2; exit 1; } diff --git a/test/model-role-invocation.test.ts b/test/model-role-invocation.test.ts index c37126742..f2d1bdb38 100644 --- a/test/model-role-invocation.test.ts +++ b/test/model-role-invocation.test.ts @@ -103,6 +103,15 @@ const NOTICE = 'NOTICE: gstack plan reviews now use an independent plan-review m const marker = (state: string) => path.join(state, '.model-policy-notice-v1'); describe('plan-review role membership in shared generators', () => { + test('role failures relay source-aware repairs while legacy native-setting advice stays no-role', () => { + const role = outsideVoicePreflight(ctxFor('autoplan', 'claude', HOST_PATHS.claude), { disabledBehavior: 'codex-only', role: 'plan-review' }); + expect(role).toContain('source-specific Repair/HINT lines'); + expect(role).toContain('policy, auth or model selection'); + expect(role).not.toContain('or config.toml `model`'); + const legacy = outsideVoicePreflight(ctxFor('review', 'claude', HOST_PATHS.claude), { disabledBehavior: 'codex-only' }); + expect(legacy).toContain('or config.toml `model`'); + }); + const roleBearing: Array<[string, (ctx: TemplateContext) => string]> = [ ['autoplan', ctx => RESOLVERS.OUTSIDE_INVOCATION(ctx, ['autoplan'])], ['spec', ctx => RESOLVERS.OUTSIDE_INVOCATION(ctx, ['spec'])], diff --git a/test/outside-voice-invocation.test.ts b/test/outside-voice-invocation.test.ts index f9f35b286..3684ef4c9 100644 --- a/test/outside-voice-invocation.test.ts +++ b/test/outside-voice-invocation.test.ts @@ -22,13 +22,21 @@ const PROMPT_TEXT = 'review this literal text: $(touch NEVER) `touch NEVER` "\'\ fs.writeFileSync(PROMPT, PROMPT_TEXT); const fakeSource = ` -import {writeFileSync} from 'node:fs'; +import {existsSync,writeFileSync} from 'node:fs'; const args = process.argv.slice(2); // The free sandbox preflight (\`codex sandbox ... true\`) succeeds unless a test plants its failure. if (args[0] === 'sandbox') { if (process.env.FAKE_SANDBOX_STDERR) { console.error(process.env.FAKE_SANDBOX_STDERR); process.exit(1); } process.exit(0); } const claude = process.env.FAKE_PROVIDER === 'claude-code'; const prompt = claude || (args[0] === 'exec' && args[1] === '-') ? await Bun.stdin.text() : args[0] === 'exec' ? args[1] : ''; writeFileSync(process.env.CAPTURE!, JSON.stringify({args,prompt,cwd:process.cwd()})); +if (process.env.FAKE_NOTICE_ACK) { + const deadline = Date.now() + 1500; + while (!existsSync(process.env.FAKE_NOTICE_ACK)) { + if (Date.now() >= deadline) { console.error('policy notice was not forwarded before provider execution'); process.exit(17); } + await Bun.sleep(10); + } + writeFileSync(process.env.FAKE_PAID_MARKER!, 'provider execution started'); +} if (process.env.FAKE_MODE === 'timeout') { if (!claude) console.log('Partial finding before timeout'); await new Promise(() => {}); @@ -86,6 +94,45 @@ function invoke(host: 'codex' | 'claude', options: Partial { + test('Claude policy notice reaches the caller before provider execution, while diagnostics remain available', async () => { + const state = fs.mkdtempSync(path.join(TMP, 'notice-state-')); + const ack = path.join(state, 'caller-saw-notice'); + const paid = path.join(state, 'provider-started'); + const diagnostics = path.join(state, 'provider-stderr'); + fs.writeFileSync(diagnostics, 'provider diagnostic retained\n'); + const ctx: TemplateContext = { skillName: 'plan-eng-review', tmplPath: 'plan-eng-review/SKILL.md.tmpl', host: 'codex', paths: HOST_PATHS.codex }; + const command = outsideVoiceCommand(ctx, { promptFile: PROMPT, timeoutMs: 3000, role: 'plan-review' }); + const child = Bun.spawn(['bash', '-c', command], { + cwd: DIR, env: { ...environment('codex'), HOME: TMP, CLAUDE_CONFIG_DIR: path.join(TMP, '.claude'), + GSTACK_STATE_ROOT: state, GSTACK_CLAUDE_MODEL: '', FAKE_NOTICE_ACK: ack, FAKE_PAID_MARKER: paid, FAKE_STDERR_FILE: diagnostics }, + stdout: 'pipe', stderr: 'pipe', timeout: 8000, + }); + const stdout = new Response(child.stdout).text(); + let stderr = ''; + const reader = child.stderr.getReader(); + const decoder = new TextDecoder(); + try { + while (true) { + const chunk = await reader.read(); + if (chunk.done) break; + stderr += decoder.decode(chunk.value, { stream: true }); + if (stderr.includes('NOTICE: gstack') && !fs.existsSync(ack)) { + expect(fs.existsSync(paid)).toBe(false); + fs.writeFileSync(ack, 'notice observed'); + } + } + expect(await child.exited).toBe(0); + expect(await stdout).toContain('OUTSIDE_STATUS: completed provider=claude-code host=codex'); + expect(fs.existsSync(paid)).toBe(true); + expect(stderr.match(/NOTICE: gstack/g)).toHaveLength(1); + expect(stderr).toContain('CLAUDE_MODEL: plan-review via anthropic: claude-fable-5-1'); + expect(stderr).toContain('provider diagnostic retained'); + } finally { + reader.releaseLock(); + if (child.exitCode === null) child.kill(); + } + }); + for (const host of ['codex', 'claude'] as const) { test(`${host}: creative direction retains the completed recommendation gate`, () => { const options = { purpose: 'design-direction' as const };