Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Deploying crabcode with
|
| Latest commit: |
16ded02
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d2389b1f.crabcode.pages.dev |
| Branch Preview URL: | https://feat-disable-subagents.crabcode.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16ded02ac6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // `CRABCODE_DISABLE_SUBAGENTS=1` early in `main`, so this single env | ||
| // check covers flag + env without threading a bool through every | ||
| // `register_dynamic_tools` call site (TUI, print, ACP, serve). | ||
| if !super::task::subagents_disabled() { |
There was a problem hiding this comment.
When subagents are disabled and an image is sent to a text-only model with a configured vlm-agent, llm/client.rs:726-728 still sets show_vlm_agent_hint to true. The converted user message therefore commands the model to call task, while this branch omits that tool, and the normal unsupported-image warning is also suppressed. Include the disabled state when deciding whether to advertise the VLM fallback so these requests receive the warning instead of an impossible tool instruction.
Useful? React with 👍 / 👎.
| async fn disabled_env_omits_task_tool_from_dynamic_registry() { | ||
| let _lock = crate::tools::task::disable_subagents_env_lock(); | ||
| let prev = std::env::var(crate::tools::task::DISABLE_SUBAGENTS_ENV).ok(); | ||
| std::env::set_var(crate::tools::task::DISABLE_SUBAGENTS_ENV, "1"); |
There was a problem hiding this comment.
Under the default parallel Rust test runner, this process-wide environment mutation is serialized only against tests that explicitly acquire disable_subagents_env_lock; existing tests such as tools::task::tests::task_requires_parent_session_scoped_llm_config do not acquire it, yet TaskTool::execute now reads this variable. If those tests overlap, the latter returns the new disabled permission error instead of its expected missing-session error, making the suite nondeterministically fail. Use injected state for this test or ensure every reader participates in the same synchronization.
Useful? React with 👍 / 👎.
|
Closing as superseded: the requested automatic-subagent disabling behavior already exists through top-level |
1 participant
Fixes #55.
Behavior
crabcode --disable-subagents(global flag, also works ascrabcode --disable-subagents -p "…",crabcode --disable-subagents acp|serve) orCRABCODE_DISABLE_SUBAGENTS=1disables subagents everywhere (TUI, print, ACP, serve).tasktool is omitted from the registry (registry.get("task")isNone), filtered from model-facing tools, and dropped from the<available_subagents>prompt block so the model never spends tokens attempting calls that would fail.taskcall — including direct@agentmentions, which run through the same Task flow — is rejected with a permission error telling the user how to re-enable.Usage
crabcode --disable-subagents crabcode --disable-subagents -p "fix the failing test" CRABCODE_DISABLE_SUBAGENTS=1 crabcodeEnv truthy values:
1 | true | yes | y | on(case-insensitive, same convention asCRABCODE_DISABLE_CLAUDE_CODE). Flag and env are OR-ed; there is no re-enable override. Remoteservehosts enforce the host process environment.Tests
tools::task: env truthy/falsy values, flag/env precedence, disabledTaskTool::executerejects withPermissionwithout running a subagent.config::runtime: explicitdisable_subagents: truehidestaskbut keepsread; default keepstaskvisible.tools::init: disabled env omitstaskfrom the dynamic registry while keepingquestion.session::compaction::…prior_summary_billed_tokens…fails identically on cleanmain(verified via stash), so pre-existing and unrelated.Module/interface design note
Single owning module (
tools::task:DISABLE_SUBAGENTS_ENV,subagents_disabled(),resolve_subagents_disabled(flag)) behind a one-bool interface (--disable-subagents/ env /ConfigRuntimeOptions.disable_subagents).mainconverges flag→env once at startup, so the four enforcement points (permission kill-switch, registry omission, prompt suppression,executerejection) share one seam with no per-entrypoint threading;ConfigRuntimekeeps an explicit bool for env-free tests. Docs in_docs/config/index.mdx.