Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions codeframe/core/adapters/claude_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,37 @@ class ClaudeCodeAdapter(SubprocessAdapter):
https://docs.anthropic.com/en/docs/claude-code
"""

def __init__(self, allowlist: list[str] | None = None) -> None:
def __init__(
self,
allowlist: list[str] | None = None,
require_file_changes: bool = True,
) -> None:
"""Initialize the Claude Code adapter.

Args:
allowlist: Optional list of allowed tools/permissions.
If provided, uses ``--allowedTools`` flag for each tool.
When omitted, no permission flags are added (the caller
is responsible for configuring permissions externally).
When omitted, the adapter runs with
``--permission-mode bypassPermissions`` so Edit/Write/Bash
are auto-approved in non-interactive ``--print`` mode.
Without this, ``--print`` silently denies those tools and
the delegated agent can analyze but never modify files. (#739)
require_file_changes: If True (default), a run that exits 0 but touches
no files is downgraded to ``failed`` — a coding task that
writes nothing is a false completion.
"""
cli_args = ["--print"]
if allowlist:
for tool in allowlist:
cli_args.extend(["--allowedTools", tool])

super().__init__(binary="claude", cli_args=cli_args)
else:
cli_args.extend(["--permission-mode", "bypassPermissions"])

super().__init__(
binary="claude",
cli_args=cli_args,
require_file_changes=require_file_changes,
)
self._allowlist = allowlist

@property
Expand Down
67 changes: 67 additions & 0 deletions codeframe/core/adapters/subprocess_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,29 @@ def __init__(
binary: str,
cli_args: list[str] | None = None,
timeout_s: int | None = None,
require_file_changes: bool = False,
) -> None:
"""Initialize with the binary name and default CLI args.

Args:
binary: Name of the CLI binary (e.g., 'claude', 'opencode')
cli_args: Default CLI arguments appended to every invocation
timeout_s: Max execution time in seconds (default: 1800, None = no limit)
require_file_changes: If True, a run that exits 0 without producing any
work — no modified/untracked files and no new commit — is downgraded
to ``failed`` instead of ``completed``. Guards against a delegated
coding agent that "succeeds" without writing any code (e.g. edits
silently denied), which downstream gates would otherwise pass on the
unchanged tree. Only fires inside a resolvable git repo (a non-git
workspace can't be judged). Default False (analysis-capable agents).

Raises:
EnvironmentError: If the binary is not found on PATH
"""
self._binary = binary
self._cli_args = cli_args or []
self._timeout_s = timeout_s if timeout_s is not None else self.DEFAULT_TIMEOUT_S
self._require_file_changes = require_file_changes

resolved = shutil.which(binary)
if resolved is None:
Expand Down Expand Up @@ -95,6 +104,12 @@ def run(
cmd = self.build_command(prompt, workspace_path)
stdin_content = self.get_stdin(prompt)

# Baseline HEAD so a run that *commits* its work (bypassPermissions allows
# Bash) still counts as work despite an empty `git diff HEAD`. (#739)
head_before = (
self._git_head(workspace_path) if self._require_file_changes else None
)

stdout_lines: list[str] = []
stderr_chunks: list[str] = []

Expand Down Expand Up @@ -202,6 +217,38 @@ def _write_stdin() -> None:
workspace_path=workspace_path,
)
result.modified_files = modified_files

# A coding task that "succeeds" without touching any file is a false
# completion: edits were likely denied or the agent only analyzed. Fail
# hard so downstream gates don't pass on the unchanged tree. (#739)
# Only fire when we can *positively* confirm no work: a resolvable git
# repo whose HEAD didn't advance (self-committed work) and whose tree has
# no changes. A non-git workspace can't be judged, so we don't fail it.
if (
self._require_file_changes
and result.status == "completed"
and not modified_files
):
head_after = self._git_head(workspace_path)
in_git_repo = head_after is not None
# Require a known baseline to credit a commit: if the pre-run HEAD
# read failed (head_before is None) we must not let `None != sha`
# masquerade as "committed" and silently pass a real zero-file run.
# Bias toward failing loudly. (ponytail: a rare `git init` mid-run
# false-fails here — acceptable; a false COMPLETED is worse.)
committed = (
head_before is not None
and head_after is not None
and head_after != head_before
)
if in_git_repo and not committed:
result.status = "failed"
result.error = (
f"'{self._binary}' exited successfully but modified no files. "
"A coding task must change at least one file; the agent likely "
"lacked write permission or produced no edits."
)

return result

def _map_result(
Expand Down Expand Up @@ -244,6 +291,26 @@ def _detect_modified_files(self, workspace_path: Path) -> list[str]:
"""Detect files modified by the subprocess via git diff."""
return detect_modified_files(workspace_path)

def _git_head(self, workspace_path: Path) -> str | None:
"""Return the current HEAD commit sha, or None if HEAD is unresolvable.

None means "not a git repo, git unavailable, or an unborn HEAD" — i.e.
a state where modified-file detection can't judge whether work happened.
"""
try:
result = subprocess.run(
["git", "rev-parse", "HEAD"],
cwd=str(workspace_path),
capture_output=True,
text=True,
timeout=10,
)
if result.returncode != 0:
return None
return result.stdout.strip() or None
except (FileNotFoundError, OSError, subprocess.TimeoutExpired):
return None

def _extract_blocker_question(self, output: str) -> str:
"""Extract a meaningful blocker question from output."""
lines = [line.strip() for line in output.splitlines() if line.strip()]
Expand Down
73 changes: 69 additions & 4 deletions tests/core/adapters/test_claude_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,15 @@ class TestClaudeCodeAdapter:

@pytest.fixture(autouse=True)
def _no_git(self):
"""Prevent _detect_modified_files from calling real git."""
with patch.object(ClaudeCodeAdapter, "_detect_modified_files", return_value=[]):
"""Prevent git introspection from calling real git / the patched Popen.

_git_head -> None simulates a non-git workspace (guard won't fire);
tests that need the guard override it with an explicit sha.
"""
with (
patch.object(ClaudeCodeAdapter, "_detect_modified_files", return_value=[]),
patch.object(ClaudeCodeAdapter, "_git_head", return_value=None),
):
yield

def test_name(self) -> None:
Expand All @@ -40,17 +47,23 @@ def test_build_command_includes_print_flag(self) -> None:
assert cmd[0] == "/usr/bin/claude"
assert "--print" in cmd

def test_build_command_without_allowlist(self) -> None:
def test_build_command_without_allowlist_grants_permissions(self) -> None:
"""Default (no allowlist) must pass a permission config so --print mode
does not silently deny Edit/Write/Bash. (#739)"""
with patch("shutil.which", return_value="/usr/bin/claude"):
adapter = ClaudeCodeAdapter()
cmd = adapter.build_command("prompt", Path("/tmp"))
assert "--allowedTools" not in cmd
idx = cmd.index("--permission-mode")
assert cmd[idx + 1] == "bypassPermissions"

def test_build_command_with_allowlist(self) -> None:
with patch("shutil.which", return_value="/usr/bin/claude"):
adapter = ClaudeCodeAdapter(allowlist=["Edit", "Write"])
cmd = adapter.build_command("prompt", Path("/tmp"))
assert "--allowedTools" in cmd
# Explicit allowlist path keeps its own permissions — no bypass mode.
assert "--permission-mode" not in cmd
# Verify both tools are present after their respective flags
idx_edit = cmd.index("Edit")
idx_write = cmd.index("Write")
Expand All @@ -76,11 +89,63 @@ def test_successful_execution(self) -> None:
mock_process.returncode = 0
mock_process.wait.return_value = None

with patch("subprocess.Popen", return_value=mock_process):
with (
patch("subprocess.Popen", return_value=mock_process),
patch.object(
ClaudeCodeAdapter,
"_detect_modified_files",
return_value=["src/main.py"],
),
):
result = adapter.run("task-1", "fix the bug", Path("/tmp/repo"))

assert result.status == "completed"
assert "All tests pass" in result.output
assert result.modified_files == ["src/main.py"]

def test_zero_modified_files_is_failed(self) -> None:
"""A coding run that exits 0 but changes no files must fail, not
report a false completion. (#739)"""
with patch("shutil.which", return_value="/usr/bin/claude"):
adapter = ClaudeCodeAdapter()

mock_process = MagicMock()
mock_process.stdout = iter(["I analyzed the code but made no changes\n"])
mock_process.stderr = MagicMock()
mock_process.stderr.read.return_value = ""
mock_process.stdin = MagicMock()
mock_process.returncode = 0
mock_process.wait.return_value = None

# _no_git fixture already patches _detect_modified_files -> [].
# Stub HEAD to a stable sha so the guard sees a git repo with no new
# commit (before == after) and no working-tree changes.
with (
patch("subprocess.Popen", return_value=mock_process),
patch.object(ClaudeCodeAdapter, "_git_head", return_value="sha1"),
):
result = adapter.run("task-1", "fix the bug", Path("/tmp/repo"))

assert result.status == "failed"
assert "modified no files" in result.error

def test_require_file_changes_can_be_disabled(self) -> None:
"""Opting out restores plain exit-code mapping for analysis-only runs."""
with patch("shutil.which", return_value="/usr/bin/claude"):
adapter = ClaudeCodeAdapter(require_file_changes=False)

mock_process = MagicMock()
mock_process.stdout = iter(["analysis complete\n"])
mock_process.stderr = MagicMock()
mock_process.stderr.read.return_value = ""
mock_process.stdin = MagicMock()
mock_process.returncode = 0
mock_process.wait.return_value = None

with patch("subprocess.Popen", return_value=mock_process):
result = adapter.run("task-1", "analyze", Path("/tmp/repo"))

assert result.status == "completed"

def test_failed_execution(self) -> None:
with patch("shutil.which", return_value="/usr/bin/claude"):
Expand Down
73 changes: 73 additions & 0 deletions tests/core/adapters/test_subprocess_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,79 @@ def test_graceful_when_not_git_repo(self, adapter, tmp_path):
assert result.status == "completed"
assert result.modified_files == []

def _run_with(self, adapter, tmp_path, *, modified, head_before, head_after):
"""Run with _detect_modified_files and _git_head stubbed.

head_before/head_after become the two _git_head() return values (pre-run
baseline, then the guard's post-run check).
"""
mock_process = self._make_mock_process(stdout_lines=["done\n"], returncode=0)
with (
patch("subprocess.Popen", return_value=mock_process),
patch.object(
type(adapter), "_detect_modified_files", return_value=modified
),
patch.object(
type(adapter), "_git_head", side_effect=[head_before, head_after]
),
):
return adapter.run("task-1", "fix", tmp_path)

def test_require_file_changes_fails_on_empty_diff(self, tmp_path):
"""require_file_changes=True: exit 0, no changed files, HEAD unmoved -> failed. (#739)"""
with patch("shutil.which", return_value="/usr/bin/test-agent"):
adapter = SubprocessAdapter("test-agent", require_file_changes=True)
result = self._run_with(
adapter, tmp_path, modified=[], head_before="sha1", head_after="sha1"
)
assert result.status == "failed"
assert "modified no files" in result.error

def test_require_file_changes_passes_when_files_changed(self, tmp_path):
"""A run that changed working-tree files stays completed."""
with patch("shutil.which", return_value="/usr/bin/test-agent"):
adapter = SubprocessAdapter("test-agent", require_file_changes=True)
result = self._run_with(
adapter,
tmp_path,
modified=["src/main.py"],
head_before="sha1",
head_after="sha1",
)
assert result.status == "completed"
assert result.modified_files == ["src/main.py"]

def test_require_file_changes_accepts_self_committed_work(self, tmp_path):
"""An agent that commits its own work (HEAD advances) is not a false
completion even though `git diff HEAD` is empty. (#739 review)"""
with patch("shutil.which", return_value="/usr/bin/test-agent"):
adapter = SubprocessAdapter("test-agent", require_file_changes=True)
result = self._run_with(
adapter, tmp_path, modified=[], head_before="sha1", head_after="sha2"
)
assert result.status == "completed"

def test_require_file_changes_fails_when_pre_run_head_unavailable(self, tmp_path):
"""A transient pre-run HEAD read failure must not let `None != sha` fake
a commit and pass a real zero-file run. Fail safe. (#739 re-review)"""
with patch("shutil.which", return_value="/usr/bin/test-agent"):
adapter = SubprocessAdapter("test-agent", require_file_changes=True)
# head_before=None (pre-run read failed), head_after="sha1" (post succeeds).
result = self._run_with(
adapter, tmp_path, modified=[], head_before=None, head_after="sha1"
)
assert result.status == "failed"

def test_require_file_changes_skips_non_git_workspace(self, tmp_path):
"""Outside a git repo, HEAD is unresolvable and we cannot judge work —
do not fail the run. (#739 review)"""
with patch("shutil.which", return_value="/usr/bin/test-agent"):
adapter = SubprocessAdapter("test-agent", require_file_changes=True)
result = self._run_with(
adapter, tmp_path, modified=[], head_before=None, head_after=None
)
assert result.status == "completed"

def test_graceful_when_git_fails(self, adapter, tmp_path):
"""Should return empty modified_files if git diff fails."""
mock_process = self._make_mock_process(
Expand Down
Loading