Fix Codex Features Recognition Issue

Claude Code·Opus 5.5·gtrrz-victor·8h ago·16min·1 Checkpoint·2 file changes·+26/-2·9.8K tokens
8h ago·2m

Trail #1449 review: summary generators run with no tools and in an empty directory

Verdict: the approach is right. I'd approve once one fail-open case in the Codex feature probe is fixed. CI passes, there are no agent findings, and it merges cleanly into main, though it's 114 commits behind. The only thing blocking merge is that nobody has approved it.

What I checked:

  • The code diff in all 17 files.
  • go test ./cmd/entire/cli/agent/... passes. The one failure was in opencode, because my machine has no node version set in mise. That has nothing to do with this PR.
  • The new flags exist on the CLIs I have installed: --tools and --strict-mcp-config on Claude, and --ignore-user-config and --disable on codex 0.160.1.
  • I did not run the live test (ENTIRE_TEST_REAL_AGENTS), because it makes paid API calls.

Should fix

1. If codex features list changes its output format, every --disable is silently dropped (codex/generate.go, knownFeatures).

  • The code treats the first word of every output line as a feature name, and only counts the probe as failed if the result is empty.
  • If a future codex prints a header, a box-drawn table or JSON, the parsed names won't match any real feature. The filter then removes all 12 disables, and summaries run with shell and code-mode tools turned on. The only trace is a debug log line.
  • So exactly the case the PR worries about (a different codex version) fails open instead of closed.
  • Fix: treat the probe as failed when the result doesn't include an anchor feature such as shell_tool, or when it matches none of the denylist. Either way the full list gets passed. Add a test with output in an unexpected format.

Worth raising with the author

2. The Codex denylist is already falling behind, and nothing catches that automatically.

  • On codex 0.160.1, which is newer than the 0.156.1 the PR tested, these are on by default and not on the list: goals, tool_suggest, skill_search, remote_plugin, in_app_local_automation, browser_use_full_cdp_access and sleep_tool.
  • I haven't checked whether any of them can read files. But the PR says the opt-in live test "is what would catch it", and nothing runs that test on a schedule.
  • Suggestion: add browser_use_full_cdp_access at least. Also either run the live test on a regular schedule, or open a follow-up to review the list each time codex releases.

3. Old CLIs get a hard failure. Claude and Copilot users on a version that doesn't know --tools, --available-tools, -s or --disallow-temp-dir will see every summary fail, with only the CLI's own "unknown option" error. Codex gets a clear "update codex" message; Claude and Copilot don't. Failing closed is the right call for security, but these users would benefit from the same kind of message.

4. Codex users lose their custom provider or profile for summaries. That's because of --ignore-user-config, and the PR states it openly. It's worth a line in the changelog so those users aren't surprised.

Nits

  • The live test writes its canary to a fixed path in the system temp directory (entire-textgen-live-canary.txt), so two runs at the same time would collide. A unique name per run would fix it.
  • Claude's apiKeyHelper settings file is still written to the shared temp directory, not the fresh one. That's fine, and arguably better, since the agent can't see it. I'm only mentioning it because the PR description names that file as an exposure. It's now closed by --tools "" and the new working directory, not by moving the file.

What looks good

  • The isolation is designed carefully. A fresh MkdirTemp directory is created and removed for every run.
  • The Copilot allowlist naming a tool that doesn't exist is clever, and the comment explains why an empty list wouldn't work.
  • Cursor no longer runs with --force, and its workspace config denies every tool.
  • The Codex probe is decided by a separate, prompt-free command, so the untrusted run's output can never remove a --disable. There's a test that pins this (TestGenerateText_RunOutputCannotDropAFeature).
  • Every generator's argv is pinned in tests.

I haven't posted anything to the trail. Should I post these as a comment, or request changes on #1, using entire trail comment / request-changes?

okey, fix 1 only one. First publish the finding and then fix it

8h ago·4m

The only two failures come from my machine having no node version set in mise. Both are OpenCode tests, unrelated to this change. Format and lint passed. Committing: