Enforce canonical setup result contract in CLI and GUI
This commit is contained in:
@@ -29,6 +29,20 @@ from cmdforge.gui.dialogs.provider_install_dialog import (
|
||||
from cmdforge.providers import Provider, get_provider, load_providers, save_providers
|
||||
|
||||
|
||||
def _write_completion(path, data):
|
||||
payload = {
|
||||
"schema_version": 1, "helper_version": "0.1.0",
|
||||
"timestamp": "2026-09-14T00:00:00Z", "harness_id": "claude-code",
|
||||
"auth_status": "unknown", "path_updates": [], "manual_steps_remaining": [],
|
||||
"partial_changes": {"observed_created": [], "observed_modified": [], "unknown_changes": True},
|
||||
"installer_exit_code": 0,
|
||||
"recipe_id": "test-recipe", "recipe_version": "2026-09-10", "install_method": "argv",
|
||||
}
|
||||
payload.update(data)
|
||||
path.write_text(json.dumps(payload), encoding="utf-8")
|
||||
path.chmod(0o600)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def temp_providers_file(tmp_path):
|
||||
providers_file = tmp_path / ".cmdforge" / "providers.yaml"
|
||||
@@ -141,6 +155,13 @@ def test_setup_terminal_worker_success(tmp_path):
|
||||
result_file = tmp_path / "result.json"
|
||||
req_id = "test-uuid-1234"
|
||||
data = {
|
||||
"schema_version": 1,
|
||||
"helper_version": "0.1.0",
|
||||
"timestamp": "2026-09-14T00:00:00Z",
|
||||
"auth_status": "unknown",
|
||||
"manual_steps_remaining": [],
|
||||
"partial_changes": {"observed_created": [], "observed_modified": [], "unknown_changes": True},
|
||||
"verification": {"verified": True, "executable_path": "/custom/bin/claude", "version_output": "test"},
|
||||
"status": "success",
|
||||
"request_id": req_id,
|
||||
"harness_id": "claude-code",
|
||||
@@ -150,7 +171,7 @@ def test_setup_terminal_worker_success(tmp_path):
|
||||
class MockProcess:
|
||||
def __init__(self, *args, **kwargs):
|
||||
self.stdout = None
|
||||
result_file.write_text(json.dumps(data), encoding="utf-8")
|
||||
_write_completion(result_file, data)
|
||||
|
||||
def wait(self):
|
||||
return 0
|
||||
@@ -179,6 +200,26 @@ def test_setup_terminal_worker_success(tmp_path):
|
||||
assert res_dict["path_updates"] == ["/custom/bin"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("overrides", [
|
||||
{"schema_version": True}, {"unexpected": 1}, {"status": []},
|
||||
{"partial_changes": None}, {"cleanup_complete": "true"},
|
||||
])
|
||||
def test_worker_rejects_malformed_contract(tmp_path, overrides):
|
||||
path = tmp_path / "result.json"
|
||||
data = {"status": "failed", "request_id": "req-1"}
|
||||
data.update(overrides)
|
||||
_write_completion(path, data)
|
||||
process = MagicMock(stdout=None)
|
||||
process.wait.return_value = 1
|
||||
worker = SetupTerminalWorker(["fake-helper"], "claude-code", path, "req-1")
|
||||
outcomes = []
|
||||
worker.finished.connect(lambda *args: outcomes.append(args))
|
||||
with patch("subprocess.Popen", return_value=process):
|
||||
worker.run()
|
||||
assert len(outcomes) == 1 and outcomes[0][0] is False
|
||||
assert "unknown" in outcomes[0][1]
|
||||
|
||||
|
||||
def test_setup_terminal_worker_security_mismatch(tmp_path):
|
||||
"""Test SetupTerminalWorker rejects result with request_id mismatch."""
|
||||
result_file = tmp_path / "result.json"
|
||||
@@ -191,7 +232,7 @@ def test_setup_terminal_worker_security_mismatch(tmp_path):
|
||||
class MockProcess:
|
||||
def __init__(self, *args, **kwargs):
|
||||
self.stdout = None
|
||||
result_file.write_text(json.dumps(data), encoding="utf-8")
|
||||
_write_completion(result_file, data)
|
||||
|
||||
def wait(self):
|
||||
return 0
|
||||
@@ -212,7 +253,7 @@ def test_setup_terminal_worker_security_mismatch(tmp_path):
|
||||
assert len(results) == 1
|
||||
success, msg, _ = results[0]
|
||||
assert success is False
|
||||
assert "Security error" in msg
|
||||
assert "does not match the expected" in msg
|
||||
|
||||
|
||||
def test_setup_terminal_worker_incoherent_exit(tmp_path):
|
||||
@@ -227,7 +268,7 @@ def test_setup_terminal_worker_incoherent_exit(tmp_path):
|
||||
class MockProcess:
|
||||
def __init__(self, *args, **kwargs):
|
||||
self.stdout = None
|
||||
result_file.write_text(json.dumps(data), encoding="utf-8")
|
||||
_write_completion(result_file, data)
|
||||
|
||||
def wait(self):
|
||||
return 1
|
||||
@@ -248,7 +289,7 @@ def test_setup_terminal_worker_incoherent_exit(tmp_path):
|
||||
assert len(results) == 1
|
||||
success, msg, _ = results[0]
|
||||
assert success is False
|
||||
assert "error code 1" in msg
|
||||
assert "exit code 1" in msg
|
||||
|
||||
|
||||
def test_setup_terminal_worker_cancelled(tmp_path):
|
||||
@@ -256,6 +297,14 @@ def test_setup_terminal_worker_cancelled(tmp_path):
|
||||
result_file = tmp_path / "result.json"
|
||||
req_id = "cancel-uuid"
|
||||
data = {
|
||||
"schema_version": 1,
|
||||
"helper_version": "0.1.0",
|
||||
"timestamp": "2026-09-14T00:00:00Z",
|
||||
"harness_id": "claude-code",
|
||||
"auth_status": "unknown",
|
||||
"path_updates": [],
|
||||
"manual_steps_remaining": [],
|
||||
"partial_changes": {"observed_created": [], "observed_modified": [], "unknown_changes": True},
|
||||
"status": "cancelled",
|
||||
"request_id": req_id,
|
||||
}
|
||||
@@ -263,7 +312,7 @@ def test_setup_terminal_worker_cancelled(tmp_path):
|
||||
class MockProcess:
|
||||
def __init__(self, *args, **kwargs):
|
||||
self.stdout = None
|
||||
result_file.write_text(json.dumps(data), encoding="utf-8")
|
||||
_write_completion(result_file, data)
|
||||
|
||||
def wait(self):
|
||||
return 10
|
||||
@@ -300,7 +349,7 @@ def test_setup_terminal_worker_failure_with_diagnostics(tmp_path):
|
||||
class MockProcess:
|
||||
def __init__(self, *args, **kwargs):
|
||||
self.stdout = None
|
||||
result_file.write_text(json.dumps(data), encoding="utf-8")
|
||||
_write_completion(result_file, data)
|
||||
|
||||
def wait(self):
|
||||
return 1
|
||||
|
||||
@@ -56,11 +56,28 @@ def _mock_helper_run(status: str, harness_id: str = "claude-code", exit_code: in
|
||||
"error_diagnostics": diagnostics,
|
||||
}
|
||||
result_file_path.write_text(json.dumps(data), encoding="utf-8")
|
||||
result_file_path.chmod(0o600)
|
||||
return SimpleNamespace(returncode=exit_code)
|
||||
|
||||
return _run
|
||||
|
||||
|
||||
@pytest.mark.parametrize("dry_run", [False, True])
|
||||
def test_sparse_success_cannot_register_provider(temp_providers_file, capsys, dry_run):
|
||||
def fake(cmd, *args, **kwargs):
|
||||
path = Path(cmd[cmd.index("--result-file") + 1])
|
||||
request_id = cmd[cmd.index("--request-id") + 1]
|
||||
path.write_text(json.dumps({"schema_version": 1, "status": "success", "request_id": request_id}))
|
||||
path.chmod(0o600)
|
||||
return SimpleNamespace(returncode=0)
|
||||
argv = ["cmdforge", "providers", "install"] + (["--dry-run"] if dry_run else [])
|
||||
with patch("cmdforge.cli.provider_commands._find_setup_helper", return_value=["fake"]), patch("subprocess.run", side_effect=fake), patch("sys.argv", argv), patch("builtins.input") as prompt:
|
||||
assert main() == 1
|
||||
prompt.assert_not_called()
|
||||
assert get_provider("claude") is None
|
||||
assert "unknown" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_helper_discovery_finds_executable():
|
||||
"""Verify _find_setup_helper locates agent-harness-setup."""
|
||||
with patch("shutil.which", return_value="/usr/local/bin/agent-harness-setup"):
|
||||
@@ -182,7 +199,7 @@ def test_providers_install_incoherent_result_rejected(temp_providers_file, capsy
|
||||
|
||||
assert exit_code == 1
|
||||
captured = capsys.readouterr()
|
||||
assert "Treating as failure" in captured.out
|
||||
assert "not coherent with exit code 1" in captured.out
|
||||
assert get_provider("claude") is None
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user