fix: separate readiness diagnostics and bound version checks

This commit is contained in:
garrytan committed 2026-10-08 02:22:03 +00:00
1 parent 3fad722585
commit 8dac217465
6 files changed
+80 -15

No files matched your search

+1 -1
View File
@@ -793,7 +793,7 @@ Branch on the echoed `CODEX_MODE`:
- **`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.
Plan-review role: the invocation probes the [policy](https://github.com/garrytan/gstack/blob/main/docs/model-policy.md) model; its exit 1, 2 or 4 is `model_unusable`, `broken_install` or `quota_exhausted`.
Plan-review readiness probes the [policy](https://github.com/garrytan/gstack/blob/main/docs/model-policy.md) model. Relay its diagnostics; never infer quota from the review block's exit status.
Disabled/unavailable retains applicable native passes. Recheck each outside dispatch.
Record provider and completed/unavailable/disabled/skipped per phase; CEO covers
+5 -1
View File
@@ -654,9 +654,13 @@ _gstack_codex_version_check() {
# Capture the code from codex, not from `head` — a pipeline reports the LAST
# command's status, which is why a CLI that only ever printed a spawn error
# still read as healthy here (#2742). Keep stderr: it carries the diagnosis.
_ver=$(codex --version 2>&1)
_ver=$(_gstack_codex_timeout_wrapper 5 codex --version </dev/null 2>&1)
_vcode=$?
_ver=$(printf '%s' "$_ver" | head -1)
if [ "$_vcode" -eq 124 ]; then
echo 'WARN: `codex --version` timed out; version unknown. The invocation keeps its existing deadline.'
return 0
fi
# Only a NON-ZERO exit is evidence of a broken CLI. Empty-but-successful
# output stays silent by design (a CLI may legitimately print nothing), which
# the "empty output → OK" case in this file's suite pins.
+10 -5
View File
@@ -146,9 +146,11 @@ reviews stay disabled; an unavailable model is reported, never silently replaced
Policy setup and a completed review are different milestones. Install and
authenticate the opposing CLI using the [host setup instructions](../README.md#install--about-30-seconds)
before invoking a paid review. From Claude Code, use `/codex challenge --role
plan-review` with your plan. From Codex, use `/claude-code challenge --role
plan-review` (or its installed namespaced name).
before invoking a paid review. From Claude Code, use `/codex consult --role
plan-review`. From Codex, use `/claude-code consult --role plan-review` (or its
installed namespaced name). Ask it to challenge the plan and supply the full plan
text. Consult is prompt-driven and works before there is a repository diff;
Challenge mode is for reviewing code changes.
Confirm the announced selection, the provider's actual result and the coverage
status. A resolved model, a successful availability probe or a native fallback
@@ -186,8 +188,11 @@ source check without changing an issue; unlike `gstack-models`, it uses the
network. The workflow's guarded `--publish` path owns issue updates. Its JSON
report and job summary retain the source evidence.
The report distinguishes `current`, `update-candidate`, `stale` and
`unknown/source-unavailable`. Newer or newly recommended models are candidates,
The report's primary status distinguishes `current`, `update-candidate` and
`unknown/source-unavailable`. Its separate `evidenceStatus` marks retained
successful evidence `stale` after thirty days; a failed fetch remains unknown
in the headline even when that retained evidence is stale.
Newer or newly recommended models are candidates,
not automatic replacements. A support promise such as “not sooner than” is not
a scheduled retirement, and an incidental model mention is not a new default.
This evidence covers public API recommendations, not partner-platform entitlement.
+1 -1
View File
@@ -219,7 +219,7 @@ Branch on the echoed \`CODEX_MODE\`:
- **\`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 ? `
Plan-review role: the invocation probes the [policy](https://github.com/garrytan/gstack/blob/main/docs/model-policy.md) model; its exit 1, 2 or 4 is \`model_unusable\`, \`broken_install\` or \`quota_exhausted\`.` : ''}`;
Plan-review readiness probes the [policy](https://github.com/garrytan/gstack/blob/main/docs/model-policy.md) model. Relay its diagnostics; never infer quota from the review block's exit status.` : ''}`;
}
/**
+21 -2
View File
@@ -71,7 +71,7 @@ console.log(JSON.stringify({result:process.env.FAKE_TEXT || '[P1] Review found a
});
afterAll(() => fs.rmSync(DIR, {recursive:true, force:true}));
function shell(mode: Mode, extra: NodeJS.ProcessEnv = {}, options: {resume?: boolean; runtime?: string; cwd?: string} = {}) {
function shell(mode: Mode, extra: NodeJS.ProcessEnv = {}, options: {resume?: boolean; runtime?: string; cwd?: string; planReview?: boolean} = {}) {
fs.rmSync(CAPTURE, {force:true});
fs.writeFileSync(PROMPT, PROMPT_TEXT, {mode:0o600});
// These are precisely the literal substitutions the skill requests. Do not
@@ -80,7 +80,8 @@ function shell(mode: Mode, extra: NodeJS.ProcessEnv = {}, options: {resume?: boo
.replace("'<prepared-prompt-file>'", q(PROMPT))
.replace("'<gstack-runtime-root>'", q(options.runtime ?? RUNTIME))
.replace("'<base>'", q('main'))
.replace("'<fresh-or-resume>'", q(options.resume ? 'resume' : 'fresh'));
.replace("'<fresh-or-resume>'", q(options.resume ? 'resume' : 'fresh'))
.replaceAll('"$CLAUDE_RUNNER" --cwd', `"$CLAUDE_RUNNER"${options.planReview ? ' --role plan-review' : ''} --cwd`);
return spawnSync('bash', ['-c',script], {cwd:options.cwd ?? REPO, env:{...ENV,...extra}, encoding:'utf8',timeout:10000});
}
function captured() { return JSON.parse(fs.readFileSync(CAPTURE,'utf8')); }
@@ -203,4 +204,22 @@ describe('complete generated Claude Code wrapper modes', () => {
expectTempsCleaned();
}
});
test('the documented plan-review consultation consumes a plan even without a repository diff', () => {
const clean = fs.mkdtempSync(path.join(DIR, 'plan-only-'));
git(['init', '-b', 'main'], clean);
fs.writeFileSync(path.join(clean, 'file.txt'), 'unchanged\n');
git(['add', 'file.txt'], clean);
git(['commit', '-m', 'baseline'], clean);
const result = shell('consult', { HOME: clean, CLAUDE_CONFIG_DIR: path.join(clean, '.claude'),
GSTACK_STATE_ROOT: path.join(clean, 'policy'), GSTACK_CLAUDE_MODEL: '', FAKE_TEXT: 'The plan needs an overflow check.' },
{ cwd: clean, planReview: true });
expect(result.status).toBe(0);
expect(result.stdout).toContain('The plan needs an overflow check.');
const observed = JSON.parse(fs.readFileSync(CAPTURE, 'utf8'));
expect(observed.prompt).toBe(PROMPT_TEXT);
expect(observed.args).toContain('--model=claude-fable-5-1');
expect(result.stdout).not.toContain('Nothing to review');
expectTempsCleaned();
});
});
+42 -5
View File
@@ -30,7 +30,7 @@ fs.writeFileSync(FAKE, `
import { appendFileSync, writeFileSync } from 'node:fs';
const args = process.argv.slice(2);
if (args[0] === 'sandbox') process.exit(0);
if (args[0] === '--version') { if (process.env.FAKE_VERSION_SLEEP) Bun.sleepSync(Number(process.env.FAKE_VERSION_SLEEP) * 1000); console.log('codex-cli ' + (process.env.FAKE_CODEX_VERSION ?? '0.160.0')); process.exit(0); }
if (args[0] === '--version') { if (process.env.FAKE_VERSION_IGNORE_TERM) process.on('SIGTERM', () => {}); if (process.env.FAKE_VERSION_SLEEP) Bun.sleepSync(Number(process.env.FAKE_VERSION_SLEEP) * 1000); console.log('codex-cli ' + (process.env.FAKE_CODEX_VERSION ?? '0.160.0')); process.exit(0); }
const claude = process.env.FAKE_AS === 'claude';
const probe = !claude && args.includes('reply OK');
appendFileSync(process.env.CALLS!, JSON.stringify({ args, probe }) + '\\n');
@@ -43,14 +43,30 @@ if (probe) {
process.exit(0);
}
if (!claude || args.includes('-')) await Bun.stdin.text();
const response = 'Medium: the plan couples two rollouts.\\nRecommendation: split the migration because the plan couples two rollouts.';
const response = process.env.FAKE_RESPONSE || 'Medium: the plan couples two rollouts.\\nRecommendation: split the migration because the plan couples two rollouts.';
if (claude) console.log(JSON.stringify({ result: response, session_id: 's', modelUsage: { 'model-actual': { inputTokens: 1 } } }));
else writeFileSync(args[args.indexOf('-o') + 1], response);
`);
fs.writeFileSync(path.join(BIN, 'codex'), `#!/usr/bin/env bash\nexec bun "${FAKE}" "$@"\n`, { mode: 0o755 });
// Records when each supervised command starts and the deadline it was given.
const TIMEOUT_LOG = path.join(TMP, 'timeout.log');
fs.writeFileSync(path.join(BIN, 'timeout'), `#!/usr/bin/env bash\n[ -z "\${TIMEOUT_LOG:-}" ] || printf '%s %s\\n' "$(date +%s.%N)" "$*" >> "$TIMEOUT_LOG"\nexec "${Bun.which('timeout')}" "$@"\n`, { mode: 0o755 });
const TIMEOUT_FAKE = path.join(TMP, 'timeout.ts');
fs.writeFileSync(TIMEOUT_FAKE, `
import { appendFileSync } from 'node:fs';
const original = process.argv.slice(2);
if (process.env.TIMEOUT_LOG) appendFileSync(process.env.TIMEOUT_LOG, (Date.now()/1000) + ' ' + original.join(' ') + '\\n');
const args = [...original];
const grace = args[0] === '-k' ? (args.shift(), Number(args.shift())) : 10;
const duration = Number(args.shift());
const child = Bun.spawn(args, { stdin: 'inherit', stdout: 'inherit', stderr: 'inherit' });
let expired = false;
let killTimer;
const timer = setTimeout(() => { expired = true; child.kill('SIGTERM'); killTimer = setTimeout(() => child.kill('SIGKILL'), grace*1000); }, duration*1000);
const code = await child.exited;
clearTimeout(timer); clearTimeout(killTimer);
process.exit(expired ? 124 : code);
`);
for (const name of ['gtimeout', 'timeout']) fs.writeFileSync(path.join(BIN, name), `#!/usr/bin/env bash\nexec bun "${TIMEOUT_FAKE}" "$@"\n`, { mode: 0o755 });
const git = (args: string[]) => {
const r = spawnSync('git', args, { cwd: REPO, encoding: 'utf8', timeout: 5000, env: { ...process.env, GIT_AUTHOR_NAME: 'T', GIT_AUTHOR_EMAIL: 't@example.invalid', GIT_COMMITTER_NAME: 'T', GIT_COMMITTER_EMAIL: 't@example.invalid' } });
@@ -112,6 +128,17 @@ describe('plan-review role membership in shared generators', () => {
expect(legacy).toContain('or config.toml `model`');
});
test('readiness guidance never assigns quota meaning to the whole review exit status', () => {
const role = outsideVoicePreflight(ctxFor('autoplan', 'claude', HOST_PATHS.claude), { disabledBehavior: 'codex-only', role: 'plan-review' });
expect(role).not.toContain('its exit 1, 2 or 4 is');
expect(role).toContain("never infer quota from the review block's exit status");
const result = run('claude', home(), { FAKE_RESPONSE: 'Recommendation: proceed because this is a synthetic incomplete-format response.' });
expect(result.status).toBe(4);
expect(result.stdout).toContain('OUTSIDE_STATUS: unverified');
expect(result.stderr).not.toContain('MODEL_QUOTA_EXHAUSTED');
expect(result.calls.map((call: { probe: boolean }) => call.probe)).toEqual([true, false]);
});
const roleBearing: Array<[string, (ctx: TemplateContext) => string]> = [
['autoplan', ctx => RESOLVERS.OUTSIDE_INVOCATION(ctx, ['autoplan'])],
['spec', ctx => RESOLVERS.OUTSIDE_INVOCATION(ctx, ['spec'])],
@@ -325,7 +352,7 @@ describe('one deadline bounds Codex role readiness and dispatch (native F3)', ()
return { at: Number(at) - t0, duration: Number(duration), command: command.join(' ') };
});
const calls = fs.existsSync(CALLS) ? fs.readFileSync(CALLS, 'utf8').split('\n').filter(Boolean).map(line => JSON.parse(line)) : [];
return { ...r, out: `${r.stdout}${r.stderr}`, calls, dispatch: supervised.find(s => / exec - /.test(s.command)) };
return { ...r, out: `${r.stdout}${r.stderr}`, calls, supervised, dispatch: supervised.find(s => / exec - /.test(s.command)) };
}
// Readiness time already spent plus the dispatch's own limit stays within the
// provider budget (1.5s covers whole-second clock rounding and process start).
@@ -377,12 +404,22 @@ describe('one deadline bounds Codex role readiness and dispatch (native F3)', ()
}, 60000);
test('exhausted: readiness that outlives the whole budget dispatches nothing and is missing coverage', () => {
const r = timed(home(), { FAKE_VERSION_SLEEP: String(T + 1) });
const r = timed(home(), { FAKE_VERSION_SLEEP: String(T + 1), FAKE_VERSION_IGNORE_TERM: '1' });
expect(r.status).toBe(124);
expect(r.calls).toEqual([]);
expect(r.dispatch).toBeUndefined();
expect(r.out).not.toContain('OUTSIDE_STATUS: completed');
}, 60000);
test('a hanging version command is supervised before model readiness and dispatch', () => {
const r = timed(home(), { FAKE_VERSION_SLEEP: '8' });
expect(r.status).toBe(0);
const version = r.supervised.find(command => command.command === 'codex --version');
expect(version).toBeDefined();
expect(version!.duration).toBeLessThanOrEqual(5);
expect(r.out).toContain('codex --version` timed out');
withinBudget(r.dispatch!);
}, 60000);
});
describe('Claude plan-review invocation emits one selected model or none', () => {