claude-code adapter: pass --dangerously-skip-permissions#2
claude-code adapter: pass --dangerously-skip-permissions#2gabehamilton wants to merge 2 commits into
Conversation
The spawned `claude -p` subprocess has stdin=null, so it cannot answer permission prompts. Without this flag every tool call gets silently denied — stages degenerate into "partial_success with N tool errors" while doing no actual work. Verified against a local bootstrap run: before patch: install_sentrux ran 60s, status=partial_success, 17 tool errors after patch: install_sentrux ran 65s, status=success, 0 tool errors build_mock now actually writes its artifacts. fill_briefing actually reads and processes the briefing. End-to-end tool execution works. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request adds the --dangerously-skip-permissions flag to the Claude Code CLI adapter to prevent tool calls from being silently denied in non-interactive sessions where stdin is null. A security concern was raised regarding the potential for destructive actions without human oversight, and it is recommended to make this behavior an opt-in configuration via AgentRunOptions.
| // The spawned session has stdin=null so it cannot answer | ||
| // permission prompts. Without this flag every tool call gets | ||
| // silently denied and the stage degenerates into "partial_success | ||
| // with N tool errors" without actually doing work. | ||
| .arg("--dangerously-skip-permissions"); |
There was a problem hiding this comment.
Bypassing all permission prompts with --dangerously-skip-permissions is a significant security risk, as it allows the LLM to execute potentially destructive tools (such as shell commands or file writes) without human oversight. While this is necessary for non-interactive execution when stdin is null, it should ideally be an opt-in configuration. Consider adding a field to AgentRunOptions to allow users to explicitly enable this behavior, or at least ensure this security trade-off is prominently documented for users of this provider.
There was a problem hiding this comment.
Fixed in 0cd4fa3. Made it opt-in: added auto_approve_tools: bool to AgentRunOptions (defaults to false). ClaudeCodeAgentProvider only passes --dangerously-skip-permissions when that flag is true. The forge-attractor AgentProviderSubmitter (used by pipeline runs where stdin=null is the design) sets it to true; library consumers wiring up AgentProvider directly now have to make an explicit choice. Doc comment on the field explains the trade-off.
Addresses gemini-code-assist[bot] PR #2 review feedback. Skipping permission prompts is a real security trade-off; library consumers should opt in explicitly. Changes: - Add `auto_approve_tools: bool` field to AgentRunOptions (defaults to false via derive(Default)). - ClaudeCodeAgentProvider only passes --dangerously-skip-permissions when options.auto_approve_tools is true. Default behavior matches upstream claude CLI (interactive permission prompts). - AgentProviderSubmitter (forge-attractor's pipeline runtime) sets auto_approve_tools=true because pipeline stages run with stdin=null by design — every stage would otherwise fail at the first tool call. - Doc comment on the field explains the trade-off and that the field maps to --dangerously-skip-permissions on the Claude Code adapter. Library consumers that wire up AgentProvider directly now have to make an explicit choice. The forge pipeline runtime is unchanged in observable behavior (still skips prompts as before). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
The spawned `claude -p` subprocess has `stdin=null`, so it cannot answer permission prompts. Without `--dangerously-skip-permissions` every tool call gets silently denied — stages report `partial_success` with N tool errors while doing no real work.
Why it took a real run to spot this
The default permission policy in print mode appears to allow generic tools (Bash, Read, Write) at the type level, but specific invocations still trigger interactive approval flows. With `stdin=null` the subprocess just gives up. Forge sees the JSONL events for the attempted tool_use plus tool_result with `is_error: true`, increments a counter, marks the stage as `partial_success` (which forge treats as advancing), and moves on — so the pipeline progresses without producing artifacts.
Verified against a local bootstrap run
Before: `mock/` directory was never created. After: `mock/index.html`, `app.js`, `fixtures.json`, `style.css` all written.
Test plan
🤖 Generated with Claude Code