Skip to content

docs(shell): align CONFIGURATION/TOOL_SURFACE/error message with shell default (#3756 follow-up)#3787

Merged
Hmbown merged 1 commit into
mainfrom
codex/0866-shell-default-docs
Jun 29, 2026
Merged

docs(shell): align CONFIGURATION/TOOL_SURFACE/error message with shell default (#3756 follow-up)#3787
Hmbown merged 1 commit into
mainfrom
codex/0866-shell-default-docs

Conversation

@Hmbown

@Hmbown Hmbown commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Why

#3756 (merged) flipped interactive Agent shell to on-by-default (approval-gated) and updated MODES.md, but left three surfaces describing the old opt-in model. Post-merge main now has self-contradictory docs:

  • docs/CONFIGURATION.md — still said shell must be explicitly enabled.
  • docs/TOOL_SURFACE.md — still said Agent mode "usually means allow_shell = true".
  • crates/tui/src/core/engine/tool_catalog.rs — the exec_shell unavailable-reason and allow_shell hint still said shell "requires allow_shell = true".

Change

Docs + user-facing error message only — no behavior change. All three now state: interactive TUI Agent exposes shell by default with approval prompts unless allow_shell = false; headless/durable/noninteractive profiles keep the conservative default and require allow_shell = true; Plan hides shell; YOLO enables shell + auto-approval. Consistent with MODES.md.

Validation

cargo test -p codewhale-tui --bin codewhale-tuimissing_shell_tool_error_message_* (2) and tool_search_reports_known_core_action_tool... (1) pass; the improved message keeps every asserted substring. (Deliberately did not touch the #3756-entangled approval tests.)

Completes the documentation for #3756.

…l default

#3756 flipped interactive Agent shell to on-by-default (approval-gated) and
updated MODES.md, but CONFIGURATION.md, TOOL_SURFACE.md, and the exec_shell
"unavailable" reason / allow_shell hint in tool_catalog.rs still described the
old opt-in model. Post-merge `main` therefore had docs that contradicted both
the runtime and MODES.md.

Align all three with the shipped behavior: interactive TUI Agent exposes shell
by default with approval prompts unless `allow_shell = false`; headless/durable/
noninteractive profiles keep the conservative omitted-field default and require
`allow_shell = true`; Plan mode hides shell; YOLO enables shell + auto-approval.

Behavior unchanged (docs + user-facing message only). Existing shell-message
tests still pass (the improved message keeps the asserted substrings).
Completes the documentation for #3756.
@Hmbown Hmbown added this to the v0.8.66 milestone Jun 29, 2026
@claude

claude Bot commented Jun 29, 2026

Copy link
Copy Markdown

Claude finished @Hmbown's task in 1m 42s —— View job


PR Review

  • Gather context and read changed files
  • Review crates/tui/src/core/engine/tool_catalog.rs changes
  • Review docs/CONFIGURATION.md changes
  • Review docs/TOOL_SURFACE.md changes
  • Verify consistency with docs/MODES.md
  • Post review findings

Summary: Clean, focused documentation fix. No behavior changes. All three surfaces now consistently describe the new shell-on-by-default model, matching MODES.md. No correctness bugs found.

crates/tui/src/core/engine/tool_catalog.rs

exec_shell unavailable_reason (line ~74): Updated from the misleading "requires Agent or Yolo mode with allow_shell = true" to the accurate multi-path description (interactive default-on, noninteractive opt-in, Plan hidden). The tool_search_reports_known_core_action_tool_when_current_catalog_omits_it test asserts reason.contains("allow_shell = true") — the phrase "require allow_shell = true" in the new text satisfies that assertion. ✓

shell_tool_allow_shell_hint() (lines 652–658): All test assertions in missing_shell_tool_error_message_names_allow_shell_gate and _keeps_allow_shell_hint_with_suggestions are satisfied by the new text:

  • "allow_shell = false" → present ("commonly via top-level allow_shell = false")
  • "Agent mode" → present ("Interactive Agent mode")
  • "approval gating" → present ("with approval gating")
  • "/config allow_shell true", "--save" → unchanged
  • !contains("YOLO"), !contains("auto-approve") → correct, neither appears ✓

The new framing ("Shell tools are absent because this session or profile disabled shell access") is more accurate than the old imperative ("they require allow_shell = true") for the default-on world.

docs/CONFIGURATION.md

The preamble change at line ~123 is accurate and well-placed. The allow_shell changelog entry is the most important fix — the old single-line defaults to false; shell tools must be explicitly enabled was now flat-out wrong for interactive sessions. The new entry correctly distinguishes:

  • Interactive TUI Agent: omitting = on, false = hidden
  • Headless/durable/noninteractive: conservative default, require allow_shell = true
  • Plan: always hidden; YOLO: enabled + auto-approved

One pre-existing sentence at line ~138-140 (not changed in this PR) still reads: "project-level allow_shell = true is ignored". This remains technically correct (interactive sessions already have shell on; noninteractive can't be granted shell from project config), but it may be worth a follow-up clarification comment given the new default semantics.

docs/TOOL_SURFACE.md

Clean replacement of "usually means allow_shell = true" with the accurate split description. Consistent with the other changes. ✓

Verdict

Looks good to merge. All assertions pass, all three surfaces align with MODES.md, and no behavior changes slipped in.

@Hmbown Hmbown merged commit 72f6cb4 into main Jun 29, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant