Follow-ups from the review of #739 / PR #818. None blocked that merge; capturing here so they aren't lost.
1. bypassPermissions has no dangerous-command filter (security)
The built-in ReAct engine runs every Bash command through is_dangerous_command() (codeframe/core/executor.py:52, used at codeframe/core/tools.py:802-807) to block destructive patterns before execution. ClaudeCodeAdapter's default --permission-mode bypassPermissions hands the delegated claude CLI unrestricted Bash with no equivalent CodeFrame-side filter. Combined with GitHub Issues import (#565) turning externally-authored issue bodies into task prompts, this widens the prompt-injection blast radius for claude-code specifically. Consider a CodeFrame-side guard or a narrower default (e.g. a curated --allowedTools set) and soften any "matches ReAct tool access" framing in docs.
2. Exit-0 clarification responses are hard-failed instead of routed to a blocker (correctness)
_map_result only runs classify_error_for_blocker on a non-zero exit. A --print run that exits 0 after printing a clarification question (a plausible non-interactive response) now gets hard-failed by the zero-file guard (empty diff, HEAD unmoved) with a "likely lacked write permission" message — instead of routed through the blocker flow a human could answer. Consider peeking at stdout for a question/blocker-shaped response before hard-failing.
3. require_file_changes default coupled to "no allowlist", not allowlist content
ClaudeCodeAdapter(require_file_changes=True) defaults on even if a caller passes a read-only allowlist (e.g. ["Read","Grep"]) for an analysis-only task. Not exploitable today (only engine_registry.get_external_adapter constructs it, and it never passes allowlist), but a future read-only caller would silently get hard failures. Consider deriving the default from whether the allowlist grants write tools.
4. Distinguish "git detection errored" from "nothing changed" (diagnostics)
detect_modified_files and _git_head both return empty/None on any git error (non-repo, missing binary, TimeoutExpired, lock contention) with no logging. Now that empty→failed for claude-code, a transient git hiccup can cause a real (though now fail-safe) false failure that's indistinguishable from "agent changed nothing" in logs. Add a logger.warning/debug in the error branches.
Follow-ups from the review of #739 / PR #818. None blocked that merge; capturing here so they aren't lost.
1.
bypassPermissionshas no dangerous-command filter (security)The built-in ReAct engine runs every Bash command through
is_dangerous_command()(codeframe/core/executor.py:52, used atcodeframe/core/tools.py:802-807) to block destructive patterns before execution.ClaudeCodeAdapter's default--permission-mode bypassPermissionshands the delegatedclaudeCLI unrestricted Bash with no equivalent CodeFrame-side filter. Combined with GitHub Issues import (#565) turning externally-authored issue bodies into task prompts, this widens the prompt-injection blast radius forclaude-codespecifically. Consider a CodeFrame-side guard or a narrower default (e.g. a curated--allowedToolsset) and soften any "matches ReAct tool access" framing in docs.2. Exit-0 clarification responses are hard-failed instead of routed to a blocker (correctness)
_map_resultonly runsclassify_error_for_blockeron a non-zero exit. A--printrun that exits 0 after printing a clarification question (a plausible non-interactive response) now gets hard-failed by the zero-file guard (empty diff, HEAD unmoved) with a "likely lacked write permission" message — instead of routed through the blocker flow a human could answer. Consider peeking at stdout for a question/blocker-shaped response before hard-failing.3.
require_file_changesdefault coupled to "no allowlist", not allowlist contentClaudeCodeAdapter(require_file_changes=True)defaults on even if a caller passes a read-only allowlist (e.g.["Read","Grep"]) for an analysis-only task. Not exploitable today (onlyengine_registry.get_external_adapterconstructs it, and it never passesallowlist), but a future read-only caller would silently get hard failures. Consider deriving the default from whether the allowlist grants write tools.4. Distinguish "git detection errored" from "nothing changed" (diagnostics)
detect_modified_filesand_git_headboth return empty/None on any git error (non-repo, missing binary,TimeoutExpired, lock contention) with no logging. Now that empty→failed forclaude-code, a transient git hiccup can cause a real (though now fail-safe) false failure that's indistinguishable from "agent changed nothing" in logs. Add alogger.warning/debugin the error branches.