Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

unsloth start: keep the local subagent unattended and out of plan mode#7437

Open
danielhanchen wants to merge 1 commit into
mainunslothai/unsloth:mainfrom
claude-subagent-plan-gateunslothai/unsloth:claude-subagent-plan-gateCopy head branch name to clipboard
Open

unsloth start: keep the local subagent unattended and out of plan mode#7437
danielhanchen wants to merge 1 commit into
mainunslothai/unsloth:mainfrom
claude-subagent-plan-gateunslothai/unsloth:claude-subagent-plan-gateCopy head branch name to clipboard

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

Problem

Two gaps in the local subagent added by #7326 and #7313, both found by driving it end to end rather than by reading the code.

The child could stall. It is launched with --print and its output is consumed by the parent, so there is nobody to answer a permission prompt. If the child decided to ask a question or enter plan mode, it sat there until the parent gave up, and from the outside that is indistinguishable from a slow model.

Plan mode leaked. SKILL.md asks the parent to call the read-only agent when the session is in plan mode, but that is only a request. A model that ignores it reaches the editing agent, which is exactly what plan mode is meant to prevent. Worth noting permissions.defaultMode is commonly set to plan, so this is the default path for those users, not an edge case.

Changes

Disallow the blocking tools on the child. AskUserQuestion and EnterPlanMode always, since neither can be answered unattended. The read-only child additionally drops Edit, Write and NotebookEdit, and keeps ExitPlanMode because that is what the tool does under plan mode normally. The editing child drops ExitPlanMode instead, as it was never in plan mode to exit.

Add a PreToolUse hook that enforces the routing. The hook reads permission_mode off stdin itself and denies the editing agent when it is plan, returning a reason that points at the read-only agent. It does not rely on the model having read SKILL.md.

The hook fails open. Any exception, unreadable stdin or malformed payload exits 0 without a decision, because a broken hook must never be able to block the parent session. It is also skipped under the WSL bridge, where sys.executable is a Windows path that the distro cannot run.

Scope

Session-local, as before. The hook is written into the generated session plugin under the temporary plugin root, not into ~/.claude. Nothing touches the user's existing profile, settings, agents or models.

Tests

26 passed, 1 skipped.

test_claude_plan_gate.py is new and covers the gate directly: denies under plan mode, allows under every other mode, allows when permission_mode is absent, and fails open on malformed stdin, empty stdin and a null payload. It also asserts the hook is wired to the scoped MCP tool name and that it is not emitted for the WSL bridge.

test_claude_subagent_mcp.py gains assertions that both child variants pass the right --disallowedTools set.

All changed files AST-checked, including parsing the plan-gate script source that is emitted as a string literal.

The local subagent child could stall waiting on a permission prompt, and a
parent session in plan mode could still reach the editing agent.

- Drop human-blocking tools from the child so it runs unattended. The
  read-only child also drops the file writers.
- Emit a PreToolUse hook that reads permission_mode itself and denies the
  editing agent under plan mode, so routing holds when the model ignores
  SKILL.md. Fails open, and is skipped under the WSL bridge where a Windows
  interpreter path is not runnable in the distro.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0c559e7fa

ℹ️ 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".

# also drops file writers; it keeps ExitPlanMode, as Claude does under plan mode.
"--disallowedTools",
(
"AskUserQuestion,EnterPlanMode,Edit,Write,NotebookEdit"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Disallow ExitPlanMode for the unattended read-only child

When the read-only child finishes planning, Claude normally invokes ExitPlanMode, which requests approval before leaving plan mode. This branch deliberately leaves that tool enabled even though the process has stdin connected to DEVNULL and no caller can answer, so a normal read-only delegation can still hang at the approval step—the behavior this change is intended to eliminate. It can also leave read-only mode if approval is otherwise supplied; add ExitPlanMode to this disallowed set and have the child return its plan directly.

Useful? React with 👍 / 👎.

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.