diff --git a/AGENTS.md b/AGENTS.md index c394f7c5b..44b192684 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -148,6 +148,7 @@ Prefer the developer daemon as the default way to build, run, and validate. Four - **Lane-serialized job queue** — every job claims named lanes (`build`, `debugArtifact`, `liveApp`, `release`, `style`); the daemon runs jobs that share a lane one at a time while letting unrelated lanes proceed concurrently. That serial queue is what stops multiple agents from building, launching, or running style tooling over each other and corrupting `.build` or the live app. - **Machine-wide heavy-job slots** — Swift/Xcode-heavy daemon jobs acquire per-user global heavy admission under `/tmp/repoprompt-ce-dev-locks-/`, independent of daemon socket overrides. The default capacity is 1; set `REPOPROMPT_DEV_HEAVY_SLOTS=N` explicitly to allow more concurrent heavy jobs. If `job status` or `job wait` shows `global-wait` or "waiting for global heavy slot", another checkout/worktree is already running heavy work; wait on the ticket instead of bypassing conductor with direct `swift`/`xcodebuild` commands or starting duplicate jobs. Holder metadata is display-only and identifies the operation/ticket/repo/worktree when available. +- **Opt-in bounded Swift parallelism** — by default SwiftPM uses the active processor count for build/test jobs. Set `REPOPROMPT_SWIFT_JOBS=N` (a positive decimal integer, no leading zeros or signs) to pass `--jobs N` to coordinated `swift build` and `swift test` invocations, including `make dev-build`, `make dev-swift-build`, `make dev-test`, `make dev-provider-test`, and `make dev-run`. The same value is honored by `Scripts/package_app.sh` and `Scripts/build_swiftpm_release_products.sh`. This is independent of `REPOPROMPT_DEV_HEAVY_SLOTS`; a per-job limit is not a machine-wide heavy slot. The requested/effective limit is part of the daemon env snapshot and `request-key` fingerprint, so jobs with different limits are not reused, and the limit is echoed in the command log and `Phases` summary. When unset, SwiftPM's default behavior is unchanged. - **Machine-wide live-app lock** — debug app stop/launch/relaunch/smoke-launch lifecycle work acquires a separate per-user `live-app.lock`, so worktree-local daemons cannot mutate the singleton debug app at the same time. - **Tickets + async jobs** — every job gets a ticket and can run detached (`--async`). Fire a build, keep working, and query or wait on it later (`job status` / `job wait`) instead of blocking on a long compile. Jobs survive reconnects and are reusable by `--request-key`. diff --git a/Scripts/build_swiftpm_release_products.sh b/Scripts/build_swiftpm_release_products.sh index 2fc8dedbe..e678a709e 100755 --- a/Scripts/build_swiftpm_release_products.sh +++ b/Scripts/build_swiftpm_release_products.sh @@ -82,6 +82,13 @@ SWIFT_BUILD_ARGS=(-c release) if sentry_linking_enabled; then SWIFT_BUILD_ARGS+=(-debug-info-format dwarf) fi +if [[ -n "${REPOPROMPT_SWIFT_JOBS:-}" ]]; then + if [[ "$REPOPROMPT_SWIFT_JOBS" =~ ^[1-9][0-9]*$ ]]; then + SWIFT_BUILD_ARGS+=("--jobs" "$REPOPROMPT_SWIFT_JOBS") + else + fail "REPOPROMPT_SWIFT_JOBS must be a positive integer" + fi +fi ARM64_BIN_DIR="" X86_64_BIN_DIR="" diff --git a/Scripts/conductor.py b/Scripts/conductor.py index e599407b7..5c22f8f71 100755 --- a/Scripts/conductor.py +++ b/Scripts/conductor.py @@ -174,6 +174,7 @@ socket default: /tmp/conductor-/.sock (directory mode 0700) machine locks: /tmp/repoprompt-ce-dev-locks-/ (directory mode 0700; independent of socket overrides) heavy slots: REPOPROMPT_DEV_HEAVY_SLOTS=N (default 1) + swift jobs: REPOPROMPT_SWIFT_JOBS=N (opt-in; when unset, SwiftPM uses the active processor count) overrides: REPOPROMPT_DEV_DAEMON_STATE_DIR, REPOPROMPT_DEV_DAEMON_SOCKET (socket parent must be owned 0700) Protocol version: {PROTOCOL_VERSION} @@ -411,6 +412,25 @@ def global_heavy_slot_paths(env: Optional[Dict[str, str]] = None) -> List[Path]: return [root / f"global-heavy-{index}.lock" for index in range(configured_global_heavy_slots(env))] +def configured_swift_jobs(env: Optional[Dict[str, str]] = None) -> Optional[int]: + source = env if env is not None else os.environ + raw = source.get("REPOPROMPT_SWIFT_JOBS") + if raw is None or raw == "": + return None + # Match the shell validation in package_app.sh and build_swiftpm_release_products.sh: + # a positive decimal integer with no leading zeros and no sign. + if not re.fullmatch(r"[1-9][0-9]*", raw): + raise ConductorError("REPOPROMPT_SWIFT_JOBS must be a positive integer") + return int(raw) + + +def append_swift_jobs_arg(argv: List[str], env: Optional[Dict[str, str]] = None) -> List[str]: + jobs = configured_swift_jobs(env) + if jobs is None: + return argv + return argv + ["--jobs", str(jobs)] + + def live_app_lock_path() -> Path: return machine_lock_dir() / "live-app.lock" @@ -1256,6 +1276,7 @@ class OperationRegistry: ] CONDUCTOR_ENV_KEYS = [ "REPOPROMPT_DEV_HEAVY_SLOTS", + "REPOPROMPT_SWIFT_JOBS", ] TELEMETRY_ENV_KEYS = [ "REPOPROMPT_ENABLE_SENTRY", @@ -1353,7 +1374,8 @@ def prepare(self, request: Dict[str, Any]) -> Tuple[List[str], List[str], Path, lanes = ["build"] if product == "all": return self._internal_argv("swift_build_all", {}), lanes, cwd, env, effective_timeout - return ["swift", "build", "--product", str(product)], lanes, cwd, env, effective_timeout + argv = ["swift", "build", "--product", str(product)] + return append_swift_jobs_arg(argv, env), lanes, cwd, env, effective_timeout if operation == "build": return [script("package_app.sh"), "debug"], ["build", "debugArtifact"], cwd, env, effective_timeout if operation == "package": @@ -1368,7 +1390,7 @@ def prepare(self, request: Dict[str, Any]) -> Tuple[List[str], List[str], Path, argv.append("list") elif args.get("filter"): argv.extend(["--filter", str(args["filter"])]) - return argv, ["build"], cwd, env, effective_timeout + return append_swift_jobs_arg(argv, env), ["build"], cwd, env, effective_timeout if operation == "provider-test": argv = ["swift", "test"] if args.get("testProduct"): @@ -1377,7 +1399,7 @@ def prepare(self, request: Dict[str, Any]) -> Tuple[List[str], List[str], Path, argv.append("list") elif args.get("filter"): argv.extend(["--filter", str(args["filter"])]) - return argv, ["build"], self.repo_root / "Packages" / "RepoPromptAgentProviders", env, effective_timeout + return append_swift_jobs_arg(argv, env), ["build"], self.repo_root / "Packages" / "RepoPromptAgentProviders", env, effective_timeout if operation == "install-debug-cli": return [script("install_debug_cli.sh"), "install", "--build"], ["build", "debugArtifact"], cwd, env, effective_timeout if operation == "debug-cli-status": @@ -2024,9 +2046,20 @@ def _run_job(self, ticket: str) -> None: global_heavy_slot = self._acquire_global_heavy_slot(job.ticket) if global_heavy_slot is None: return + # Only parse/validate the Swift jobs limit for SwiftPM-invoking operations. + # Packaging/build scripts read REPOPROMPT_SWIFT_JOBS themselves and must not + # be blocked here; sleep/diagnostics and other lanes must not be affected. + if job.operation in {"swift-build", "test", "provider-test"}: + swift_jobs_limit = configured_swift_jobs(env) + else: + swift_jobs_limit = None start_line = f"$ {format_argv(argv)}\n" with job.log_path.open("ab") as log_file: with self.lock: + if swift_jobs_limit is not None: + header_line = f"==> Swift jobs limit: {swift_jobs_limit}\n" + self._append_tail_locked(job, header_line) + log_file.write(header_line.encode("utf-8", errors="replace")) self._append_tail_locked(job, start_line) log_file.write(start_line.encode("utf-8", errors="replace")) log_file.flush() @@ -3704,9 +3737,12 @@ def run_operation_command( env: Optional[Dict[str, str]] = None, allow_exit_codes: Optional[set[int]] = None, timeout: Optional[float] = None, + swift_jobs_limit: Optional[int] = None, ) -> Tuple[int, str, str]: allowed = allow_exit_codes if allow_exit_codes is not None else {0} print(f"\n==> {name}", flush=True) + if swift_jobs_limit is not None: + print(f"==> Swift jobs limit: {swift_jobs_limit}", flush=True) print(f"$ {format_argv(argv)}", flush=True) try: completed = subprocess.run( @@ -4228,8 +4264,17 @@ def operation_app_stop(repo_root: Path, args: Dict[str, Any]) -> int: def operation_swift_build_all(repo_root: Path) -> int: + swift_jobs_limit = configured_swift_jobs(os.environ) for product in ["RepoPrompt", "repoprompt-mcp"]: - code, _stdout, _stderr = run_operation_command(f"swift build --product {product}", ["swift", "build", "--product", product], repo_root) + argv = ["swift", "build", "--product", product] + if swift_jobs_limit is not None: + argv.extend(["--jobs", str(swift_jobs_limit)]) + code, _stdout, _stderr = run_operation_command( + f"swift build --product {product}", + argv, + repo_root, + swift_jobs_limit=swift_jobs_limit, + ) if code != 0: return code return 0 diff --git a/Scripts/package_app.sh b/Scripts/package_app.sh index ada8d754b..508850102 100755 --- a/Scripts/package_app.sh +++ b/Scripts/package_app.sh @@ -212,6 +212,13 @@ SWIFT_BUILD_ARGS=(-c "$CONF") if sentry_linking_enabled; then SWIFT_BUILD_ARGS+=(-debug-info-format dwarf) fi +if [[ -n "${REPOPROMPT_SWIFT_JOBS:-}" ]]; then + if [[ "$REPOPROMPT_SWIFT_JOBS" =~ ^[1-9][0-9]*$ ]]; then + SWIFT_BUILD_ARGS+=("--jobs" "$REPOPROMPT_SWIFT_JOBS") + else + fail "REPOPROMPT_SWIFT_JOBS must be a positive integer" + fi +fi PUBLIC_UNIVERSAL_RELEASE=0 ARCHITECTURE_POLICY="matching" if (( IS_RELEASE )) && (( ! USE_LOCAL_SELF_SIGNED_RELEASE )); then diff --git a/Scripts/test_conductor_lifecycle.py b/Scripts/test_conductor_lifecycle.py index 5bde3f164..def009407 100644 --- a/Scripts/test_conductor_lifecycle.py +++ b/Scripts/test_conductor_lifecycle.py @@ -795,7 +795,10 @@ def wait_for_terminal_job(self, state: conductor.DaemonState, ticket: str, timeo while time.monotonic() < deadline: with state.condition: job = state.jobs[ticket] - if job.state in conductor.TERMINAL_STATES: + # A job becomes terminal before the runner's final persistence + # and lane-release work is complete. Wait for that finalizer so + # TemporaryDirectory cleanup cannot race its record write. + if job.state in conductor.TERMINAL_STATES and ticket not in state.active_lanes.values(): return job time.sleep(0.01) with state.condition: @@ -1769,6 +1772,213 @@ def test_test_cli_forwards_watchdog_options_and_requires_threshold(self) -> None conductor.handle_real_operation(state.paths, "test", ["--xctest-stall-wake-probe"]) +class SwiftJobsTests(LifecycleTestCase): + def test_swift_jobs_env_unset_uses_swiftpm_default(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + registry = conductor.OperationRegistry(Path(tmp)) + build_argv, _lanes, _cwd, _env, _timeout = registry.prepare( + {"operation": "swift-build", "args": {"product": "RepoPrompt"}} + ) + test_argv, _lanes, _cwd, _env, _timeout = registry.prepare( + {"operation": "test", "args": {"filter": "ExampleTests"}} + ) + provider_argv, _lanes, _cwd, _env, _timeout = registry.prepare( + {"operation": "provider-test", "args": {"list": True}} + ) + + self.assertEqual(build_argv, ["swift", "build", "--product", "RepoPrompt"]) + self.assertEqual(test_argv, ["swift", "test", "--filter", "ExampleTests"]) + self.assertEqual(provider_argv, ["swift", "test", "list"]) + + def test_request_env_is_authoritative(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + registry = conductor.OperationRegistry(Path(tmp)) + with mock.patch.dict(os.environ, {"REPOPROMPT_SWIFT_JOBS": "99"}, clear=False): + with_env = registry.prepare( + { + "operation": "swift-build", + "args": {"product": "RepoPrompt"}, + "env": {"REPOPROMPT_SWIFT_JOBS": "4"}, + } + ) + without_env = registry.prepare( + {"operation": "swift-build", "args": {"product": "RepoPrompt"}, "env": {}} + ) + empty_env = registry.prepare( + { + "operation": "swift-build", + "args": {"product": "RepoPrompt"}, + "env": {"REPOPROMPT_SWIFT_JOBS": ""}, + } + ) + + self.assertEqual(with_env[0], ["swift", "build", "--product", "RepoPrompt", "--jobs", "4"]) + self.assertEqual(without_env[0], ["swift", "build", "--product", "RepoPrompt"]) + self.assertEqual(empty_env[0], ["swift", "build", "--product", "RepoPrompt"]) + + def test_swift_jobs_env_set_appends_jobs_flag(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + registry = conductor.OperationRegistry(Path(tmp)) + env = {"REPOPROMPT_SWIFT_JOBS": "4"} + build_argv, _lanes, _cwd, _env, _timeout = registry.prepare( + {"operation": "swift-build", "args": {"product": "RepoPrompt"}, "env": env} + ) + test_argv, _lanes, _cwd, _env, _timeout = registry.prepare( + {"operation": "test", "args": {"filter": "ExampleTests"}, "env": env} + ) + provider_argv, _lanes, _cwd, _env, _timeout = registry.prepare( + {"operation": "provider-test", "args": {"list": True}, "env": env} + ) + + self.assertEqual(build_argv, ["swift", "build", "--product", "RepoPrompt", "--jobs", "4"]) + self.assertEqual(test_argv, ["swift", "test", "--filter", "ExampleTests", "--jobs", "4"]) + self.assertEqual(provider_argv, ["swift", "test", "list", "--jobs", "4"]) + self.assertEqual(_env["REPOPROMPT_SWIFT_JOBS"], "4") + + def test_swift_jobs_env_invalid_raises(self) -> None: + for invalid in ["0", "-1", "abc", "01", "+1", " 1", "1.0"]: + with self.subTest(value=invalid): + with self.assertRaisesRegex(conductor.ConductorError, "must be a positive integer"): + conductor.configured_swift_jobs({"REPOPROMPT_SWIFT_JOBS": invalid}) + + def test_swift_jobs_env_valid_via_os_environ(self) -> None: + with mock.patch.dict(os.environ, {"REPOPROMPT_SWIFT_JOBS": "8"}, clear=False): + self.assertEqual(conductor.configured_swift_jobs(), 8) + + def test_swift_jobs_env_empty_via_os_environ(self) -> None: + with mock.patch.dict(os.environ, {"REPOPROMPT_SWIFT_JOBS": ""}, clear=False): + self.assertIsNone(conductor.configured_swift_jobs()) + + def test_swift_jobs_limit_includes_env_in_fingerprint(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + registry = conductor.OperationRegistry(Path(tmp)) + f1 = registry.fingerprint( + { + "operation": "swift-build", + "args": {"product": "RepoPrompt"}, + "env": {"REPOPROMPT_SWIFT_JOBS": "2"}, + } + ) + f2 = registry.fingerprint( + { + "operation": "swift-build", + "args": {"product": "RepoPrompt"}, + "env": {"REPOPROMPT_SWIFT_JOBS": "2"}, + } + ) + f3 = registry.fingerprint( + { + "operation": "swift-build", + "args": {"product": "RepoPrompt"}, + "env": {"REPOPROMPT_SWIFT_JOBS": "3"}, + } + ) + f4 = registry.fingerprint( + {"operation": "swift-build", "args": {"product": "RepoPrompt"}, "env": {}} + ) + + self.assertEqual(f1, f2) + self.assertNotEqual(f1, f3) + self.assertNotEqual(f1, f4) + + def test_non_swift_ops_unaffected_by_swift_jobs_env(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + registry = conductor.OperationRegistry(Path(tmp)) + env = {"REPOPROMPT_SWIFT_JOBS": "4"} + build_argv, _, _, _, _ = registry.prepare( + {"operation": "build", "args": {}, "env": env} + ) + package_argv, _, _, _, _ = registry.prepare( + {"operation": "package", "args": {"config": "release"}, "env": env} + ) + release_argv, _, _, _, _ = registry.prepare( + {"operation": "release", "args": {"subcommand": "package"}, "env": env} + ) + install_argv, _, _, _, _ = registry.prepare( + {"operation": "install-debug-cli", "args": {}, "env": env} + ) + + self.assertNotIn("--jobs", build_argv) + self.assertNotIn("--jobs", package_argv) + self.assertNotIn("--jobs", release_argv) + self.assertNotIn("--jobs", install_argv) + + def test_swift_build_all_forwards_jobs_limit(self) -> None: + with mock.patch.dict(os.environ, {"REPOPROMPT_SWIFT_JOBS": "3"}, clear=False): + with mock.patch.object(conductor, "run_operation_command", return_value=(0, "", "")) as run_cmd: + code = conductor.operation_swift_build_all(Path("/tmp/repo")) + + self.assertEqual(code, 0) + self.assertEqual(run_cmd.call_count, 2) + for call in run_cmd.call_args_list: + self.assertEqual(call.kwargs.get("swift_jobs_limit"), 3) + self.assertIn("--jobs", call.args[1]) + self.assertIn("3", call.args[1]) + + def test_swift_jobs_packaging_scripts_use_same_regex(self) -> None: + for script_name in ["package_app.sh", "build_swiftpm_release_products.sh"]: + with self.subTest(script=script_name): + script = SCRIPT_DIR / script_name + text = script.read_text(encoding="utf-8") + self.assertIn("REPOPROMPT_SWIFT_JOBS", text) + self.assertIn('[[ "$REPOPROMPT_SWIFT_JOBS" =~ ^[1-9][0-9]*$ ]]', text) + self.assertIn('"--jobs" "$REPOPROMPT_SWIFT_JOBS"', text) + + def test_swift_jobs_shell_regex_matches_python(self) -> None: + for value in ["0", "-1", "abc", "01", "+1", " 1", "1.0", "4", "42"]: + with self.subTest(value=value): + python_valid = None + try: + conductor.configured_swift_jobs({"REPOPROMPT_SWIFT_JOBS": value}) + python_valid = True + except conductor.ConductorError: + python_valid = False + result = subprocess.run( + ["bash", "-c", "[[ \"$1\" =~ ^[1-9][0-9]*$ ]]", "bash", value], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + shell_valid = result.returncode == 0 + self.assertEqual( + python_valid, + shell_valid, + f"mismatch for {value!r}: python={python_valid}, shell={shell_valid}", + ) + + def test_invalid_swift_jobs_env_does_not_affect_non_swift_ops(self) -> None: + tmp, state = self.make_state() + self.addCleanup(tmp.cleanup) + # Invalid REPOPROMPT_SWIFT_JOBS should be ignored by sleep operations. + payload = state.enqueue( + { + "operation": "sleep", + "args": {"seconds": 0.1}, + "env": {"REPOPROMPT_SWIFT_JOBS": "0"}, + } + ) + result = state.job_wait(payload["ticket"], None, timeout=5.0) + self.assertEqual(result["state"], "completed") + self.assertEqual(result["exitCode"], 0) + self.assertNotIn("positive integer", (result.get("error") or "").lower()) + + def test_swift_build_all_rejects_invalid_swift_jobs_env_at_daemon(self) -> None: + tmp, state = self.make_state() + self.addCleanup(tmp.cleanup) + # swift-build product all uses __operation_runner, so the daemon must be + # the one that validates the limit in _run_job. + env = {"REPOPROMPT_SWIFT_JOBS": "0", "REPOPROMPT_DEV_HEAVY_SLOTS": "8"} + payload = state.enqueue( + { + "operation": "swift-build", + "args": {"product": "all"}, + "env": env, + } + ) + result = state.job_wait(payload["ticket"], None, timeout=5.0) + self.assertEqual(result["state"], "failed") + self.assertIn("positive integer", (result.get("error") or "").lower()) + + class ProcessTreeCancellationTests(LifecycleTestCase): def wait_until(self, predicate, timeout: float = 5.0) -> bool: deadline = time.monotonic() + timeout diff --git a/Tests/RepoPromptTests/AgentMode/ACPAgentSessionControllerModeConfigTests.swift b/Tests/RepoPromptTests/AgentMode/ACPAgentSessionControllerModeConfigTests.swift index 34afbbc6a..0aafbed26 100644 --- a/Tests/RepoPromptTests/AgentMode/ACPAgentSessionControllerModeConfigTests.swift +++ b/Tests/RepoPromptTests/AgentMode/ACPAgentSessionControllerModeConfigTests.swift @@ -710,8 +710,12 @@ final class ACPAgentSessionControllerModeConfigTests: XCTestCase { let diagnostic = try XCTUnwrap(results.first { $0.type == "final_content" }?.text) XCTAssertTrue(diagnostic.contains("OpenCode ACP completed with stopReason=end_turn")) - XCTAssertTrue(diagnostic.contains("OpenCode stderr reported: Authentication Failed.")) XCTAssertTrue(diagnostic.contains("RepoPrompt did not receive model text to render.")) + // stderr and JSON-RPC stdout arrive on independent pipes, so process-side + // write order cannot guarantee that the terminal diagnostic sees this line. + // The controller must still surface the provider error as a system event. + let stderr = try XCTUnwrap(results.first { $0.type == "system" }?.text) + XCTAssertTrue(stderr.contains("Authentication Failed")) } func testTransportTerminationDrainsPromptSettlementWaiters() async throws {