From b4ba71b28d594ee95e274c1f078c923fdb45563b Mon Sep 17 00:00:00 2001 From: garrytan Date: Thu, 8 Oct 2026 03:02:22 +0000 Subject: [PATCH] fix: retry released probe locks and retain late-rejection repair context --- bin/gstack-codex-probe | 13 +++++++++++-- test/codex-probe-cache.test.ts | 24 ++++++++++++++++++++++++ test/model-role-invocation.test.ts | 13 +++++++++++++ test/run-in-background-guidance.test.ts | 2 +- 4 files changed, 49 insertions(+), 3 deletions(-) diff --git a/bin/gstack-codex-probe b/bin/gstack-codex-probe index 613f8ef9f..f09173b7b 100755 --- a/bin/gstack-codex-probe +++ b/bin/gstack-codex-probe @@ -207,7 +207,9 @@ _gstack_codex_role_ready() { _gstack_codex_auth_probe || return 1 _gstack_codex_sandbox_preflight || return $? _gstack_codex_version_check - _gstack_codex_model_probe "$1" + _gstack_codex_model_probe "$1" || return $? + [ -z "${_GSTACK_CODEX_SEL_REPAIR:-}" ] || echo "HINT: If the review rejects the selected model, repair its winning source: $_GSTACK_CODEX_SEL_REPAIR." + return 0 } # --- Sandbox (B1) ----------------------------------------------------------- @@ -595,7 +597,14 @@ _gstack_codex_lock_acquire() { rmdir "$_dir" 2>/dev/null return 2 fi - [ -d "$_dir" ] || return 2 + if [ ! -d "$_dir" ]; then + { [ -e "$_dir" ] || [ -L "$_dir" ]; } && return 2 + { [ -d "${_dir%/*}" ] && [ -w "${_dir%/*}" ] && [ -x "${_dir%/*}" ]; } || return 2 + [ "$_i" -ge "$_tries" ] && return 2 + _i=$((_i + 1)) + sleep 0.1 + continue + fi _now=$(date +%s 2>/dev/null || echo 0) _owner=$(cat "$_dir/owner" 2>/dev/null) _dead=0 diff --git a/test/codex-probe-cache.test.ts b/test/codex-probe-cache.test.ts index 092f46f2e..002fdb921 100644 --- a/test/codex-probe-cache.test.ts +++ b/test/codex-probe-cache.test.ts @@ -67,6 +67,30 @@ const withFixture = (body: (f: Fixture) => void | Promise) => async () => }; describe('bounded multi-signature probe cache', () => { + test('a write-lock holder releasing after a failed mkdir does not drop a negative entry', withFixture((f) => { + const lock = path.join(f.locks, 'write'); + const once = path.join(f.home, 'release-once'); + fs.mkdirSync(lock, { recursive: true }); + expect(fs.realpathSync(lock).startsWith(`${fs.realpathSync(f.home)}${path.sep}`)).toBe(true); + fs.writeFileSync(path.join(lock, 'owner'), `${process.pid} ${os.hostname()} ${Math.floor(Date.now() / 1000)}\n`); + fs.writeFileSync(once, 'armed'); + fs.writeFileSync(path.join(f.home, 'bin/mkdir'), `#!/usr/bin/env bash +if [ "$#" -eq 1 ] && [ "$1" = "$RACE_LOCK" ] && [ -f "$RACE_ONCE" ]; then + rm -f "$RACE_ONCE" "$RACE_LOCK/owner" + rmdir "$RACE_LOCK" + exit 1 +fi +exec ${JSON.stringify(Bun.which('mkdir'))} "$@" +`, { mode: 0o755 }); + const result = probe(f, { ...model('gpt-race'), STUB_MODE: 'quota', RACE_LOCK: lock, RACE_ONCE: once }); + expect(result.stdout).toContain('rc=4'); + expect(fs.existsSync(f.cache)).toBe(true); + expect(entries(f)).toHaveLength(1); + expect(entries(f)[0]).toStartWith('MODEL_QUOTA_EXHAUSTED '); + expect(probe(f, model('gpt-race')).stdout).toContain('MODEL_QUOTA_EXHAUSTED (cached)'); + expect(calls(f)).toBe(1); + })); + test('A -> B -> A makes exactly two paid probes', withFixture((f) => { expect(probe(f, model('gpt-a')).stdout).toContain('MODEL_OK'); expect(probe(f, model('gpt-b')).stdout).toContain('MODEL_OK'); diff --git a/test/model-role-invocation.test.ts b/test/model-role-invocation.test.ts index 1af5b2996..ce79e0550 100644 --- a/test/model-role-invocation.test.ts +++ b/test/model-role-invocation.test.ts @@ -43,6 +43,7 @@ if (probe) { process.exit(0); } if (!claude || args.includes('-')) await Bun.stdin.text(); +if (!claude && process.env.FAKE_DISPATCH_MODEL_ERROR) { console.error('ERROR: The model is not supported by this account.'); process.exit(1); } const response = process.env.FAKE_RESPONSE || 'Medium: the plan couples two rollouts.\\nRecommendation: split the migration because the plan couples two rollouts.'; if (claude && process.env.FAKE_PROVIDER_ERROR) { console.log(JSON.stringify({ is_error: true, result: process.env.FAKE_PROVIDER_ERROR })); process.exit(Number(process.env.FAKE_ERROR_EXIT ?? '1')); } if (claude) console.log(JSON.stringify({ result: response, session_id: 's', modelUsage: { 'model-actual': { inputTokens: 1 } } })); @@ -306,6 +307,18 @@ describe('Codex plan-review invocation binds one selection', () => { expect(r.stdout).not.toContain('OUTSIDE_STATUS'); }); + test('a dispatch-only Codex model rejection retains the selected-source repair after unverified readiness', () => { + const h = home(); + fs.writeFileSync(path.join(h.state, '.codex-model-probe.locks'), 'not a directory'); + const r = run('claude', h, { GSTACK_CODEX_MODEL: 'gpt-env', FAKE_DISPATCH_MODEL_ERROR: '1' }); + expect(r.status).toBe(1); + expect(r.calls.map((call: { probe: boolean }) => call.probe)).toEqual([false]); + expect(r.stderr).toContain('MODEL_PROBE_INCONCLUSIVE'); + expect(r.stderr).toContain('If the review rejects the selected model'); + expect(r.stderr).toContain('unset GSTACK_CODEX_MODEL'); + expect(r.stderr).toContain('model is not supported'); + }); + test('a broken CLI found by the role probe exits 2 and never dispatches', () => { const r = run('claude', home(), { FAKE_PROBE: 'broken' }); expect(r.status).toBe(2); diff --git a/test/run-in-background-guidance.test.ts b/test/run-in-background-guidance.test.ts index ec587a1c6..dc70821da 100644 --- a/test/run-in-background-guidance.test.ts +++ b/test/run-in-background-guidance.test.ts @@ -99,7 +99,7 @@ exec ${quote(Bun.which('cat')!)} "$@" })); }; // #2914: the selected model and its source are printed before the paid call. - const selected = (cached = false) => `CODEX_MODEL: gpt-6-astra (exec; role: plan-review, tier: frontier; source: gstack catalog frontier/openai (verified 2026-10-07))\nAUTH_OK\nMODEL_OK${cached ? ' (cached)' : ''}\n`; + const selected = (cached = false) => `CODEX_MODEL: gpt-6-astra (exec; role: plan-review, tier: frontier; source: gstack catalog frontier/openai (verified 2026-10-07))\nAUTH_OK\nMODEL_OK${cached ? ' (cached)' : ''}\nHINT: If the review rejects the selected model, repair its winning source: gstack-config set model_frontier_openai ; gstack-config set plan_review_tier smart; gstack-config set plan_review_tier host.\n`; return { dir, run, stale, staleError, calls, selected, created: () => fs.existsSync(created) ? fs.readFileSync(created, 'utf8').trim().split('\n') : [], cleanup: () => fs.rmSync(dir, { recursive: true, force: true }) };