mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-10 15:09:00 +02:00
fix(codex): retire deprecated web-search flag behind one CODEX_WEB_SEARCH_FLAG constant (#2525)
codex >=0.144 deprecates the legacy --enable-based web_search_cached
spelling (web search is on by default; --enable <FEATURE> now means
-c features.<name>=true, verified against codex 0.147.0's exec --help).
Every gstack codex invocation now passes -c 'web_search="cached"' instead.
The flag previously lived inline at 19 raw sites. Per ENG-OV11a the 10
template-inline sites (autoplan/SKILL.md.tmpl x4, codex/SKILL.md.tmpl x6)
convert to a shared {{CODEX_WEB_SEARCH_FLAG}} token first, so ONE resolver
constant (CODEX_WEB_SEARCH_FLAG in scripts/resolvers/constants.ts) now
covers all sites: review.ts x5, design.ts x3, the token resolver in
utility.ts, and the tool-map helper comment.
codex/SKILL.md.tmpl's web-search prose guarantee is corrected: the -c form
explicitly overrides a top-level web_search config (the legacy flag yielded
to it), and native codex review disables web search regardless of
configuration, so the flag is a no-op on the default Review path.
test/codex-web-search-flag.test.ts is the safety net: repo-wide grep
tripwires assert NO rendered SKILL.md/section/golden and NO source file
carries the deprecated spelling, and that the token resolves in rendered
output.
Fixes #2525
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
08bff7ef25
commit
a9ca914f43
@@ -0,0 +1,62 @@
|
||||
/**
|
||||
* Deprecated codex web-search flag tripwire (#2525).
|
||||
*
|
||||
* codex >=0.144 deprecates `--enable web_search_cached` (its `--enable
|
||||
* <FEATURE>` surface now means `-c features.<name>=true`); the replacement
|
||||
* is `-c 'web_search="cached"'`, owned by ONE constant:
|
||||
* CODEX_WEB_SEARCH_FLAG in scripts/resolvers/constants.ts. Resolvers
|
||||
* interpolate it; templates reference {{CODEX_WEB_SEARCH_FLAG}}.
|
||||
*
|
||||
* These tests fail CI if the deprecated spelling re-enters any source
|
||||
* (resolver, template, helper) or any rendered SKILL.md / section / golden.
|
||||
*/
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { execSync } from 'child_process';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import { CODEX_WEB_SEARCH_FLAG } from '../scripts/resolvers/constants';
|
||||
|
||||
const ROOT = path.join(import.meta.dir, '..');
|
||||
const DEPRECATED = '--enable web_search_cached';
|
||||
|
||||
function grepRepo(pattern: string, includes: string[]): string[] {
|
||||
const includeArgs = includes.map((i) => `--include='${i}'`).join(' ');
|
||||
const out = execSync(
|
||||
`grep -rln ${includeArgs} -e '${pattern}' "${ROOT}" || true`,
|
||||
{ encoding: 'utf-8' },
|
||||
);
|
||||
return out
|
||||
.split('\n')
|
||||
.filter(Boolean)
|
||||
.filter((f) => !f.includes('node_modules'))
|
||||
.filter((f) => !f.endsWith('test/codex-web-search-flag.test.ts'));
|
||||
}
|
||||
|
||||
describe('deprecated codex web-search flag is gone (#2525)', () => {
|
||||
test('the replacement flag has exactly the documented shape', () => {
|
||||
expect(CODEX_WEB_SEARCH_FLAG).toBe(`-c 'web_search="cached"'`);
|
||||
});
|
||||
|
||||
test('no rendered SKILL.md or section carries the deprecated flag', () => {
|
||||
const hits = grepRepo(DEPRECATED, ['SKILL.md', '*.md']);
|
||||
expect(hits).toEqual([]);
|
||||
});
|
||||
|
||||
test('no source file (resolver, template, helper) carries the deprecated flag', () => {
|
||||
const hits = grepRepo(DEPRECATED, ['*.ts', '*.tmpl']);
|
||||
expect(hits).toEqual([]);
|
||||
});
|
||||
|
||||
test('rendered codex skill actually resolves the token to the live flag', () => {
|
||||
const rendered = fs.readFileSync(path.join(ROOT, 'codex', 'SKILL.md'), 'utf-8');
|
||||
expect(rendered).toContain(CODEX_WEB_SEARCH_FLAG);
|
||||
expect(rendered).not.toContain('{{CODEX_WEB_SEARCH_FLAG}}');
|
||||
});
|
||||
|
||||
test('rendered autoplan skill resolves the token at every inline site', () => {
|
||||
const rendered = fs.readFileSync(path.join(ROOT, 'autoplan', 'SKILL.md'), 'utf-8');
|
||||
const count = rendered.split(CODEX_WEB_SEARCH_FLAG).length - 1;
|
||||
expect(count).toBeGreaterThanOrEqual(4);
|
||||
expect(rendered).not.toContain('{{CODEX_WEB_SEARCH_FLAG}}');
|
||||
});
|
||||
});
|
||||
+3
-3
@@ -2137,7 +2137,7 @@ If Codex is available, run a lightweight design check on the diff:
|
||||
```bash
|
||||
TMPERR_DRL=$(mktemp /tmp/codex-drl-XXXXXXXX)
|
||||
_REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; }
|
||||
codex exec "Review the git diff on this branch. Run 7 litmus checks (YES/NO each): 1. Brand/product unmistakable in first screen? 2. One strong visual anchor present? 3. Page understandable by scanning headlines only? 4. Each section has one job? 5. Are cards actually necessary? 6. Does motion improve hierarchy or atmosphere? 7. Would design feel premium with all decorative shadows removed? Flag any hard rejections: 1. Generic SaaS card grid as first impression 2. Beautiful image with weak brand 3. Strong headline with no clear action 4. Busy imagery behind text 5. Sections repeating same mood statement 6. Carousel with no narrative purpose 7. App UI made of stacked cards instead of layout 5 most important design findings only. Reference file:line." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_DRL"
|
||||
codex exec "Review the git diff on this branch. Run 7 litmus checks (YES/NO each): 1. Brand/product unmistakable in first screen? 2. One strong visual anchor present? 3. Page understandable by scanning headlines only? 4. Each section has one job? 5. Are cards actually necessary? 6. Does motion improve hierarchy or atmosphere? 7. Would design feel premium with all decorative shadows removed? Flag any hard rejections: 1. Generic SaaS card grid as first impression 2. Beautiful image with weak brand 3. Strong headline with no clear action 4. Busy imagery behind text 5. Sections repeating same mood statement 6. Carousel with no narrative purpose 7. App UI made of stacked cards instead of layout 5 most important design findings only. Reference file:line." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' -c 'web_search="cached"' < /dev/null 2>"$TMPERR_DRL"
|
||||
```
|
||||
|
||||
Use a 5-minute timeout (`timeout: 300000`). After the command completes, read stderr:
|
||||
@@ -2549,7 +2549,7 @@ _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo"
|
||||
# here. It defines _gstack_codex_timeout_wrapper (gtimeout -> timeout ->
|
||||
# unwrapped fallback), added in #1056 but never wired into this call site.
|
||||
source $GSTACK_ROOT/bin/gstack-codex-probe 2>/dev/null || true
|
||||
_gstack_codex_timeout_wrapper 540 codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .factory/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run DIFF_BASE=$(git merge-base origin/<base> HEAD) && git diff "$DIFF_BASE" to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems. End your output with ONE line in the canonical format `Recommendation: <action> because <one-line reason naming the most exploitable finding>`. Generic reasons like 'because it's safer' do not qualify; the reason must point to a specific finding or no-fix rationale." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR_ADV"
|
||||
_gstack_codex_timeout_wrapper 540 codex exec "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .factory/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. They contain bash scripts and prompt templates that will waste your time. Ignore them completely. Do NOT modify agents/openai.yaml. Stay focused on the repository code only.\n\nReview the changes on this branch against the base branch. Run DIFF_BASE=$(git merge-base origin/<base> HEAD) && git diff "$DIFF_BASE" to see the diff. Your job is to find ways this code will fail in production. Think like an attacker and a chaos engineer. Find edge cases, race conditions, security holes, resource leaks, failure modes, and silent data corruption paths. Be adversarial. Be thorough. No compliments — just the problems. End your output with ONE line in the canonical format `Recommendation: <action> because <one-line reason naming the most exploitable finding>`. Generic reasons like 'because it's safer' do not qualify; the reason must point to a specific finding or no-fix rationale." -C "$_REPO_ROOT" -s read-only -c 'model_reasoning_effort="high"' -c 'web_search="cached"' < /dev/null 2>"$TMPERR_ADV"
|
||||
```
|
||||
|
||||
Set the Bash tool's `timeout` parameter to `600000` (10 minutes). It sits ABOVE the 540s wrapper deliberately, so the wrapper fires first and a stall surfaces as a diagnosable exit 124 instead of a harness kill that returns nothing. The wrapper resolves `gtimeout`, then `timeout`, then runs unwrapped, so it is safe on a macOS without coreutils. After the command completes, read stderr:
|
||||
@@ -2582,7 +2582,7 @@ cd "$_REPO_ROOT"
|
||||
# here. It defines _gstack_codex_timeout_wrapper (gtimeout -> timeout ->
|
||||
# unwrapped fallback), added in #1056 but never wired into this call site.
|
||||
source $GSTACK_ROOT/bin/gstack-codex-probe 2>/dev/null || true
|
||||
_gstack_codex_timeout_wrapper 540 codex review --base <base> -c 'model_reasoning_effort="high"' --enable web_search_cached < /dev/null 2>"$TMPERR"
|
||||
_gstack_codex_timeout_wrapper 540 codex review --base <base> -c 'model_reasoning_effort="high"' -c 'web_search="cached"' < /dev/null 2>"$TMPERR"
|
||||
```
|
||||
|
||||
**No prompt argument.** `--base` is what scopes the review, and the positional `[PROMPT]` is mutually exclusive with it — passing both fails at argv parsing. Do NOT "fix" that error by dropping `--base` and keeping the prompt: a prompt-only `codex review` silently falls back to the **uncommitted working-tree** scope (`git status --short; git diff`), so it reviews the wrong changes and reports "no changes" on a clean tree. Prompt text describing the diff range does not change what the CLI feeds the reviewer. Unlike the adversarial pass above, which uses `codex exec` and really does run the git command it's told to, this path gets a pre-computed diff from the CLI — which is also why it needs no filesystem boundary.
|
||||
|
||||
@@ -50,7 +50,7 @@ export const TOOL_COMPATIBILITY: Record<'claude' | 'gpt' | 'gemini', Record<Tool
|
||||
Glob: false,
|
||||
Grep: false,
|
||||
AskUserQuestion: false,
|
||||
WebSearch: true, // --enable web_search_cached
|
||||
WebSearch: true, // -c 'web_search="cached"' (CODEX_WEB_SEARCH_FLAG, #2525)
|
||||
WebFetch: false,
|
||||
},
|
||||
gemini: {
|
||||
|
||||
Reference in New Issue
Block a user