fix(setup): port-in-use prompt, subprocess timeouts, parse-time validation

+377 -36

2 changed files

expand all

tests

test_setup.py

think

setup.py

+194 -11

tests/test_setup.py

··· 57 57 ) 58 58 59 59 60 + def port_advisory_payload() -> str: 61 + return doctor_payload( 62 + [ 63 + { 64 + "name": "port_5015_free", 65 + "severity": "advisory", 66 + "status": "warn", 67 + "detail": "port 5015 is in use by pid 123", 68 + "fix": "kill 123", 69 + } 70 + ] 71 + ) 72 + 73 + 60 74 def patch_subprocess( 61 75 monkeypatch: pytest.MonkeyPatch, 62 76 *, 63 77 doctor_stdout: str | None = None, 64 78 doctor_returncode: int = 0, 65 79 command_returncode: int = 0, 80 + doctor_timeout: bool = False, 81 + popen_timeout_command: list[str] | None = None, 66 82 ) -> list[list[str]]: 67 83 calls: list[list[str]] = [] 68 84 ··· 71 87 ) -> subprocess.CompletedProcess[str]: 72 88 calls.append(command) 73 89 if "doctor" in command: 90 + if doctor_timeout: 91 + raise subprocess.TimeoutExpired(command, setup.DOCTOR_TIMEOUT_SECONDS) 74 92 return subprocess.CompletedProcess( 75 93 command, 76 94 doctor_returncode, ··· 79 97 ) 80 98 return subprocess.CompletedProcess(command, command_returncode) 81 99 100 + class FakePopen: 101 + def __init__(self, command: list[str], **kwargs: object) -> None: 102 + del kwargs 103 + self.command = command 104 + self.terminated = False 105 + calls.append(command) 106 + 107 + def wait(self, timeout: float | None = None) -> int: 108 + if self.command == popen_timeout_command and not self.terminated: 109 + raise subprocess.TimeoutExpired(self.command, timeout) 110 + return command_returncode 111 + 112 + def terminate(self) -> None: 113 + self.terminated = True 114 + 115 + def kill(self) -> None: 116 + self.terminated = True 117 + 82 118 monkeypatch.setattr(setup.subprocess, "run", fake_run) 119 + monkeypatch.setattr(setup.subprocess, "Popen", FakePopen) 83 120 return calls 84 121 85 122 ··· 689 726 monkeypatch.delenv("SOLSTONE_JOURNAL", raising=False) 690 727 calls = patch_subprocess( 691 728 monkeypatch, 692 - doctor_stdout=doctor_payload( 693 - [ 694 - { 695 - "name": "port_5015_free", 696 - "severity": "advisory", 697 - "status": "warn", 698 - "detail": "port 5015 is in use by pid 123", 699 - "fix": "kill 123", 700 - } 701 - ] 702 - ), 729 + doctor_stdout=port_advisory_payload(), 703 730 ) 704 731 journal = tmp_path / "journal" 705 732 ··· 709 736 assert "port 5015 is already in use" in capsys.readouterr().err 710 737 assert_command(calls, 0, expected_doctor_command()) 711 738 assert len(calls) == 1 739 + 740 + 741 + def test_interactive_port_in_use_prompts_for_choice( 742 + tmp_path: Path, 743 + monkeypatch: pytest.MonkeyPatch, 744 + ) -> None: 745 + home = patch_home(monkeypatch, tmp_path) 746 + patch_source_checkout(monkeypatch, tmp_path) 747 + patch_tty(monkeypatch) 748 + monkeypatch.delenv("SOLSTONE_JOURNAL", raising=False) 749 + (home / ".claude").mkdir() 750 + answers = iter(["1", "8080"]) 751 + monkeypatch.setattr("builtins.input", lambda _prompt: next(answers)) 752 + calls = patch_subprocess(monkeypatch, doctor_stdout=port_advisory_payload()) 753 + patch_service_health(monkeypatch) 754 + 755 + rc = setup.main([]) 756 + 757 + assert rc == 0 758 + journal = home / "Documents" / "journal" 759 + assert read_manifest(journal)["args_resolved"]["port"] == { 760 + "value": 8080, 761 + "source": "prompt", 762 + } 763 + assert_command(calls, 0, expected_doctor_command()) 764 + assert_command(calls, 4, expected_service_install_command(port=8080)) 765 + assert len(calls) == 5 766 + 767 + 768 + def test_interactive_port_in_use_proceed_anyway( 769 + tmp_path: Path, 770 + monkeypatch: pytest.MonkeyPatch, 771 + ) -> None: 772 + home = patch_home(monkeypatch, tmp_path) 773 + patch_source_checkout(monkeypatch, tmp_path) 774 + patch_tty(monkeypatch) 775 + monkeypatch.delenv("SOLSTONE_JOURNAL", raising=False) 776 + (home / ".claude").mkdir() 777 + answers = iter(["2"]) 778 + monkeypatch.setattr("builtins.input", lambda _prompt: next(answers)) 779 + calls = patch_subprocess(monkeypatch, doctor_stdout=port_advisory_payload()) 780 + patch_service_health(monkeypatch) 781 + 782 + rc = setup.main([]) 783 + 784 + assert rc == 0 785 + assert_command(calls, 0, expected_doctor_command()) 786 + assert_command(calls, 4, expected_service_install_command(port=5015)) 787 + assert len(calls) == 5 788 + 789 + 790 + def test_interactive_port_in_use_abort( 791 + tmp_path: Path, 792 + monkeypatch: pytest.MonkeyPatch, 793 + capsys: pytest.CaptureFixture[str], 794 + ) -> None: 795 + patch_home(monkeypatch, tmp_path) 796 + patch_source_checkout(monkeypatch, tmp_path) 797 + patch_tty(monkeypatch) 798 + monkeypatch.delenv("SOLSTONE_JOURNAL", raising=False) 799 + answers = iter(["3"]) 800 + monkeypatch.setattr("builtins.input", lambda _prompt: next(answers)) 801 + calls = patch_subprocess(monkeypatch, doctor_stdout=port_advisory_payload()) 802 + 803 + rc = setup.main([]) 804 + 805 + assert rc == 2 806 + assert "setup aborted by user" in capsys.readouterr().err 807 + assert_command(calls, 0, expected_doctor_command()) 808 + assert expected_service_install_command() not in calls 809 + assert len(calls) == 1 810 + 811 + 812 + def test_doctor_timeout_records_failure( 813 + tmp_path: Path, 814 + monkeypatch: pytest.MonkeyPatch, 815 + ) -> None: 816 + patch_home(monkeypatch, tmp_path) 817 + patch_source_checkout(monkeypatch, tmp_path) 818 + monkeypatch.delenv("SOLSTONE_JOURNAL", raising=False) 819 + journal = tmp_path / "journal" 820 + patch_subprocess(monkeypatch, doctor_timeout=True) 821 + 822 + rc = setup.main(["--yes", "--journal", str(journal)]) 823 + 824 + assert rc == 1 825 + step = read_manifest(journal)["steps"][-1] 826 + assert step["name"] == "doctor" 827 + assert step["status"] == "failed" 828 + assert "timed out after 30s" in step["error"]["message"] 829 + assert step["error"]["exit_code"] == 1 830 + 831 + 832 + def test_install_models_timeout_records_failure( 833 + tmp_path: Path, 834 + monkeypatch: pytest.MonkeyPatch, 835 + ) -> None: 836 + patch_home(monkeypatch, tmp_path) 837 + patch_source_checkout(monkeypatch, tmp_path) 838 + monkeypatch.delenv("SOLSTONE_JOURNAL", raising=False) 839 + journal = tmp_path / "journal" 840 + patch_subprocess( 841 + monkeypatch, 842 + popen_timeout_command=expected_install_models_command(), 843 + ) 844 + 845 + rc = setup.main(["--yes", "--journal", str(journal), "--step-timeout-seconds", "1"]) 846 + 847 + assert rc == 1 848 + step = read_manifest(journal)["steps"][-1] 849 + assert step["name"] == "install_models" 850 + assert step["status"] == "failed" 851 + assert "timed out after 1s" in step["error"]["message"] 852 + assert step["error"]["exit_code"] == 1 853 + 854 + 855 + def test_step_timeout_seconds_passes_through_to_help_and_explain( 856 + tmp_path: Path, 857 + monkeypatch: pytest.MonkeyPatch, 858 + capsys: pytest.CaptureFixture[str], 859 + ) -> None: 860 + with pytest.raises(SystemExit) as raised: 861 + setup.main(["--help"]) 862 + assert raised.value.code == 0 863 + assert "--step-timeout-seconds" in capsys.readouterr().out 864 + 865 + patch_home(monkeypatch, tmp_path) 866 + patch_source_checkout(monkeypatch, tmp_path) 867 + monkeypatch.delenv("SOLSTONE_JOURNAL", raising=False) 868 + 869 + rc = setup.main(["--explain", "--yes"]) 870 + 871 + assert rc == 0 872 + assert "step_timeout_seconds: 1800" in capsys.readouterr().out 873 + 874 + 875 + def test_empty_journal_arg_rejected_at_parse_time( 876 + capsys: pytest.CaptureFixture[str], 877 + ) -> None: 878 + with pytest.raises(SystemExit) as raised: 879 + setup.main(["--journal", ""]) 880 + 881 + assert raised.value.code == 2 882 + assert "--journal must not be empty" in capsys.readouterr().err 883 + 884 + 885 + @pytest.mark.parametrize("port", ["0", "99999"]) 886 + def test_port_out_of_range_rejected_at_parse_time( 887 + capsys: pytest.CaptureFixture[str], 888 + port: str, 889 + ) -> None: 890 + with pytest.raises(SystemExit) as raised: 891 + setup.main(["--port", port]) 892 + 893 + assert raised.value.code == 2 894 + assert "--port must be in 1024-65535" in capsys.readouterr().err 712 895 713 896 714 897 def test_packaged_install_skips_service(

+183 -25

think/setup.py

··· 31 31 MANIFEST_SCHEMA_VERSION = 1 32 32 HEALTH_ATTEMPTS = 20 33 33 HEALTH_SLEEP_SECONDS = 1.0 34 + DOCTOR_TIMEOUT_SECONDS = 30 34 35 35 36 StepStatus = Literal["ok", "skipped", "failed"] 36 37 ··· 54 55 port: int 55 56 port_source: str 56 57 port_supplied: bool 58 + step_timeout_seconds: int 57 59 variant: str 58 60 variant_source: str 59 61 yes: bool ··· 87 89 self.exit_code = exit_code 88 90 89 91 92 + def _journal_arg(value: str) -> Path: 93 + if not value or not value.strip(): 94 + raise argparse.ArgumentTypeError("--journal must not be empty") 95 + path = Path(value).expanduser() 96 + try: 97 + resolved = path.resolve() 98 + except (OSError, RuntimeError) as exc: 99 + raise argparse.ArgumentTypeError( 100 + f"--journal could not be resolved: {exc}" 101 + ) from exc 102 + if resolved == Path.cwd().resolve(): 103 + raise argparse.ArgumentTypeError("--journal must not be empty") 104 + return path 105 + 106 + 107 + def _port_arg(value: str) -> int: 108 + try: 109 + port = int(value) 110 + except ValueError as exc: 111 + raise argparse.ArgumentTypeError( 112 + f"--port must be in 1024-65535 (got {value})" 113 + ) from exc 114 + if not 1024 <= port <= 65535: 115 + raise argparse.ArgumentTypeError(f"--port must be in 1024-65535 (got {port})") 116 + return port 117 + 118 + 90 119 def build_parser() -> argparse.ArgumentParser: 91 120 parser = argparse.ArgumentParser( 92 121 prog="sol setup", ··· 95 124 parser.add_argument( 96 125 "--journal", 97 126 metavar="PATH", 98 - type=Path, 127 + type=_journal_arg, 99 128 default=None, 100 129 help="journal directory to persist in ~/.config/solstone/config.toml", 101 130 ) 102 131 parser.add_argument( 103 132 "--port", 104 133 metavar="INT", 105 - type=int, 134 + type=_port_arg, 106 135 default=5015, 107 136 help="convey service port (default: 5015)", 108 137 ) ··· 113 142 help="Parakeet model/runtime variant passed to sol install-models (default: auto)", 114 143 ) 115 144 parser.add_argument( 145 + "--step-timeout-seconds", 146 + metavar="INT", 147 + type=int, 148 + default=1800, 149 + help=( 150 + "timeout for model, skill, and wrapper steps in seconds " 151 + "(default: 1800; doctor uses a separate 30s timeout)" 152 + ), 153 + ) 154 + parser.add_argument( 116 155 "-y", 117 156 "--yes", 118 157 "--non-interactive", ··· 183 222 cfg_path = config_path() 184 223 manifest_path = journal_path / ".setup-state.json" 185 224 port_supplied = arg_supplied(raw_argv, "--port") 225 + step_timeout_supplied = arg_supplied(raw_argv, "--step-timeout-seconds") 186 226 variant_supplied = arg_supplied(raw_argv, "--variant") 187 227 188 228 args_resolved: dict[str, object] = { ··· 194 234 "value": args.port, 195 235 "source": "cli" if port_supplied else "default", 196 236 }, 237 + "step_timeout_seconds": { 238 + "value": args.step_timeout_seconds, 239 + "source": "cli" if step_timeout_supplied else "default", 240 + }, 197 241 "variant": { 198 242 "value": args.variant, 199 243 "source": "cli" if variant_supplied else "default", ··· 248 292 port=args.port, 249 293 port_source="cli" if port_supplied else "default", 250 294 port_supplied=port_supplied, 295 + step_timeout_seconds=args.step_timeout_seconds, 251 296 variant=args.variant, 252 297 variant_source="cli" if variant_supplied else "default", 253 298 yes=bool(args.yes), ··· 433 478 return " ".join(command) 434 479 435 480 436 - def run_inherited(command: list[str]) -> int: 437 - result = subprocess.run(command, stdout=None, stderr=None, check=False) 438 - return int(result.returncode) 481 + def run_inherited(command: list[str], *, timeout: float | None = None) -> int: 482 + if timeout is None: 483 + result = subprocess.run(command, stdout=None, stderr=None, check=False) 484 + return int(result.returncode) 485 + proc = subprocess.Popen(command, stdout=None, stderr=None) 486 + try: 487 + return int(proc.wait(timeout=timeout)) 488 + except subprocess.TimeoutExpired: 489 + proc.terminate() 490 + try: 491 + proc.wait(timeout=5) 492 + except subprocess.TimeoutExpired: 493 + proc.kill() 494 + proc.wait() 495 + raise 439 496 440 497 441 498 def doctor_command(ctx: SetupContext) -> list[str]: ··· 493 550 started_at = utc_now() 494 551 command = doctor_command(ctx) 495 552 print_step_header(step_index, "doctor", command) 496 - result = subprocess.run( 497 - command, 498 - capture_output=True, 499 - text=True, 500 - check=False, 501 - ) 553 + try: 554 + result = subprocess.run( 555 + command, 556 + capture_output=True, 557 + text=True, 558 + check=False, 559 + timeout=DOCTOR_TIMEOUT_SECONDS, 560 + ) 561 + except subprocess.TimeoutExpired: 562 + return step_result( 563 + "doctor", 564 + "failed", 565 + [], 566 + started_at, 567 + { 568 + "message": f"doctor timed out after {DOCTOR_TIMEOUT_SECONDS}s", 569 + "exit_code": 1, 570 + }, 571 + ) 502 572 if result.returncode != 0: 503 573 if result.stdout: 504 574 print(result.stdout, end="" if result.stdout.endswith("\n") else "\n") ··· 539 609 and check.get("severity") == "advisory" 540 610 and check.get("status") in ("warn", "fail") 541 611 ] 542 - maybe_dead_end_on_port(ctx) 612 + maybe_handle_port_in_use(ctx) 543 613 print(f"[step {step_index}/{TOTAL_STEPS}] doctor passed") 544 614 return step_result("doctor", "ok", [], started_at) 545 615 546 616 547 - def maybe_dead_end_on_port(ctx: SetupContext) -> None: 548 - if ( 549 - ctx.skip_service 550 - or ctx.port_supplied 551 - or ctx.mode is not SetupMode.NON_INTERACTIVE 552 - ): 553 - return 617 + def _port_advisory_present(ctx: SetupContext) -> bool: 554 618 for advisory in ctx.doctor_advisories: 555 619 if advisory.get("name") == "port_5015_free": 556 - dead_end_port_in_use(ctx) 620 + return True 621 + detail = str(advisory.get("detail", "")) 622 + if f"port {ctx.port}" in detail: 623 + return True 624 + return False 625 + 626 + 627 + def maybe_handle_port_in_use(ctx: SetupContext) -> None: 628 + if ctx.skip_service or ctx.port_supplied: 629 + return 630 + if not _port_advisory_present(ctx): 631 + return 632 + if ctx.mode is SetupMode.NON_INTERACTIVE: 633 + dead_end_port_in_use(ctx) 634 + return 635 + prompt_port_choice(ctx) 636 + 637 + 638 + def prompt_port_choice(ctx: SetupContext) -> None: 639 + advisory = next( 640 + ( 641 + item 642 + for item in ctx.doctor_advisories 643 + if item.get("name") == "port_5015_free" 644 + ), 645 + None, 646 + ) 647 + if advisory: 557 648 detail = advisory.get("detail") 558 - if isinstance(detail, str) and f"port {ctx.port}" in detail: 559 - dead_end_port_in_use(ctx) 649 + if detail: 650 + print(f" {detail}") 651 + fix = advisory.get("fix") 652 + if fix: 653 + print(f" suggested fix: {fix}") 654 + print() 655 + print(" 1) enter a different port") 656 + print(" 2) proceed anyway on this port") 657 + print(" 3) abort setup") 658 + while True: 659 + choice = input("choice [1/2/3]: ").strip() 660 + if choice == "1": 661 + while True: 662 + raw = input("port: ").strip() 663 + try: 664 + new_port = _port_arg(raw) 665 + except argparse.ArgumentTypeError as exc: 666 + print(f" {exc}") 667 + continue 668 + ctx.port = new_port 669 + ctx.port_source = "prompt" 670 + ctx.args_resolved["port"] = {"value": new_port, "source": "prompt"} 671 + return 672 + if choice == "2": 673 + return 674 + if choice == "3": 675 + raise SetupDeadEnd("setup aborted by user", 2) 676 + print(" invalid choice; enter 1, 2, or 3") 560 677 561 678 562 679 def step_journal(ctx: SetupContext, step_index: int) -> StepResult: ··· 627 744 ) 628 745 command = install_models_command(ctx) 629 746 print_step_header(step_index, "install-models", command) 630 - rc = run_inherited(command) 747 + try: 748 + rc = run_inherited(command, timeout=ctx.step_timeout_seconds) 749 + except subprocess.TimeoutExpired: 750 + return step_result( 751 + "install_models", 752 + "failed", 753 + model_paths(), 754 + started_at, 755 + { 756 + "message": ( 757 + f"install_models timed out after {ctx.step_timeout_seconds}s" 758 + ), 759 + "exit_code": 1, 760 + }, 761 + ) 631 762 if rc != 0: 632 763 return step_result( 633 764 "install_models", ··· 654 785 ) 655 786 command = skills_command() 656 787 print_step_header(step_index, "skills", command) 657 - rc = run_inherited(command) 788 + try: 789 + rc = run_inherited(command, timeout=ctx.step_timeout_seconds) 790 + except subprocess.TimeoutExpired: 791 + return step_result( 792 + "skills", 793 + "failed", 794 + [skill_path], 795 + started_at, 796 + { 797 + "message": f"skills timed out after {ctx.step_timeout_seconds}s", 798 + "exit_code": 1, 799 + }, 800 + ) 658 801 if rc != 0: 659 802 return step_result( 660 803 "skills", ··· 676 819 ) 677 820 command = wrapper_command() 678 821 print_step_header(step_index, "wrapper", command) 679 - rc = run_inherited(command) 822 + try: 823 + rc = run_inherited(command, timeout=ctx.step_timeout_seconds) 824 + except subprocess.TimeoutExpired: 825 + return step_result( 826 + "wrapper", 827 + "failed", 828 + [wrapper_path], 829 + started_at, 830 + { 831 + "message": f"wrapper timed out after {ctx.step_timeout_seconds}s", 832 + "exit_code": 1, 833 + }, 834 + ) 680 835 if rc != 0: 681 836 return step_result( 682 837 "wrapper", ··· 822 977 print(f" journal: {ctx.journal_path} ({ctx.journal_source})") 823 978 print(f" port: {ctx.port} ({ctx.port_source})") 824 979 print(f" variant: {ctx.variant} ({ctx.variant_source})") 980 + timeout_resolved = ctx.args_resolved["step_timeout_seconds"] 981 + timeout_source = timeout_resolved["source"] 982 + print(f" step_timeout_seconds: {ctx.step_timeout_seconds} ({timeout_source})") 825 983 print(f" source checkout: {ctx.is_source_checkout}") 826 984 print() 827 985 print(f"[step 1/6] {_STEP_NAME[step_doctor]}")

Configure Feed

Configure Feed