From 4daf8d5c3c72f6693f832b7f96e9d90113e630bc Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Mon, 26 Oct 2026 08:03:34 -0700 Subject: [PATCH 1/2] feat: make doctor output a concise checklist Show a short fixed-copy checklist by default and retain the original technical report with --verbose. Keep diagnostic collection, JSON output, and exit codes unchanged. Extend safety, installed-wheel, entry-point, and completion coverage. Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- README.md | 23 +++- amplifier_app_cli/commands/doctor.py | 28 +++- amplifier_app_cli/lib/diagnostics.py | 75 +++++++++++ tests/check_doctor_wheel.py | 70 ++++++++-- tests/test_doctor.py | 195 ++++++++++++++++++++++++++- tests/test_doctor_entrypoint.py | 66 ++++++++- tests/test_shell_completion.py | 2 +- 7 files changed, 425 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 3320f569..41a2e19d 100644 --- a/README.md +++ b/README.md @@ -74,17 +74,24 @@ saved selections and user-added bundles are unchanged. ### Passive CLI diagnosis ```bash -amplifier doctor # Local runtime and settings declarations -amplifier doctor --format json # One structured report -amplifier doctor --session # Include this project's session settings +amplifier doctor # Short local-settings checklist +amplifier doctor --verbose # Original technical report +amplifier doctor --format json # Unchanged structured report +amplifier doctor --session --verbose # Include that session's settings ``` Doctor reads installed distribution metadata and bounded settings files from the current directory and Amplifier home. It does not resolve effective settings, fetch/install dependencies, load credentials, mount modules, or repair anything. +The default checklist shows whether user, project and local settings could be +read. A session row appears only with `--session`, which selects this project's +exact session's saved settings, not its live state. Missing settings are optional. +AI connections and running sessions are not tested. `--verbose` shows technical +details and has no effect with `--format json`. Existing-session state remains **unknown**, including when `--session` is used. Exit codes: `0` collection complete, `1` partial/refused, `2` invalid arguments. -Collection complete is not a health verdict. +Partial means some collection was limited; refused means collection stopped. +Collection complete is not a health verdict or proof that Amplifier works. The JSON report always marks health `not_assessed`; `issues` records collection limits, not every possible environment defect. The bounded diagnostic YAML parser intentionally accepts less syntax than the @@ -92,8 +99,9 @@ runtime parser: aliases, explicit tags, and duplicate keys report `unsupported_yaml`, not malformed `invalid_yaml`. This is a collection limit, not an instruction to repair settings that the runtime accepts. -Declaration values and arbitrary names use report-local equality refs. Scope, -container/member and zero-based parsed-order indices locate their declarations; +In technical text and JSON, declaration values and arbitrary names use +report-local equality refs. Scope, container/member and zero-based parsed-order +indices locate their declarations; they do not establish precedence or serve as mutation selectors. Local paths still identify the environment: review a report before sharing. @@ -108,7 +116,8 @@ The lightweight console dispatcher avoids importing the full interactive CLI implementation on that route. Explicit Python access to the public `cli` export still initializes the ordinary Click group, regardless of process arguments. -For agentic interpretation, delegate to **`app-cli:cli-doctor`** in a CLI session. +Next: Tell Amplifier what went wrong and ask for the CLI doctor agent +(**`app-cli:cli-doctor`**) in a CLI session. Doctor does not start an agent or model. It uses the report and loads the existing `amplifier-config` skill when needed. The agent's own child state is not proof of another session's live state. Diagnosis does not authorize changes or live probes. diff --git a/amplifier_app_cli/commands/doctor.py b/amplifier_app_cli/commands/doctor.py index 5cecbae7..810594d4 100644 --- a/amplifier_app_cli/commands/doctor.py +++ b/amplifier_app_cli/commands/doctor.py @@ -7,6 +7,7 @@ from ..lib.diagnostics import collect_diagnostics from ..lib.diagnostics import current_target +from ..lib.diagnostics import render_summary from ..lib.diagnostics import render_text from ..lib.diagnostics import valid_session_id @@ -26,10 +27,20 @@ def parse_args(self, ctx, args): @click.command(cls=_DoctorCommand) -@click.option("--format", "output_format", type=click.Choice(["text", "json"]), default="text") -@click.option("--session", "session_id", callback=_session, metavar="SESSION_ID") -def doctor(output_format, session_id): - """Report local declarations without resolving or activating them.""" +@click.option( + "--format", "output_format", type=click.Choice(["text", "json"]), default="text", + help="Show a text checklist (default) or the unchanged JSON report.", +) +@click.option( + "--session", "session_id", callback=_session, metavar="SESSION_ID", + help="Include this project's exact session's settings, not its live state.", +) +@click.option( + "--verbose", is_flag=True, + help="Show technical details (text only; does not affect JSON).", +) +def doctor(output_format, session_id, verbose): + """Read local configuration without changing settings.""" try: target = current_target(session_id) report = collect_diagnostics(target) @@ -38,7 +49,12 @@ def doctor(output_format, session_id): from ..lib.diagnostics import unavailable_report report = unavailable_report() - click.echo(json.dumps(report, ensure_ascii=True) if output_format == "json" else render_text(report)) + if output_format == "json": + click.echo(json.dumps(report, ensure_ascii=True)) + elif verbose: + click.echo(render_text(report)) + else: + click.echo(render_summary(report)) if report["collection_status"] != "complete": raise click.exceptions.Exit(1) @@ -49,7 +65,7 @@ def main(): code = doctor.main(args=sys.argv[2:], prog_name="amplifier doctor", standalone_mode=False) raise SystemExit(code or 0) except click.ClickException: - click.echo("Usage: amplifier doctor [--format text|json] [--session SESSION_ID]", err=True) + click.echo("Usage: amplifier doctor [--format text|json] [--session SESSION_ID] [--verbose]", err=True) click.echo("Error: invalid_doctor_arguments", err=True) raise SystemExit(2) from None except click.exceptions.Exit as error: diff --git a/amplifier_app_cli/lib/diagnostics.py b/amplifier_app_cli/lib/diagnostics.py index b6a6b205..e667bdb4 100644 --- a/amplifier_app_cli/lib/diagnostics.py +++ b/amplifier_app_cli/lib/diagnostics.py @@ -459,6 +459,81 @@ def emit(row): return report +def render_summary(report: dict) -> str: + """Render a fixed-copy checklist, not effective settings or a health verdict.""" + status_copy = { + "valid": "Read", + "empty": "No entries", + "missing": "Not present (optional)", + "invalid_yaml": "Could not parse format or encoding", + "invalid_shape": "Could not inspect: expected named settings", + "unsupported_yaml": "Doctor cannot inspect this YAML syntax", + "unsupported_file": "Not read: not a regular file", + "unsupported_symlink": "Not read: symbolic links are not inspected", + "unreadable": "Could not read", + "too_large": "Doctor's inspection limit was reached", + } + notice_copy = { + "ownership_conflict": "Collection stopped: This installation is linked to another Amplifier home.", + "ownership_unknown": "Collection stopped: Installation ownership could not be checked.", + "target_unavailable": "Collection stopped: The local environment could not be inspected.", + "invalid_session_id": "Collection stopped: The session selection was rejected.", + "distribution_metadata_unavailable": "Some installation details could not be read or shown.", + "distribution_version_withheld": "Some installation details could not be read or shown.", + "path_withheld": "Some location details were withheld.", + "declarations_truncated": "Some configuration entries were omitted.", + "environment_channels_truncated": "Some environment entries were omitted.", + } + collection = report["collection_status"] + lines = [ + "Amplifier doctor", + { + "complete": "Configuration report collected", + "partial": "Configuration report incomplete", + }.get(collection, "Configuration report could not be collected"), + "", + ] + scopes = [ + ("global", "User settings (global)"), + ("project", "Project settings"), + ("local", "Local settings"), + ] + if report["target"]["session_selected"]: + scopes.append(("session", "Session settings")) + explained = set() + for scope, label in scopes: + row = next((row for row in report["scopes"] if row["scope"] == scope), {}) + status = row.get("status") + copy = "Not read" if collection == "refused" else status_copy.get(status, "Not read") + lines.append(label + ": " + copy) + if collection != "refused" and status in status_copy: + explained.add((scope, "settings_" + status)) + lines.append("AI connections and running session: Not tested") + + notices = [] + if report["ownership"]["status"] == "override": + notices.append("An existing ownership override allowed collection.") + for issue in report["issues"]: + code = issue["code"] + if code in notice_copy: + copy = notice_copy[code] + elif (issue.get("scope"), code) in explained: + continue + else: + copy = "Other collection limits were reported; see technical details." + if copy not in notices: + notices.append(copy) + lines.extend(notices) + lines.extend(( + "Effective settings were not resolved.", + "No settings were changed.", + "These results do not confirm Amplifier is working.", + "Next: Tell Amplifier what went wrong and ask for the CLI doctor agent.", + "Technical details: amplifier doctor --verbose (review before sharing).", + )) + return "\n".join(lines) + + def render_text(report: dict) -> str: """Render the allowlisted report, with no resolution or health inference.""" lines = [ diff --git a/tests/check_doctor_wheel.py b/tests/check_doctor_wheel.py index 1314b71c..aa4f42bc 100644 --- a/tests/check_doctor_wheel.py +++ b/tests/check_doctor_wheel.py @@ -65,31 +65,56 @@ def _run(command, args, env, project, code, *, collection=None): if code == 2: assert result.stdout == "" assert errors == [ - "Usage: amplifier doctor [--format text|json] [--session SESSION_ID]", + "Usage: amplifier doctor [--format text|json] [--session SESSION_ID] [--verbose]", "Error: invalid_doctor_arguments", ] else: assert errors == [], errors + if "--help" in args: + assert "Show technical details (text only; does not affect JSON)." in " ".join(result.stdout.split()) if collection is not None: - assert result.stdout.count("\n") == 1 - report = json.loads(result.stdout) - assert report["collection_status"] == collection - assert report["health"] == {"status": "not_assessed", "reason": "collection_only"} - return report + if "json" in args: + assert result.stdout.count("\n") == 1 + report = json.loads(result.stdout) + assert report["collection_status"] == collection + assert report["health"] == {"status": "not_assessed", "reason": "collection_only"} + return report + if "--verbose" in args: + assert result.stdout.startswith("Amplifier doctor (passive)\n") + assert "Collection: " + collection + " (not a health verdict)" in result.stdout + assert "Effective settings: unknown (effective_resolution_not_performed)" in result.stdout + else: + header = { + "complete": "Configuration report collected", + "partial": "Configuration report incomplete", + "refused": "Configuration report could not be collected", + }[collection] + assert result.stdout.startswith("Amplifier doctor\n" + header + "\n") + assert "AI connections and running session: Not tested" in result.stdout + assert "These results do not confirm Amplifier is working." in result.stdout + assert str(project) not in result.stdout + assert env["AMPLIFIER_HOME"] not in result.stdout + return result.stdout def _check_entry(command, root, env, ownership_pth): project = root / "project" app_home = Path(env["AMPLIFIER_HOME"]) for args, code, status in ( - (["--format", "json"], 0, "complete"), ([], 0, None), + (["--format", "json"], 0, "complete"), ([], 0, "complete"), + (["--verbose"], 0, "complete"), + (["--format", "text", "--verbose"], 0, "complete"), + (["--format", "json", "--verbose"], 0, "complete"), (["--help"], 0, None), + (["--verbose", "--help"], 0, None), (["--session", "synthetic_session-01", "--format", "json"], 0, "complete"), (["--format", "synthetic-secret"], 2, None), (["--unknown=synthetic-secret"], 2, None), (["--session", "../synthetic-secret"], 2, None), (["--session"], 2, None), (["synthetic-secret"], 2, None), + (["--verbose=synthetic-secret"], 2, None), + (["--verbose", "--format", "synthetic-secret"], 2, None), ): _run(command, args, env, project, code, collection=status) assert not app_home.exists() @@ -100,7 +125,13 @@ def _check_entry(command, root, env, ownership_pth): settings.write_text("bundle: [synthetic-secret", encoding="utf-8") before = settings.read_bytes() try: - _run(command, ["--format", "json"], env, project, 1, collection="partial") + for args in ( + ["--format", "json"], ["--format", "json", "--verbose"], + [], ["--verbose"], ["--format", "text", "--verbose"], + ): + output = _run(command, args, env, project, 1, collection="partial") + if not args: + assert "User settings (global): Could not parse format or encoding" in output assert settings.read_bytes() == before assert list(app_home.iterdir()) == [settings] finally: @@ -116,12 +147,23 @@ def _check_entry(command, root, env, ownership_pth): guarded_env = dict(env) if override: guarded_env["AMPLIFIER_ALLOW_SHARED_VENV"] = "1" - report = _run( - command, ["--format", "json"], guarded_env, project, code, - collection=collection, - ) - assert report["ownership"]["status"] == status - assert report["ownership"]["foreign_count"] == 1 + for args in ( + ["--format", "json"], ["--format", "json", "--verbose"], + [], ["--verbose"], ["--format", "text", "--verbose"], + ): + output = _run(command, args, guarded_env, project, code, collection=collection) + if "json" in args: + assert output["ownership"]["status"] == status + assert output["ownership"]["foreign_count"] == 1 + elif "--verbose" in args: + assert "Ownership: " + status in output + elif override: + assert "An existing ownership override allowed collection." in output + else: + assert "User settings (global): Not read" in output + assert "Project settings: Not read" in output + assert "Local settings: Not read" in output + assert "Collection stopped: This installation is linked to another Amplifier home." in output assert not app_home.exists() assert not list(project.iterdir()) finally: diff --git a/tests/test_doctor.py b/tests/test_doctor.py index 5f2bc646..30f092f7 100644 --- a/tests/test_doctor.py +++ b/tests/test_doctor.py @@ -1,5 +1,6 @@ """Synthetic collector fixtures: nothing here comes from a real user home.""" +from copy import deepcopy import importlib.metadata as metadata import json import os @@ -44,6 +45,7 @@ def safe_report(target): report = diag.collect_diagnostics(target) assert SECRET not in json.dumps(report) assert SECRET not in diag.render_text(report) + assert SECRET not in diag.render_summary(report) return report @@ -58,6 +60,20 @@ def test_missing_files_complete_without_creating_home(target): assert report["effective_settings"]["reason"] == "effective_resolution_not_performed" assert report["live_session"]["reason"] == "not_observed" assert not target.home.exists() + assert diag.render_summary(report).splitlines() == [ + "Amplifier doctor", + "Configuration report collected", + "", + "User settings (global): Not present (optional)", + "Project settings: Not present (optional)", + "Local settings: Not present (optional)", + "AI connections and running session: Not tested", + "Effective settings were not resolved.", + "No settings were changed.", + "These results do not confirm Amplifier is working.", + "Next: Tell Amplifier what went wrong and ask for the CLI doctor agent.", + "Technical details: amplifier doctor --verbose (review before sharing).", + ] @pytest.mark.parametrize("text,status", [ @@ -198,6 +214,9 @@ def test_limits_are_explicit(target, monkeypatch): assert report["environment_channels"]["module_count"] == 210 assert report["collection_status"] == "partial" assert {"declarations_truncated", "environment_channels_truncated"} <= {r["code"] for r in report["issues"]} + summary = diag.render_summary(report) + assert "Some configuration entries were omitted." in summary + assert "Some environment entries were omitted." in summary def test_regular_files_only(target): @@ -254,6 +273,7 @@ def test_guard_refusal_stops_collection_and_override_is_reported(target, monkeyp report = safe_report(target) assert report["collection_status"] == "complete" assert report["ownership"]["status"] == "override" + assert "An existing ownership override allowed collection." in diag.render_summary(report) assert not target.home.exists() @@ -267,6 +287,7 @@ def fail(*args, **kwargs): assert report["ownership"]["status"] == "unknown" assert report["runtime"] is None assert report["issues"] == [{"code": "ownership_unknown"}] + assert "Collection stopped: Installation ownership could not be checked." in diag.render_summary(report) @pytest.mark.parametrize("suffix", ["\x1b[31m", "\u202e", "/https://user:password@example.test", "?token=synthetic"]) @@ -276,6 +297,7 @@ def test_unsafe_path_withheld(target, tmp_path, suffix): assert report["target"]["cwd"] is None assert {"code": "path_withheld"} in report["issues"] assert suffix not in diag.render_text(report) + assert "Some location details were withheld." in diag.render_summary(report) assert report["collection_status"] == "partial" @@ -346,6 +368,7 @@ def fail(name): monkeypatch.setattr(metadata, "distribution", fail) report = safe_report(target) assert report["collection_status"] == "partial" + assert "Some installation details could not be read or shown." in diag.render_summary(report) def test_text_and_json_exit_contract(target): @@ -360,4 +383,174 @@ def test_text_and_json_exit_contract(target): assert json.loads(result.output)["collection_status"] == "partial" result = runner.invoke(doctor) assert result.exit_code == 1 - assert "not a health verdict" in result.output \ No newline at end of file + assert "Configuration report incomplete" in result.output + result = runner.invoke(doctor, ["--verbose"]) + assert result.exit_code == 1 + assert "not a health verdict" in result.output + + +@pytest.mark.parametrize("status,copy", [ + ("valid", "Read"), + ("empty", "No entries"), + ("missing", "Not present (optional)"), + ("invalid_yaml", "Could not parse format or encoding"), + ("invalid_shape", "Could not inspect: expected named settings"), + ("unsupported_yaml", "Doctor cannot inspect this YAML syntax"), + ("unsupported_file", "Not read: not a regular file"), + ("unsupported_symlink", "Not read: symbolic links are not inspected"), + ("unreadable", "Could not read"), + ("too_large", "Doctor's inspection limit was reached"), + ("unknown", "Not read"), + ("not_selected", "Not read"), + ("\x1b[31mfuture-private-status", "Not read"), +], ids=[ + "read", "empty", "missing", "format", "shape", "syntax", "file", "symlink", + "unreadable", "limit", "unknown", "not-selected", "future", +]) +def test_summary_scope_statuses(target, status, copy): + report = safe_report(target) + report["scopes"][0]["status"] = status + assert diag.render_summary(report).splitlines()[3] == "User settings (global): " + copy + + +@pytest.mark.parametrize("selected", [False, True], ids=["no-session", "session"]) +def test_summary_refusal_never_claims_scopes_were_read(target, selected): + report = safe_report(target) + report["target"]["session_selected"] = selected + report["collection_status"] = "refused" + report["issues"] = [{"code": "ownership_conflict"}] + for row, status in zip(report["scopes"], ("valid", "missing", "empty", "not_selected")): + row["status"] = status + lines = diag.render_summary(report).splitlines() + assert lines[1] == "Configuration report could not be collected" + expected = [ + "User settings (global): Not read", "Project settings: Not read", "Local settings: Not read", + ] + if selected: + expected.append("Session settings: Not read") + assert lines[3:3 + len(expected)] == expected + assert ("Session settings: Not read" in lines) is selected + assert "Collection stopped: This installation is linked to another Amplifier home." in lines + + +def test_summary_selected_session_only_and_fixed_order(target): + report = safe_report(diag.DiagnosticTarget(target.cwd, target.home, "synthetic_session")) + report["scopes"].reverse() + lines = diag.render_summary(report).splitlines() + assert lines[3:8] == [ + "User settings (global): Not present (optional)", + "Project settings: Not present (optional)", + "Local settings: Not present (optional)", + "Session settings: Not present (optional)", + "AI connections and running session: Not tested", + ] + report["target"]["session_selected"] = False + report["scopes"][0]["status"] = "valid" + assert "Session settings:" not in diag.render_summary(report) + + +def test_summary_notices_are_all_preserved_and_deduplicated(target): + report = safe_report(target) + report["collection_status"] = "partial" + report["ownership"]["status"] = "override" + report["issues"] = [ + {"code": code} for code in ( + "ownership_conflict", "ownership_unknown", "target_unavailable", "invalid_session_id", + "distribution_metadata_unavailable", "distribution_version_withheld", + "path_withheld", "declarations_truncated", "environment_channels_truncated", + "future_limit", "future_limit", + ) + ] * 2 + lines = diag.render_summary(report).splitlines() + for notice in ( + "Collection stopped: This installation is linked to another Amplifier home.", + "Collection stopped: Installation ownership could not be checked.", + "Collection stopped: The local environment could not be inspected.", + "Collection stopped: The session selection was rejected.", + "An existing ownership override allowed collection.", + "Some installation details could not be read or shown.", + "Some location details were withheld.", + "Some configuration entries were omitted.", + "Some environment entries were omitted.", + "Other collection limits were reported; see technical details.", + ): + assert lines.count(notice) == 1 + report["collection_status"] = "complete" + report["issues"] = [] + assert "An existing ownership override allowed collection." in diag.render_summary(report) + + +def test_summary_scope_issues_explained_only_when_represented(target): + report = safe_report(target) + report["scopes"][0]["status"] = "unsupported_yaml" + report["issues"] = [{"code": "settings_unsupported_yaml", "scope": "global"}] + assert "Other collection limits" not in diag.render_summary(report) + for issue in ( + {"code": "settings_unsupported_yaml", "scope": "session"}, + {"code": "settings_unsupported_yaml"}, + {"code": "settings_unreadable", "scope": "global"}, + {"code": "settings_future", "scope": "global"}, + {"code": "future", "scope": "global"}, + {"code": "settings_unsupported_yaml", "scope": "future"}, + ): + report["issues"] = [issue] + assert "Other collection limits were reported; see technical details." in diag.render_summary(report) + + +def test_summary_is_fixed_ascii_copy_and_does_not_mutate_report(target): + report = safe_report(target) + payload = "private-target-\x1b[31m\u202e\N{SNOWMAN}" + report["target"].update(cwd=payload, home=payload, session_selected=True) + report["runtime"] = {"version": payload, "local_target": payload} + report["declarations"] = [{"value_ref": payload, "source_kind": payload}] + report["environment_channels"] = {"modules": [payload]} + report["ownership"]["status"] = payload + report["scopes"][0].update(path=payload, status=payload) + report["scopes"][3].update(path=payload, status="valid", session_id=payload) + report["issues"] = [{"code": payload, "scope": payload}] + before = deepcopy(report) + summary = diag.render_summary(report) + assert report == before + assert summary.isascii() + assert payload not in summary + assert "Other collection limits were reported; see technical details." in summary + assert "All checks pass" not in summary + assert "Session settings: Read" in summary + + +@pytest.mark.parametrize("status,code", [ + ("complete", 0), ("partial", 1), ("refused", 1), +], ids=["complete", "partial", "refused"]) +def test_json_bytes_unchanged_with_verbose_and_same_collector(target, status, code): + if status == "partial": + write_settings(target, "bundle: [") + elif status == "refused": + foreign = target.home.parent / "foreign-home" / "cache" / "synthetic-0123456789abcdef" + (guard.site_packages_dir() / "synthetic.pth").write_text(str(foreign), encoding="utf-8") + report = safe_report(target) + assert report["collection_status"] == status + expected = (json.dumps(report, ensure_ascii=True) + "\n").encode() + runner = CliRunner() + for args in (["--format", "json"], ["--format", "json", "--verbose"]): + result = runner.invoke(doctor, args) + assert result.exit_code == code, result.output + assert result.stdout_bytes == expected + + +@pytest.mark.parametrize("args", [["--verbose"], ["--format", "text", "--verbose"]], + ids=["verbose", "text-verbose"]) +def test_verbose_is_exact_original_technical_renderer(target, args): + write_settings(target, "bundle: {active: synthetic}") + report = safe_report(target) + result = CliRunner().invoke(doctor, args) + assert result.exit_code == 0 + assert result.output == diag.render_text(report) + "\n" + + +def test_help_explains_presentation_and_saved_session(target): + result = CliRunner().invoke(doctor, ["--help"], terminal_width=120) + assert result.exit_code == 0 + assert "Read local configuration without changing settings." in result.output + assert "text checklist (default)" in result.output + assert "exact session's settings, not its live state." in result.output + assert "Show technical details (text only; does not affect JSON)." in result.output \ No newline at end of file diff --git a/tests/test_doctor_entrypoint.py b/tests/test_doctor_entrypoint.py index e99acf20..24e67b17 100644 --- a/tests/test_doctor_entrypoint.py +++ b/tests/test_doctor_entrypoint.py @@ -77,7 +77,11 @@ def _run(tmp_path, kind, args, *, settings=None): @pytest.mark.parametrize("args,code", [ (["--format", "json"], 0), ([], 0), + (["--verbose"], 0), + (["--format", "text", "--verbose"], 0), + (["--format", "json", "--verbose"], 0), (["--help"], 0), + (["--verbose", "--help"], 0), (["--session", "synthetic_session-01", "--format", "json"], 0), (["--format", "synthetic-secret"], 2), (["--format", "\x1b[31msynthetic-secret"], 2), @@ -85,6 +89,12 @@ def _run(tmp_path, kind, args, *, settings=None): (["--session", "../synthetic-secret"], 2), (["--session"], 2), (["synthetic-secret"], 2), + (["--verbose=synthetic-secret"], 2), + (["--verbose", "--format", "synthetic-secret"], 2), +], ids=[ + "json", "summary", "verbose", "text-verbose", "json-verbose", "help", "verbose-help", + "session-json", "bad-format", "control-format", "unknown", "bad-session", + "missing-session", "extra", "verbose-value", "verbose-bad-format", ]) def test_published_entries_are_passive(tmp_path, kind, args, code): result, errors = _run(tmp_path, kind, args) @@ -93,21 +103,41 @@ def test_published_entries_are_passive(tmp_path, kind, args, code): if code == 2: assert result.stdout == "" assert errors == [ - "Usage: amplifier doctor [--format text|json] [--session SESSION_ID]", + "Usage: amplifier doctor [--format text|json] [--session SESSION_ID] [--verbose]", "Error: invalid_doctor_arguments", ] else: assert errors == [] - if args == ["--format", "json"]: + if "json" in args: assert result.stdout.count("\n") == 1 assert json.loads(result.stdout)["collection_status"] == "complete" + elif code == 0 and "--help" in args: + assert "--verbose" in result.stdout + assert "Show technical details (text only; does not affect JSON)." in " ".join(result.stdout.split()) + elif code == 0 and "--verbose" in args: + assert result.stdout.startswith("Amplifier doctor (passive)\n") + assert "Collection: complete (not a health verdict)" in result.stdout + elif code == 0: + assert result.stdout.startswith("Amplifier doctor\nConfiguration report collected\n") + assert str(tmp_path) not in result.stdout @pytest.mark.parametrize("kind", ["console", "module", "legacy_module"]) -def test_real_partial_entry_returns_one(tmp_path, kind): - result, errors = _run(tmp_path, kind, ["--format", "json"], settings="bundle: [synthetic-secret") +@pytest.mark.parametrize("args", [ + ["--format", "json"], [], ["--verbose"], ["--format", "text", "--verbose"], + ["--format", "json", "--verbose"], +], ids=["json", "summary", "verbose", "text-verbose", "json-verbose"]) +def test_real_partial_entry_returns_one(tmp_path, kind, args): + result, errors = _run(tmp_path, kind, args, settings="bundle: [synthetic-secret") assert result.returncode == 1 - assert json.loads(result.stdout)["collection_status"] == "partial" + if "json" in args: + assert json.loads(result.stdout)["collection_status"] == "partial" + elif "--verbose" in args: + assert "Collection: partial (not a health verdict)" in result.stdout + assert "global: invalid_yaml " in result.stdout + else: + assert result.stdout.startswith("Amplifier doctor\nConfiguration report incomplete\n") + assert "User settings (global): Could not parse format or encoding" in result.stdout assert "synthetic-secret" not in result.stdout + result.stderr assert errors == [] @@ -216,6 +246,32 @@ def _run_import_check(tmp_path, code, *args): return result +def test_ordinary_click_group_uses_same_doctor_flag_and_json_contract(tmp_path): + code = """ +import importlib +import json +from click.testing import CliRunner +from amplifier_app_cli import cli +from amplifier_app_cli.lib import diagnostics +module = importlib.import_module('amplifier_app_cli.commands.doctor') +assert cli.commands['doctor'] is module.doctor +assert next(p for p in module.doctor.params if p.name == 'verbose').is_flag +runner = CliRunner() +for status, exit_code in (('complete', 0), ('partial', 1), ('refused', 1)): + report = diagnostics.unavailable_report() + report['collection_status'] = status + module.collect_diagnostics = lambda target: report + for args in (['--format', 'json'], ['--format', 'json', '--verbose']): + result = runner.invoke(cli, ['doctor', *args]) + assert result.exit_code == exit_code, result.output + assert result.stdout_bytes == (json.dumps(report, ensure_ascii=True) + '\\n').encode() + result = runner.invoke(cli, ['doctor', '--verbose']) + assert result.exit_code == exit_code, result.output + assert result.output == diagnostics.render_text(report) + '\\n' +""" + _run_import_check(tmp_path, code) + + @pytest.mark.parametrize("route", ["ordinary", "foreign_doctor"]) @pytest.mark.parametrize("order", ["package_first", "main_first", "module_cli", "command_first", "submodule_first"]) def test_ordinary_exports_and_command_identity(order, route, tmp_path): diff --git a/tests/test_shell_completion.py b/tests/test_shell_completion.py index 582abfb1..6d50e7b2 100644 --- a/tests/test_shell_completion.py +++ b/tests/test_shell_completion.py @@ -346,7 +346,7 @@ def test_actual_cli_protocol_emits_native_dynamic_records( ( "amplifier doctor --", "2", - {"plain,--format", "plain,--session", "plain,--help"}, + {"plain,--format", "plain,--session", "plain,--verbose", "plain,--help"}, ), ("amplifier doctor --format ", "3", {"plain,text", "plain,json"}), ( From e136d9a063a2af2fda4aad53b14a17f4cb568f12 Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Mon, 26 Oct 2026 08:20:45 -0700 Subject: [PATCH 2/2] test: respect native newlines in doctor JSON assertions Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- tests/test_doctor.py | 5 ++++- tests/test_doctor_entrypoint.py | 7 ++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_doctor.py b/tests/test_doctor.py index 30f092f7..27063f4d 100644 --- a/tests/test_doctor.py +++ b/tests/test_doctor.py @@ -529,12 +529,15 @@ def test_json_bytes_unchanged_with_verbose_and_same_collector(target, status, co (guard.site_packages_dir() / "synthetic.pth").write_text(str(foreign), encoding="utf-8") report = safe_report(target) assert report["collection_status"] == status - expected = (json.dumps(report, ensure_ascii=True) + "\n").encode() + expected = (json.dumps(report, ensure_ascii=True) + os.linesep).encode("utf-8") runner = CliRunner() + outputs = [] for args in (["--format", "json"], ["--format", "json", "--verbose"]): result = runner.invoke(doctor, args) assert result.exit_code == code, result.output assert result.stdout_bytes == expected + outputs.append(result.stdout_bytes) + assert outputs[0] == outputs[1] @pytest.mark.parametrize("args", [["--verbose"], ["--format", "text", "--verbose"]], diff --git a/tests/test_doctor_entrypoint.py b/tests/test_doctor_entrypoint.py index 24e67b17..186c20db 100644 --- a/tests/test_doctor_entrypoint.py +++ b/tests/test_doctor_entrypoint.py @@ -250,6 +250,7 @@ def test_ordinary_click_group_uses_same_doctor_flag_and_json_contract(tmp_path): code = """ import importlib import json +import os from click.testing import CliRunner from amplifier_app_cli import cli from amplifier_app_cli.lib import diagnostics @@ -261,10 +262,14 @@ def test_ordinary_click_group_uses_same_doctor_flag_and_json_contract(tmp_path): report = diagnostics.unavailable_report() report['collection_status'] = status module.collect_diagnostics = lambda target: report + expected = (json.dumps(report, ensure_ascii=True) + os.linesep).encode('utf-8') + outputs = [] for args in (['--format', 'json'], ['--format', 'json', '--verbose']): result = runner.invoke(cli, ['doctor', *args]) assert result.exit_code == exit_code, result.output - assert result.stdout_bytes == (json.dumps(report, ensure_ascii=True) + '\\n').encode() + assert result.stdout_bytes == expected + outputs.append(result.stdout_bytes) + assert outputs[0] == outputs[1] result = runner.invoke(cli, ['doctor', '--verbose']) assert result.exit_code == exit_code, result.output assert result.output == diagnostics.render_text(report) + '\\n'