From b9746bfa320cdd1464eb6c91fbadad35c84d2459 Mon Sep 17 00:00:00 2001 From: Andrew Branson Date: Sun, 9 Aug 2026 15:03:34 +0200 Subject: Harden build workflows and OBS selection Add preflight, cancellation, confined status, and timeout handling for local Sailfish and remote Android build jobs. Vendor helper 2.0.0 with explicit backend/pull controls, build locking, local RPM validation, metadata, and structured failure reporting. Expose named internal, partner, and community OBS servers while retaining raw osc API aliases. --- tests/test_build_jobs.py | 318 +++++++++++++++++++++++++++++++++++++++++++++++ tests/test_server.py | 83 ++++++++++++- 2 files changed, 400 insertions(+), 1 deletion(-) create mode 100644 tests/test_build_jobs.py (limited to 'tests') diff --git a/tests/test_build_jobs.py b/tests/test_build_jobs.py new file mode 100644 index 0000000..bec80b2 --- /dev/null +++ b/tests/test_build_jobs.py @@ -0,0 +1,318 @@ +from __future__ import annotations + +from concurrent.futures import ThreadPoolExecutor +import base64 +import json +import os +from pathlib import Path +import re +import subprocess +import sys +import tempfile +import time +import unittest +from unittest.mock import patch + +from sailfish_devel_mcp.config import AndroidBuildHostConfig, Config, DeviceConfig, PathConfig +from sailfish_devel_mcp.runner import CommandResult +from sailfish_devel_mcp import tools +from sailfish_devel_mcp.vendor import build_sailfishos + + +class BuildJobTests(unittest.TestCase): + def make_config(self, root: Path) -> Config: + return Config( + path=None, + default_device="test", + devices={"test": DeviceConfig(name="test", ssh_target="root@test")}, + paths=PathConfig( + git_root=root, + obs_root=root / "OBS", + ssh_config=root / "ssh_config", + build_sailfishos=root / "build_sailfishos.py", + ), + ) + + def wait_for_terminal(self, status_path: Path, timeout: float = 5.0) -> dict[str, object]: + deadline = time.monotonic() + timeout + status: dict[str, object] = {} + while time.monotonic() < deadline: + status = tools._read_job_status(status_path) or {} + if tools._job_is_terminal(status): + return status + time.sleep(0.05) + self.fail(f"job did not finish: {status}") + + def test_status_rejects_path_traversal(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + config = self.make_config(root) + with patch.dict(os.environ, {"SAILFISH_DEVEL_MCP_STATE_DIR": str(root / "state")}): + with self.assertRaisesRegex(ValueError, "unsupported characters"): + tools.handle_build_status(config, {"job_id": "../outside"}) + + def test_status_uses_confined_log_path(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + state = root / "state" + job_dir = state / "builds" / "build-safe" + job_dir.mkdir(parents=True) + external = root / "secret.log" + external.write_text("secret\n", encoding="utf-8") + (job_dir / "build.log").write_text("safe\n", encoding="utf-8") + tools._write_json_atomic( + job_dir / "status.json", + { + "job_id": "build-safe", + "state": "finished", + "returncode": 0, + "log_path": str(external), + }, + ) + with patch.dict(os.environ, {"SAILFISH_DEVEL_MCP_STATE_DIR": str(state)}): + response = tools.handle_build_status(self.make_config(root), {"job_id": "build-safe"}) + + self.assertIn("safe", response["structuredContent"]["log_tail"]) + self.assertNotIn("secret", response["structuredContent"]["log_tail"]) + + def test_atomic_status_writes_have_unique_temporaries(self): + with tempfile.TemporaryDirectory() as temporary: + path = Path(temporary) / "status.json" + with ThreadPoolExecutor(max_workers=8) as executor: + list(executor.map(lambda value: tools._write_json_atomic(path, {"value": value}), range(80))) + payload = json.loads(path.read_text(encoding="utf-8")) + leftovers = list(path.parent.glob(".status.json.*.tmp")) + + self.assertIn(payload["value"], range(80)) + self.assertEqual(leftovers, []) + + def test_background_job_records_metadata_and_finishes(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + metadata = root / "last-build.json" + with patch.dict(os.environ, {"SAILFISH_DEVEL_MCP_STATE_DIR": str(root / "state")}): + job = tools._start_background_command( + "test build", + [ + sys.executable, + "-c", + "from pathlib import Path; import sys; Path(sys.argv[1]).write_text(sys.argv[2])", + str(metadata), + json.dumps({"rpms": ["/tmp/example.rpm"]}), + ], + timeout=5, + metadata_path=metadata, + ) + status = self.wait_for_terminal(Path(job["status_path"])) + + self.assertEqual(status["state"], "finished") + self.assertEqual(status["returncode"], 0) + self.assertEqual(status["artifacts"], ["/tmp/example.rpm"]) + + def test_background_job_start_failure_becomes_terminal(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + metadata = root / "last-build.json" + metadata.write_text(json.dumps({"rpms": ["/tmp/stale.rpm"]}), encoding="utf-8") + with patch.dict(os.environ, {"SAILFISH_DEVEL_MCP_STATE_DIR": str(root / "state")}): + job = tools._start_background_command( + "broken build", + [str(root / "does-not-exist")], + timeout=5, + metadata_path=metadata, + ) + status = self.wait_for_terminal(Path(job["status_path"])) + + self.assertEqual(status["state"], "failed") + self.assertEqual(status["failure_class"], "supervisor") + self.assertNotIn("artifacts", status) + + def test_job_list_reconciles_lost_supervisor(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + state = root / "state" + job_dir = state / "builds" / "build-lost" + job_dir.mkdir(parents=True) + tools._write_json_atomic( + job_dir / "status.json", + {"job_id": "build-lost", "state": "running", "supervisor_pid": 99999999}, + ) + with patch.dict(os.environ, {"SAILFISH_DEVEL_MCP_STATE_DIR": str(state)}): + response = tools.handle_build_status(self.make_config(root), {}) + + self.assertEqual(response["structuredContent"]["jobs"][0]["state"], "failed") + self.assertEqual( + response["structuredContent"]["jobs"][0]["failure_class"], + "supervisor-lost", + ) + + def test_background_job_can_be_cancelled(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + state = root / "state" + config = self.make_config(root) + with patch.dict(os.environ, {"SAILFISH_DEVEL_MCP_STATE_DIR": str(state)}): + job = tools._start_background_command("slow build", ["/bin/sleep", "30"], timeout=60) + status_path = Path(job["status_path"]) + deadline = time.monotonic() + 3 + while time.monotonic() < deadline: + status = tools._read_job_status(status_path) or {} + if status.get("state") == "running": + break + time.sleep(0.05) + response = tools.handle_build_cancel(config, {"job_id": job["job_id"]}) + status = self.wait_for_terminal(status_path) + + self.assertFalse(response["isError"]) + self.assertEqual(status["state"], "cancelled") + self.assertTrue(status["cancelled"]) + + def test_build_command_exposes_helper_controls(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + (root / "rpm").mkdir() + (root / "rpm" / "sample.spec").write_text("Name: sample\n", encoding="utf-8") + (root / "build_sailfishos.py").write_text("# helper\n", encoding="utf-8") + command, project = tools._sailfish_build_command( + self.make_config(root), + { + "project_path": str(root), + "backend": "local", + "local_sdk": "/srv/mer/sdks/sfossdk/sdk-chroot", + "target": "aarch64-devel", + "pull_policy": "missing", + "no_vcs_apply": True, + "allow_untrusted_rpms": True, + }, + dry_run=True, + ) + + self.assertEqual(project, root) + self.assertIn("--backend", command) + self.assertIn("/srv/mer/sdks/sfossdk/sdk-chroot", command) + self.assertIn("aarch64-devel", command) + self.assertIn("missing", command) + self.assertIn("--no-vcs-apply", command) + self.assertIn("--allow-untrusted-rpms", command) + self.assertEqual(command[-2:], ["--dry-run", "--json"]) + + def test_build_command_rejects_invalid_arch_shape(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + (root / "build_sailfishos.py").write_text("# helper\n", encoding="utf-8") + with self.assertRaisesRegex(ValueError, "arch must be"): + tools._sailfish_build_command( + self.make_config(root), + {"project_path": str(root), "arch": ["aarch64", 7]}, + ) + + def test_build_command_rejects_untrusted_local_sdk_mount(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + (root / "build_sailfishos.py").write_text("# helper\n", encoding="utf-8") + with self.assertRaisesRegex(ValueError, "under /srv/mer"): + tools._sailfish_build_command( + self.make_config(root), + { + "project_path": str(root), + "backend": "local", + "local_sdk": "/etc/passwd", + }, + ) + + def test_preflight_returns_parsed_plan(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + (root / "rpm").mkdir() + (root / "rpm" / "sample.spec").write_text("Name: sample\n", encoding="utf-8") + (root / "build_sailfishos.py").write_text("# helper\n", encoding="utf-8") + plan = {"backend": "docker", "mutates_project": False} + result = CommandResult(("python3",), 0, json.dumps(plan), "") + with patch("sailfish_devel_mcp.tools.run", return_value=result): + response = tools.handle_build_preflight( + self.make_config(root), + {"project_path": str(root), "release": "5.0.0.43", "arch": "aarch64"}, + ) + + self.assertFalse(response["isError"]) + self.assertEqual(response["structuredContent"]["plan"], plan) + + def test_android_job_scripts_are_atomic_and_identity_checked(self): + host = AndroidBuildHostConfig( + name="builder", + ssh_target="builder@example", + project_dir="/src/android", + state_dir="/tmp/builds", + ) + start = tools._android_build_start_command( + host=host, + project_dir=host.project_dir, + state_dir=host.state_dir, + job_id="android-test", + shell="bash", + shell_command="m services", + build_timeout=3600, + ) + cancel = tools._android_build_cancel_command(host.state_dir, "android-test") + encoded = re.search(r" ([A-Za-z0-9+/=]+) \| base64 -d", start) + self.assertIsNotNone(encoded) + run_script = base64.b64decode(encoded.group(1)).decode("utf-8") + + self.assertIn('if ! mkdir "$job_dir"', start) + self.assertNotIn('mkdir -p "$state_dir" "$job_dir"', start) + self.assertIn("nohup setsid", start) + self.assertIn("process_start", start) + self.assertIn("build_timeout=3600", run_script) + self.assertIn("android-build-watchdog", run_script) + self.assertIn('kill -KILL -"$2"', run_script) + self.assertIn("expected_start", cancel) + self.assertIn('kill -TERM -"$pid"', cancel) + + def test_vendored_helper_has_versioned_interface(self): + self.assertEqual(build_sailfishos.HELPER_VERSION, "2.0.0") + args = build_sailfishos.parse_args( + ["--backend", "docker", "--pull-policy", "never", "--dry-run", "--json"] + ) + self.assertEqual(args.backend, "docker") + self.assertEqual(args.pull_policy, "never") + + def test_android_timeout_terminates_remote_process_group(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + project = root / "project" + state = root / "state" + project.mkdir() + host = AndroidBuildHostConfig( + name="local-test", + ssh_target="unused", + project_dir=str(project), + state_dir=str(state), + ) + start = tools._android_build_start_command( + host=host, + project_dir=str(project), + state_dir=str(state), + job_id="android-timeout", + shell="sh", + shell_command="sleep 30", + build_timeout=1, + ) + started = subprocess.run(["sh", "-c", start], check=False, capture_output=True, text=True) + self.assertEqual(started.returncode, 0, started.stderr) + deadline = time.monotonic() + 5 + while time.monotonic() < deadline and not (state / "android-timeout" / "timed_out").exists(): + time.sleep(0.05) + timed_out = (state / "android-timeout" / "timed_out").exists() + pid = int((state / "android-timeout" / "pid").read_text(encoding="utf-8")) + expected_start = (state / "android-timeout" / "process_start").read_text(encoding="utf-8").strip() + actual_start = tools._process_start_time(pid) + while time.monotonic() < deadline and actual_start == expected_start: + time.sleep(0.05) + actual_start = tools._process_start_time(pid) + + self.assertTrue(timed_out) + self.assertNotEqual(actual_start, expected_start) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_server.py b/tests/test_server.py index 3a80e80..232d411 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -65,17 +65,26 @@ class McpServerTests(unittest.TestCase): response = server.handle( {"jsonrpc": "2.0", "id": 2, "method": "tools/list", "params": {}} ) - names = {tool["name"] for tool in response["result"]["tools"]} + tools = response["result"]["tools"] + names = {tool["name"] for tool in tools} self.assertIn("sailfish_device_topmost_pid", names) self.assertIn("sailfish_device_touch", names) self.assertIn("sailfish_device_touch_workflow", names) self.assertIn("sailfish_device_user_session_command", names) self.assertIn("sailfish_device_browser_launch", names) self.assertIn("sailfish_sdk_refresh_metadata", names) + self.assertIn("sailfish_build_preflight", names) + self.assertIn("sailfish_build_cancel", names) self.assertIn("sailfish_android_build_hosts", names) self.assertIn("sailfish_android_build", names) self.assertIn("sailfish_android_build_status", names) + self.assertIn("sailfish_android_build_cancel", names) self.assertIn("sailfish_qml_check_translator_ternaries", names) + obs_results = next(tool for tool in tools if tool["name"] == "sailfish_obs_results") + self.assertEqual( + obs_results["inputSchema"]["properties"]["server"]["enum"], + ["internal", "partner", "community"], + ) def test_qml_ternary_checker_reports_inline_ternary_qstrid(self) -> None: with tempfile.TemporaryDirectory() as tmp: @@ -608,6 +617,78 @@ class McpServerTests(unittest.TestCase): "/build/home%3Aexample/5.0.0/aarch64/browser/_log?nostream=1", ) + def test_obs_results_selects_named_servers(self) -> None: + aliases = { + "internal": "jolla", + "partner": "partner", + "community": "community", + } + with tempfile.TemporaryDirectory() as tmp: + mcp_server = self.make_server(Path(tmp)) + for named_server, api_alias in aliases.items(): + with self.subTest(server=named_server): + with patch("sailfish_devel_mcp.tools.run") as mocked_run: + mocked_run.return_value = CommandResult(("osc",), 0, "", "") + response = mcp_server.handle( + { + "jsonrpc": "2.0", + "id": 13, + "method": "tools/call", + "params": { + "name": "sailfish_obs_results", + "arguments": { + "project": "home:example", + "package": "browser", + "server": named_server, + }, + }, + } + ) + + self.assertFalse(response["result"].get("isError", False)) + argv = list(mocked_run.call_args.args[0]) + self.assertEqual( + argv, + [ + "osc", + "-A", + api_alias, + "results", + "home:example", + "browser", + ], + ) + structured = response["result"]["structuredContent"] + self.assertEqual(structured["server"], named_server) + self.assertEqual(structured["api_alias"], api_alias) + + def test_obs_results_rejects_server_with_api_alias(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + server = self.make_server(Path(tmp)) + with patch("sailfish_devel_mcp.tools.run") as mocked_run: + response = server.handle( + { + "jsonrpc": "2.0", + "id": 14, + "method": "tools/call", + "params": { + "name": "sailfish_obs_results", + "arguments": { + "project": "home:example", + "server": "partner", + "api_alias": "community", + }, + }, + } + ) + + self.assertTrue(response["result"].get("isError", False)) + self.assertIn( + "server and api_alias cannot be combined", + response["result"]["content"][0]["text"], + ) + mocked_run.assert_not_called() + def test_sdk_refresh_metadata_uses_local_sdk_main_target(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) -- cgit v1.2.3