kin: auto-commit after pipeline
This commit is contained in:
parent
95ba853b49
commit
24996b3974
4 changed files with 77 additions and 11 deletions
|
|
@ -259,10 +259,9 @@ def _run_claude(
|
||||||
"--output-format", "json",
|
"--output-format", "json",
|
||||||
"--model", model,
|
"--model", model,
|
||||||
]
|
]
|
||||||
if allow_write:
|
|
||||||
cmd.append("--dangerously-skip-permissions")
|
|
||||||
|
|
||||||
is_noninteractive = noninteractive or os.environ.get("KIN_NONINTERACTIVE") == "1"
|
is_noninteractive = noninteractive or os.environ.get("KIN_NONINTERACTIVE") == "1"
|
||||||
|
if allow_write or noninteractive:
|
||||||
|
cmd.append("--dangerously-skip-permissions")
|
||||||
if timeout is None:
|
if timeout is None:
|
||||||
env_timeout = os.environ.get("KIN_AGENT_TIMEOUT")
|
env_timeout = os.environ.get("KIN_AGENT_TIMEOUT")
|
||||||
if env_timeout:
|
if env_timeout:
|
||||||
|
|
@ -824,6 +823,7 @@ def _is_test_failure(result: dict) -> bool:
|
||||||
# Roles that trigger auto-test when project.auto_test_enabled is set
|
# Roles that trigger auto-test when project.auto_test_enabled is set
|
||||||
_AUTO_TEST_ROLES = {"backend_dev", "frontend_dev"}
|
_AUTO_TEST_ROLES = {"backend_dev", "frontend_dev"}
|
||||||
_WORKTREE_ROLES = {"backend_dev", "frontend_dev", "debugger"}
|
_WORKTREE_ROLES = {"backend_dev", "frontend_dev", "debugger"}
|
||||||
|
_DEV_GUARD_ROLES = {"backend_dev", "frontend_dev", "debugger"}
|
||||||
|
|
||||||
|
|
||||||
def _detect_test_command(project_path: str) -> str | None:
|
def _detect_test_command(project_path: str) -> str | None:
|
||||||
|
|
@ -1632,6 +1632,7 @@ def run_pipeline(
|
||||||
dry_run=False,
|
dry_run=False,
|
||||||
allow_write=True,
|
allow_write=True,
|
||||||
noninteractive=noninteractive,
|
noninteractive=noninteractive,
|
||||||
|
working_dir_override=worktree_path,
|
||||||
)
|
)
|
||||||
allow_write = True # subsequent steps also with allow_write
|
allow_write = True # subsequent steps also with allow_write
|
||||||
total_cost += retry.get("cost_usd") or 0
|
total_cost += retry.get("cost_usd") or 0
|
||||||
|
|
@ -1712,6 +1713,21 @@ def run_pipeline(
|
||||||
"total_duration_seconds": total_duration,
|
"total_duration_seconds": total_duration,
|
||||||
"pipeline_id": pipeline["id"] if pipeline else None,
|
"pipeline_id": pipeline["id"] if pipeline else None,
|
||||||
}
|
}
|
||||||
|
# Fix D: suspicious guard — empty merged_files means agent made no changes
|
||||||
|
if not merge_result.get("merged_files"):
|
||||||
|
result["suspicious"] = True
|
||||||
|
result["suspicious_reason"] = "agent reported success but no file changes detected"
|
||||||
|
_logger.warning("KIN-117: suspicious step %s — no merged files after worktree merge", role)
|
||||||
|
try:
|
||||||
|
if pipeline:
|
||||||
|
models.write_log(
|
||||||
|
conn, pipeline["id"],
|
||||||
|
f"Step {i+1}/{len(steps)} suspicious: {role} reported success but merged no files",
|
||||||
|
level="WARN",
|
||||||
|
extra={"role": role},
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
cleanup_worktree(worktree_path, p_path)
|
cleanup_worktree(worktree_path, p_path)
|
||||||
except Exception:
|
except Exception:
|
||||||
pass # Worktree errors must never block pipeline
|
pass # Worktree errors must never block pipeline
|
||||||
|
|
@ -2028,6 +2044,20 @@ def run_pipeline(
|
||||||
if p_path.is_dir():
|
if p_path.is_dir():
|
||||||
changed_files = _get_changed_files(str(p_path))
|
changed_files = _get_changed_files(str(p_path))
|
||||||
|
|
||||||
|
# Fix D: retroactive suspicious guard — if no files changed after pipeline,
|
||||||
|
# mark successful dev-agent steps as suspicious (agent may have not written files)
|
||||||
|
if changed_files is not None and not changed_files:
|
||||||
|
for _r in results:
|
||||||
|
if (_r.get("role") in _DEV_GUARD_ROLES
|
||||||
|
and _r.get("success")
|
||||||
|
and not _r.get("suspicious")):
|
||||||
|
_r["suspicious"] = True
|
||||||
|
_r["suspicious_reason"] = "agent reported success but no file changes detected"
|
||||||
|
_logger.warning(
|
||||||
|
"KIN-117: suspicious step %s — no file changes after pipeline",
|
||||||
|
_r.get("role"),
|
||||||
|
)
|
||||||
|
|
||||||
last_role = steps[-1].get("role", "") if steps else ""
|
last_role = steps[-1].get("role", "") if steps else ""
|
||||||
# For dept pipelines: if last step is a _head, check the last worker in its sub-pipeline
|
# For dept pipelines: if last step is a _head, check the last worker in its sub-pipeline
|
||||||
effective_last_role = _last_sub_role if (_is_department_head(last_role) and _last_sub_role) else last_role
|
effective_last_role = _last_sub_role if (_is_department_head(last_role) and _last_sub_role) else last_role
|
||||||
|
|
|
||||||
|
|
@ -71,15 +71,34 @@ def merge_worktree(worktree_path: str, project_path: str) -> dict:
|
||||||
branch_name = Path(worktree_path).name
|
branch_name = Path(worktree_path).name
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
# Stage all changes in the worktree before merge
|
||||||
|
subprocess.run(
|
||||||
|
[git, "-C", worktree_path, "add", "-A"],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
timeout=30,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Commit staged changes; ignore failure (nothing to commit = returncode 1)
|
||||||
|
commit_result = subprocess.run(
|
||||||
|
[git, "-C", worktree_path, "commit", "-m", f"kin: {branch_name}"],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
timeout=30,
|
||||||
|
)
|
||||||
|
commit_had_changes = commit_result.returncode == 0
|
||||||
|
|
||||||
merge_result = subprocess.run(
|
merge_result = subprocess.run(
|
||||||
[git, "-C", project_path, "merge", "--no-ff", branch_name],
|
[git, "merge", "--no-ff", branch_name],
|
||||||
|
cwd=project_path,
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
timeout=60,
|
timeout=60,
|
||||||
)
|
)
|
||||||
if merge_result.returncode == 0:
|
if merge_result.returncode == 0:
|
||||||
diff_result = subprocess.run(
|
diff_result = subprocess.run(
|
||||||
[git, "-C", project_path, "diff", "HEAD~1", "HEAD", "--name-only"],
|
[git, "diff", "HEAD~1", "HEAD", "--name-only"],
|
||||||
|
cwd=project_path,
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
timeout=10,
|
timeout=10,
|
||||||
|
|
@ -90,9 +109,15 @@ def merge_worktree(worktree_path: str, project_path: str) -> dict:
|
||||||
_logger.info("Merged worktree %s: %d files", branch_name, len(merged_files))
|
_logger.info("Merged worktree %s: %d files", branch_name, len(merged_files))
|
||||||
return {"success": True, "conflicts": [], "merged_files": merged_files}
|
return {"success": True, "conflicts": [], "merged_files": merged_files}
|
||||||
|
|
||||||
|
# Merge failed — if commit was also empty (nothing to commit), treat as success
|
||||||
|
if not commit_had_changes:
|
||||||
|
_logger.info("Worktree %s: nothing to commit, skipping merge", branch_name)
|
||||||
|
return {"success": True, "conflicts": [], "merged_files": []}
|
||||||
|
|
||||||
# Merge failed — collect conflicts and abort
|
# Merge failed — collect conflicts and abort
|
||||||
conflict_result = subprocess.run(
|
conflict_result = subprocess.run(
|
||||||
[git, "-C", project_path, "diff", "--name-only", "--diff-filter=U"],
|
[git, "diff", "--name-only", "--diff-filter=U"],
|
||||||
|
cwd=project_path,
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
timeout=10,
|
timeout=10,
|
||||||
|
|
@ -100,7 +125,8 @@ def merge_worktree(worktree_path: str, project_path: str) -> dict:
|
||||||
conflicts = [f.strip() for f in conflict_result.stdout.splitlines() if f.strip()]
|
conflicts = [f.strip() for f in conflict_result.stdout.splitlines() if f.strip()]
|
||||||
|
|
||||||
subprocess.run(
|
subprocess.run(
|
||||||
[git, "-C", project_path, "merge", "--abort"],
|
[git, "merge", "--abort"],
|
||||||
|
cwd=project_path,
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
timeout=10,
|
timeout=10,
|
||||||
)
|
)
|
||||||
|
|
|
||||||
|
|
@ -507,7 +507,11 @@ class TestMergeWorktree:
|
||||||
merge_ok = MagicMock(returncode=0, stdout="", stderr="")
|
merge_ok = MagicMock(returncode=0, stdout="", stderr="")
|
||||||
diff_ok = MagicMock(returncode=0, stdout="src/api.py\nsrc/models.py\n", stderr="")
|
diff_ok = MagicMock(returncode=0, stdout="src/api.py\nsrc/models.py\n", stderr="")
|
||||||
|
|
||||||
with patch("core.worktree.subprocess.run", side_effect=[merge_ok, diff_ok]):
|
add_ok = MagicMock(returncode=0, stdout="", stderr="")
|
||||||
|
commit_ok = MagicMock(returncode=0, stdout="", stderr="")
|
||||||
|
# Fix B adds git add -A + git commit before merge
|
||||||
|
with patch("core.worktree.subprocess.run",
|
||||||
|
side_effect=[add_ok, commit_ok, merge_ok, diff_ok]):
|
||||||
result = merge_worktree(worktree, str(tmp_path))
|
result = merge_worktree(worktree, str(tmp_path))
|
||||||
|
|
||||||
assert result["success"] is True
|
assert result["success"] is True
|
||||||
|
|
@ -523,8 +527,11 @@ class TestMergeWorktree:
|
||||||
conflict_files = MagicMock(returncode=0, stdout="src/models.py\n", stderr="")
|
conflict_files = MagicMock(returncode=0, stdout="src/models.py\n", stderr="")
|
||||||
abort = MagicMock(returncode=0)
|
abort = MagicMock(returncode=0)
|
||||||
|
|
||||||
|
add_ok = MagicMock(returncode=0, stdout="", stderr="")
|
||||||
|
commit_ok = MagicMock(returncode=0, stdout="", stderr="")
|
||||||
|
# Fix B adds git add -A + git commit before merge
|
||||||
with patch("core.worktree.subprocess.run",
|
with patch("core.worktree.subprocess.run",
|
||||||
side_effect=[merge_fail, conflict_files, abort]):
|
side_effect=[add_ok, commit_ok, merge_fail, conflict_files, abort]):
|
||||||
result = merge_worktree(worktree, str(tmp_path))
|
result = merge_worktree(worktree, str(tmp_path))
|
||||||
|
|
||||||
assert result["success"] is False
|
assert result["success"] is False
|
||||||
|
|
|
||||||
|
|
@ -144,8 +144,10 @@ class TestMergeWorktree:
|
||||||
worktree_path = tmp_path / ".kin_worktrees" / "WT1-001-debugger"
|
worktree_path = tmp_path / ".kin_worktrees" / "WT1-001-debugger"
|
||||||
worktree_path.mkdir(parents=True)
|
worktree_path.mkdir(parents=True)
|
||||||
|
|
||||||
# First call: merge succeeds; second: diff
|
# Fix B adds git add -A + git commit before merge
|
||||||
mock_run.side_effect = [
|
mock_run.side_effect = [
|
||||||
|
_ok_run(), # git add -A
|
||||||
|
_ok_run(), # git commit -m
|
||||||
_ok_run(), # git merge --no-ff
|
_ok_run(), # git merge --no-ff
|
||||||
_ok_run(), # git diff HEAD~1 HEAD --name-only
|
_ok_run(), # git diff HEAD~1 HEAD --name-only
|
||||||
]
|
]
|
||||||
|
|
@ -172,7 +174,8 @@ class TestMergeWorktree:
|
||||||
|
|
||||||
merge_fail = _fail_run("CONFLICT (content)")
|
merge_fail = _fail_run("CONFLICT (content)")
|
||||||
|
|
||||||
mock_run.side_effect = [merge_fail, conflict_diff, abort_result]
|
# Fix B adds git add -A + git commit before merge
|
||||||
|
mock_run.side_effect = [_ok_run(), _ok_run(), merge_fail, conflict_diff, abort_result]
|
||||||
|
|
||||||
result = merge_worktree(str(worktree_path), str(tmp_path))
|
result = merge_worktree(str(worktree_path), str(tmp_path))
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue