fix: disclose role selections before Claude review execution

This commit is contained in:
garrytan committed 2026-10-08 01:16:36 +00:00
1 parent 76cf566227
commit 3daba9a20f
7 files changed
+67 -8

No files matched your search

+1 -1
View File
@@ -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=<supported-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.
+4
View File
@@ -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
+1 -1
View File
@@ -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`,
+1 -1
View File
@@ -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=<supported-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=<supported-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 ? `
+3 -4
View File
@@ -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; }
+9
View File
@@ -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'])],
+48 -1
View File
@@ -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<OutsideCommandOptions
function capture() { return JSON.parse(fs.readFileSync(CAPTURE,'utf8')); }
describe('generated outside-review dispatch', () => {
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 };