From 8f506a76530e89af5df23c24c04020233e6ce20f Mon Sep 17 00:00:00 2001 From: Jeff Huber Date: Wed, 2 Sep 2026 13:22:37 -0700 Subject: [PATCH] fix: tune doctor adoption postures --- README.md | 2 + docs/install.md | 6 +++ docs/quickstart.md | 6 ++- docs/try-in-10-minutes.md | 7 ++- src/code_mower/doctor.py | 42 ++++++++++++++++- src/code_mower/doctor_checks/adoption.py | 32 ++++++++----- src/code_mower/doctor_checks/common.py | 2 + src/code_mower/doctor_checks/github.py | 2 + .../doctor_checks/github_human_token.py | 46 ++++++++++++++++--- src/code_mower/doctor_checks/providers.py | 13 ++++-- src/code_mower/doctor_checks/runner.py | 3 ++ tests/test_doctor_github_checks.py | 30 ++++++++++++ tests/test_doctor_registry.py | 45 ++++++++++++++++++ 13 files changed, 209 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index f76791a8..3830efe6 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,8 @@ for a real repository. It checks your runtime, GitHub setup, provider CLIs, token posture, optional cloud setup, private-repo Actions cost traps, and first-run adoption gaps. `--preflight` remains the compatibility preset for older scripts; `--adoption` adds explicit repo targeting and setup guidance. +Use `--hosted-builders` or `--orchestrator-only` when the current machine is +observing/coordinating lanes and will not run local Codex or Claude wrappers. Example, shortened: diff --git a/docs/install.md b/docs/install.md index 59dfa2d2..96bae2f2 100644 --- a/docs/install.md +++ b/docs/install.md @@ -191,6 +191,12 @@ code-mower doctor --adoption --hosted-builders --repo OWNER/REPO --json code-mower doctor --adoption --orchestrator-only --repo OWNER/REPO --json ``` +In those observer/coordinator postures, missing local wrapper environment +variables and missing `DISPATCH_TOKEN` setup are surfaced as owner setup or +promotion tasks, not as proof the install is broken. Use the default +reviewer-gate posture on the machine that will actually run local audit +wrappers or unattended dispatch. + Then follow [Try Code Mower In 10 Minutes](try-in-10-minutes.md) for the first audited PR or [Build Loop In 30 Minutes](build-loop-in-30-minutes.md) after the reviewer gate is working. diff --git a/docs/quickstart.md b/docs/quickstart.md index 13ad1bfa..3f2bf998 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -288,7 +288,8 @@ code-mower doctor --adoption --repo OWNER/REPO --json ``` `doctor --adoption` is the recommended early-adopter preset for GitHub auth, -Python/runtime checks, provider CLI probes, private-repo caveats, Actions cost +Python/runtime checks, provider CLI probes for machines that run local lanes, +private-repo caveats, Actions cost diagnostics, branch-protection source, repository auto-merge, human automation token metadata, optional cloud-token setup, and first-run setup gaps such as starter config or missing owner/trusted-author posture. Use `--strict` only @@ -297,7 +298,8 @@ when warnings should fail a bootstrap job. For auth-specific doctor failures, se If this machine observes or dispatches hosted builders but does not run local Codex/Claude audits, use `--hosted-builders` or `--orchestrator-only` with `doctor --adoption`; those postures keep GitHub, cloud, setup, and privacy -checks visible while marking local CLI probes skipped. +checks visible while marking local CLI probes skipped and treating local wrapper +env gaps as setup tasks for the machine that will execute those lanes. When setup is visible enough to start work, use one command to check live lane state: diff --git a/docs/try-in-10-minutes.md b/docs/try-in-10-minutes.md index 99a0a30c..d7016a55 100644 --- a/docs/try-in-10-minutes.md +++ b/docs/try-in-10-minutes.md @@ -86,7 +86,8 @@ adopters need: - recommended profile selection; - Python/runtime checks; -- local provider CLI discovery and smoke probes; +- local provider CLI discovery and smoke probes when this machine will run + local lanes; - stale terminal-label hygiene for merge-authority reviewer lanes; - GitHub repository visibility, permissions, branch protection, and Actions cost diagnostics; and @@ -96,6 +97,10 @@ Warnings are setup guidance. They are only fatal when you pass `--strict`. In JSON mode, check the top-level `run_plan` field first. It tells you whether the preflight included GitHub and optional cloud checks before you inspect individual provider warnings. +Use `--hosted-builders` or `--orchestrator-only` when this machine observes or +coordinates lanes without running Codex/Claude local audit wrappers; those +postures skip local-wrapper probes and keep missing local wrapper env vars out +of the warning list. For merge-authority lanes such as Codex or Claude audit, look for `provider.review_hygiene`. It should pass for lanes that can satisfy the merge diff --git a/src/code_mower/doctor.py b/src/code_mower/doctor.py index 0d2e7340..821c870b 100644 --- a/src/code_mower/doctor.py +++ b/src/code_mower/doctor.py @@ -51,6 +51,39 @@ normalize_repo_slug = _doctor_checks.normalize_repo_slug +def _source_repo_uses_starter_config(cwd: Path, config_path: Path) -> bool: + """Return true when doctor is running from Code Mower's source tree.""" + + try: + config_path.resolve().relative_to(cwd.resolve()) + except ValueError: + return False + return ( + config_path.name == "code-mower.example.yml" + and (cwd / "pyproject.toml").is_file() + and (cwd / "src" / "code_mower" / "templates" / "code-mower.example.yml").is_file() + ) + + +def _doctor_config_source_label( + *, + config_arg: str, + config_path: Path, + easy: bool, + cwd: Path | None = None, +) -> str: + """Classify the config source for adoption-facing doctor output.""" + + cwd = cwd or Path.cwd() + if config_arg != "code-mower.yml": + return "explicit_config" + if config_path.name == "code-mower.example.yml" and easy: + if _source_repo_uses_starter_config(cwd, config_path): + return "source_tree_starter" + return "packaged_starter" + return "repository_config" + + _DOCTOR_COMPAT_EXPORTS = ( DEFAULT_CLOUD_TOKEN_DIR, DEFAULT_CLOUD_TOKEN_ENV, @@ -68,6 +101,7 @@ resolve_doctor_config_path_for_script, resolve_doctor_provider_templates_path, _token_file_mentions_cloud_token, + _doctor_config_source_label, ) @@ -202,12 +236,18 @@ def main(argv: Sequence[str] | None = None) -> int: repo_slug = detect_repo_slug(Path.cwd()) repo_source = "git_remote" if repo_slug else "" provider_templates_path = resolve_doctor_provider_templates_path(args.provider_templates) + config_path = resolve_doctor_config_path(args.config, easy=args.easy) report = run_doctor( - config_path=resolve_doctor_config_path(args.config, easy=args.easy), + config_path=config_path, provider_templates_path=provider_templates_path, profile=args.profile, repo_slug=repo_slug, repo_source=repo_source, + config_source=_doctor_config_source_label( + config_arg=args.config, + config_path=config_path, + easy=args.easy, + ), adoption=args.adoption, adoption_posture=args.adoption_posture, probe_runtime=args.probe_runtime, diff --git a/src/code_mower/doctor_checks/adoption.py b/src/code_mower/doctor_checks/adoption.py index b8486bd1..677cfda2 100644 --- a/src/code_mower/doctor_checks/adoption.py +++ b/src/code_mower/doctor_checks/adoption.py @@ -111,6 +111,7 @@ def check_adoption_setup( repo_slug: str, repo_source: str, using_packaged_example: bool, + config_source: str = "", ) -> tuple[DoctorCheck, ...]: """Return first-run adoption posture checks.""" @@ -144,17 +145,28 @@ def check_adoption_setup( ) repositories = _configured_repositories(config) + source = config_source or ( + "packaged_starter" if using_packaged_example else "repository_config" + ) + source_messages = { + "explicit_config": "using explicit Code Mower config", + "packaged_starter": "using packaged starter config for adoption checks", + "repository_config": "using repository Code Mower config", + "source_tree_starter": "using source-tree starter config for adoption checks", + } + detail = { + "config_path": str(config_path), + "config_source": source, + "configured_repositories": repositories, + "effective_repository": repo_slug, + } if using_packaged_example: checks.append( DoctorCheck( name="doctor.adoption.config_source", status=STATUS_WARN, - message="using packaged starter config for adoption checks", - detail={ - "config_path": str(config_path), - "configured_repositories": repositories, - "effective_repository": repo_slug, - }, + message=source_messages.get(source, source_messages["packaged_starter"]), + detail=detail, remediation=( "Run `code-mower init --easy --apply`, review the generated " "setup, and commit an edited code-mower.yml before relying " @@ -167,12 +179,8 @@ def check_adoption_setup( DoctorCheck( name="doctor.adoption.config_source", status=STATUS_PASS, - message="using repository Code Mower config", - detail={ - "config_path": str(config_path), - "configured_repositories": repositories, - "effective_repository": repo_slug, - }, + message=source_messages.get(source, source_messages["repository_config"]), + detail=detail, ) ) diff --git a/src/code_mower/doctor_checks/common.py b/src/code_mower/doctor_checks/common.py index 4ff9dfae..540fa87f 100644 --- a/src/code_mower/doctor_checks/common.py +++ b/src/code_mower/doctor_checks/common.py @@ -66,6 +66,7 @@ "audit-label-cleanup", "devin-audit-bridge", ) +OBSERVER_ADOPTION_POSTURES = frozenset({"hosted-builders", "orchestrator-only"}) __all__ = [ "ACTIONS_BILLING_BLOCK_PATTERNS", @@ -76,6 +77,7 @@ "DoctorCheck", "MAX_ACTIONS_FAILED_JOBS_TO_INSPECT", "MAX_ACTIONS_FAILED_RUNS_TO_INSPECT", + "OBSERVER_ADOPTION_POSTURES", "STATUS_FAIL", "STATUS_PASS", "STATUS_SKIP", diff --git a/src/code_mower/doctor_checks/github.py b/src/code_mower/doctor_checks/github.py index 76f45376..9754fbd4 100644 --- a/src/code_mower/doctor_checks/github.py +++ b/src/code_mower/doctor_checks/github.py @@ -36,6 +36,7 @@ def check_github_setup( lanes: Sequence[tuple[str, Mapping[str, Any]]], http_timeout: int, actions_cost_sample: int = ACTIONS_COST_SAMPLE_DEFAULT, + adoption_posture: str = "reviewer-gate", ) -> list[DoctorCheck]: checks: list[DoctorCheck] = [] gh_path = shutil.which("gh") @@ -142,6 +143,7 @@ def check_github_setup( config=config, lanes=lanes, http_timeout=http_timeout, + adoption_posture=adoption_posture, ) ) if has_merge_authority: diff --git a/src/code_mower/doctor_checks/github_human_token.py b/src/code_mower/doctor_checks/github_human_token.py index 1da49080..720952c3 100644 --- a/src/code_mower/doctor_checks/github_human_token.py +++ b/src/code_mower/doctor_checks/github_human_token.py @@ -5,7 +5,14 @@ from datetime import UTC, date, datetime from typing import Any, Mapping, Sequence -from .common import DoctorCheck, STATUS_FAIL, STATUS_PASS, STATUS_SKIP, STATUS_WARN +from .common import ( + OBSERVER_ADOPTION_POSTURES, + DoctorCheck, + STATUS_FAIL, + STATUS_PASS, + STATUS_SKIP, + STATUS_WARN, +) from .github_api import _github_api_json DEFAULT_HUMAN_TOKEN_SECRET = "DISPATCH_TOKEN" @@ -93,6 +100,18 @@ def _is_expiry_placeholder(value: str) -> bool: return value.strip().upper() in EXPIRY_PLACEHOLDER_VALUES +def _blocking_status_for_posture(adoption_posture: str) -> str: + return STATUS_WARN if adoption_posture in OBSERVER_ADOPTION_POSTURES else STATUS_FAIL + + +def _token_readiness_context(adoption_posture: str) -> str: + if adoption_posture == "hosted-builders": + return "hosted-builder observer posture" + if adoption_posture == "orchestrator-only": + return "orchestrator-only posture" + return "reviewer-gate posture" + + def check_human_automation_token( *, gh_path: str, @@ -100,6 +119,7 @@ def check_human_automation_token( config: Mapping[str, Any], lanes: Sequence[tuple[str, Mapping[str, Any]]], http_timeout: int, + adoption_posture: str = "reviewer-gate", now: datetime | None = None, ) -> DoctorCheck: token = human_automation_token_config(config) @@ -110,6 +130,7 @@ def check_human_automation_token( "secret": secret_name, "expires_var": expires_var, "required": human_automation_token_required(config, lanes), + "adoption_posture": adoption_posture, } if not detail["required"]: return DoctorCheck( @@ -125,15 +146,24 @@ def check_human_automation_token( http_timeout=http_timeout, ) if secret_payload is None: + status = _blocking_status_for_posture(adoption_posture) return DoctorCheck( name="github.human_automation_token", - status=STATUS_FAIL, - message=f"{slug} is missing the {secret_name} human automation token secret", + status=status, + message=( + f"{slug} is missing the {secret_name} human automation token secret" + + ( + f" for {_token_readiness_context(adoption_posture)}" + if status == STATUS_WARN + else "" + ) + ), detail={**detail, "secret_check": secret_detail}, remediation=( f"Create one human-owned fine-grained PAT secret with " f"`gh secret set {secret_name}`. Grant repository Contents read, " - "Issues read/write, and Pull requests read/write." + "Issues read/write, and Pull requests read/write before relying " + "on unattended dispatch, labels, or fix-round mentions." ), ) @@ -143,9 +173,10 @@ def check_human_automation_token( http_timeout=http_timeout, ) if variable_payload is None: + status = _blocking_status_for_posture(adoption_posture) return DoctorCheck( name="github.human_automation_token", - status=STATUS_FAIL, + status=status, message=f"{slug} is missing the {expires_var} human token expiry variable", detail={ **detail, @@ -188,9 +219,10 @@ def check_human_automation_token( ) expiry = _parse_expiry(expiry_text) if expiry is None: + status = _blocking_status_for_posture(adoption_posture) return DoctorCheck( name="github.human_automation_token", - status=STATUS_FAIL, + status=status, message=f"{slug} has an invalid {expires_var} value", detail={**detail, "expires_at": expiry_text}, remediation=( @@ -203,7 +235,7 @@ def check_human_automation_token( days_remaining = (expiry - today).days status = STATUS_PASS if days_remaining < 0: - status = STATUS_FAIL + status = _blocking_status_for_posture(adoption_posture) elif days_remaining <= EXPIRY_WARNING_DAYS: status = STATUS_WARN diff --git a/src/code_mower/doctor_checks/providers.py b/src/code_mower/doctor_checks/providers.py index 9e62955d..f503c6d8 100644 --- a/src/code_mower/doctor_checks/providers.py +++ b/src/code_mower/doctor_checks/providers.py @@ -6,6 +6,7 @@ from typing import Any, Mapping from .common import ( + OBSERVER_ADOPTION_POSTURES, DoctorCheck, STATUS_FAIL, STATUS_PASS, @@ -38,7 +39,7 @@ "selected_lanes", ] -LOCAL_CLI_SKIP_POSTURES = {"hosted-builders", "orchestrator-only"} +LOCAL_CLI_SKIP_POSTURES = OBSERVER_ADOPTION_POSTURES def selected_lanes( @@ -185,11 +186,15 @@ def check_lane_runtime( repo_root=repo_root, ) ] - checks.extend(check_token_env(lane_id, lane)) - checks.extend(check_required_env(lane_id, lane)) driver = str(lane.get("driver", "")) + skip_local_cli_runtime = ( + driver == "local_cli" and adoption_posture in LOCAL_CLI_SKIP_POSTURES + ) + if not skip_local_cli_runtime: + checks.extend(check_token_env(lane_id, lane)) + checks.extend(check_required_env(lane_id, lane)) if driver == "local_cli": - if adoption_posture in LOCAL_CLI_SKIP_POSTURES: + if skip_local_cli_runtime: checks.extend( _skip_local_cli_checks( lane_id, diff --git a/src/code_mower/doctor_checks/runner.py b/src/code_mower/doctor_checks/runner.py index 11ed5c6e..e5be0f18 100644 --- a/src/code_mower/doctor_checks/runner.py +++ b/src/code_mower/doctor_checks/runner.py @@ -84,6 +84,7 @@ def run_doctor( profile: str | None, repo_slug: str = "", repo_source: str = "", + config_source: str = "", adoption: bool = False, adoption_posture: str = "reviewer-gate", probe_runtime: bool = False, @@ -114,6 +115,7 @@ def run_doctor( adoption=adoption, repo_slug=repo_slug, repo_source=repo_source, + config_source=config_source, using_packaged_example=using_packaged_example, ) ) @@ -223,6 +225,7 @@ def run_doctor( lanes=effective_lanes, http_timeout=http_timeout, actions_cost_sample=actions_cost_sample, + adoption_posture=adoption_posture, ) ) diff --git a/tests/test_doctor_github_checks.py b/tests/test_doctor_github_checks.py index 81f454bc..5ad87ea1 100644 --- a/tests/test_doctor_github_checks.py +++ b/tests/test_doctor_github_checks.py @@ -32,6 +32,8 @@ class GitHubDoctorCheckTests(unittest.TestCase): def _human_token_check( self, api_responses: list[tuple[object, dict[str, object]]], + *, + adoption_posture: str = "reviewer-gate", ): with mock.patch( "code_mower.doctor_checks.github_human_token._github_api_json", @@ -43,6 +45,7 @@ def _human_token_check( config={"owner_surface": {"dispatch_token_env": "DISPATCH_TOKEN"}}, lanes=[("codex", {"token_env": ["DISPATCH_TOKEN", "GITHUB_TOKEN"]})], http_timeout=1, + adoption_posture=adoption_posture, now=datetime(2026, 8, 18, tzinfo=UTC), ) @@ -203,6 +206,19 @@ def test_human_automation_token_check_fails_when_secret_missing(self) -> None: self.assertEqual(check.status, "fail") self.assertIn("missing the DISPATCH_TOKEN", check.message) self.assertIn("fine-grained PAT", str(check.remediation)) + self.assertEqual(check.detail["adoption_posture"], "reviewer-gate") + + def test_human_automation_token_check_warns_for_hosted_builder_missing_secret( + self, + ) -> None: + check = self._human_token_check( + [(None, {"returncode": 1, "output_summary": "not found"})], + adoption_posture="hosted-builders", + ) + + self.assertEqual(check.status, "warn") + self.assertIn("hosted-builder observer posture", check.message) + self.assertEqual(check.detail["adoption_posture"], "hosted-builders") def test_human_automation_token_check_fails_when_expired(self) -> None: check = self._human_token_check( @@ -215,6 +231,20 @@ def test_human_automation_token_check_fails_when_expired(self) -> None: self.assertEqual(check.status, "fail") self.assertIn("expired", check.message) + def test_human_automation_token_check_warns_for_orchestrator_expired_secret( + self, + ) -> None: + check = self._human_token_check( + [ + ({"name": "DISPATCH_TOKEN"}, {}), + ({"name": "DISPATCH_TOKEN_EXPIRES_AT", "value": "2026-08-17"}, {}), + ], + adoption_posture="orchestrator-only", + ) + + self.assertEqual(check.status, "warn") + self.assertIn("expired", check.message) + def test_human_automation_token_check_rejects_timestamp_expiry(self) -> None: check = self._human_token_check( [ diff --git a/tests/test_doctor_registry.py b/tests/test_doctor_registry.py index 45cd3cb1..d7ffdebf 100644 --- a/tests/test_doctor_registry.py +++ b/tests/test_doctor_registry.py @@ -181,6 +181,7 @@ def test_hosted_builder_posture_skips_local_cli_runtime_checks(self) -> None: { "driver": "local_cli", "provider": "codex", + "token_env": ["MISSING_CODEX_TOKEN"], "provider_config": {"command": "definitely-missing-code-mower"}, }, probe_runtime=True, @@ -188,6 +189,9 @@ def test_hosted_builder_posture_skips_local_cli_runtime_checks(self) -> None: adoption_posture="hosted-builders", ) + self.assertFalse( + any(check.name in {"env.tokens", "env.required"} for check in checks) + ) local_checks = { check.name: check for check in checks @@ -209,14 +213,17 @@ def test_default_adoption_posture_checks_local_cli_runtime(self) -> None: { "driver": "local_cli", "provider": "codex", + "token_env": ["MISSING_CODEX_TOKEN"], "provider_config": {"command": "definitely-missing-code-mower"}, }, probe_runtime=True, http_timeout=1, ) + token_check = next(check for check in checks if check.name == "env.tokens") local_cli = next(check for check in checks if check.name == "runtime.local_cli") local_probe = next(check for check in checks if check.name == "runtime.local_cli.probe") + self.assertEqual(token_check.status, "warn") self.assertEqual(local_cli.status, "warn") self.assertEqual(local_probe.status, "warn") @@ -264,6 +271,42 @@ def fake_run_doctor( self.assertEqual(captured["adoption_posture"], expected) + def test_config_source_label_distinguishes_starter_and_explicit_paths(self) -> None: + with tempfile.TemporaryDirectory() as root: + root_path = Path(root) + starter_path = root_path / "src" / "code_mower" / "templates" / "code-mower.example.yml" + starter_path.parent.mkdir(parents=True) + starter_path.write_text("version: 1\n", encoding="utf-8") + (root_path / "pyproject.toml").write_text("[project]\nname = 'code-mower'\n", encoding="utf-8") + + self.assertEqual( + code_mower_doctor._doctor_config_source_label( + config_arg="code-mower.yml", + config_path=starter_path, + easy=True, + cwd=root_path, + ), + "source_tree_starter", + ) + self.assertEqual( + code_mower_doctor._doctor_config_source_label( + config_arg="custom-code-mower.yml", + config_path=root_path / "custom-code-mower.yml", + easy=False, + cwd=root_path, + ), + "explicit_config", + ) + self.assertEqual( + code_mower_doctor._doctor_config_source_label( + config_arg="code-mower.yml", + config_path=root_path / "code-mower.yml", + easy=False, + cwd=root_path, + ), + "repository_config", + ) + def test_adoption_repo_overrides_packaged_example_repository(self) -> None: report = run_doctor( config_path=ROOT / "src/code_mower/templates/code-mower.example.yml", @@ -282,6 +325,7 @@ def test_adoption_repo_overrides_packaged_example_repository(self) -> None: check for check in report.checks if check.name == "doctor.adoption.config_source" ) self.assertEqual(source_check.status, "warn") + self.assertEqual(source_check.detail["config_source"], "packaged_starter") self.assertEqual(source_check.detail["configured_repositories"], ["owner/example"]) self.assertEqual( source_check.detail["effective_repository"], @@ -307,6 +351,7 @@ def test_packaged_example_config_source_is_explicit_without_adoption_mode(self) check for check in report.checks if check.name == "doctor.adoption.config_source" ) self.assertEqual(source_check.status, "warn") + self.assertEqual(source_check.detail["config_source"], "packaged_starter") self.assertEqual(source_check.detail["configured_repositories"], ["owner/example"]) self.assertFalse(