diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 0bbb2c200..eb4812514 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -223,6 +223,7 @@ Adding a new language has a dedicated recipe, see - Add tests for new features and bug fixes - Place tests in `tests/unit/` or `tests/integration/` - Run the full suite with `uv run pytest` +- A test asserting on a `caplog` record needs `caplog.set_level(logging.INFO, logger="")` (or the level you're asserting on) — setting the root logger's level is not enough when an ancestor logger (e.g. `repowise.core`, `repowise.server`) has its own level raised, since Python resolves the *effective* level from the nearest ancestor that has one set, not from root. ## Pull Request Guidelines diff --git a/packages/cli/src/repowise/cli/commands/_tool_adapters.py b/packages/cli/src/repowise/cli/commands/_tool_adapters.py index c47fc8aee..7b4be1af9 100644 --- a/packages/cli/src/repowise/cli/commands/_tool_adapters.py +++ b/packages/cli/src/repowise/cli/commands/_tool_adapters.py @@ -139,8 +139,8 @@ def run(repo_path: Path, factory: Callable[[], Awaitable[dict]], tool_name: str) from repowise.cli.helpers import silence_logs_for_machine_output from repowise.cli.tool_bridge import call_tool - silence_logs_for_machine_output() - return call_tool(repo_path, factory, tool_name) + with silence_logs_for_machine_output(): + return call_tool(repo_path, factory, tool_name) #: MCP tool name -> the CLI command that now does the same thing. diff --git a/packages/cli/src/repowise/cli/commands/dead_code_cmd.py b/packages/cli/src/repowise/cli/commands/dead_code_cmd.py index b5470a316..4840a7d77 100644 --- a/packages/cli/src/repowise/cli/commands/dead_code_cmd.py +++ b/packages/cli/src/repowise/cli/commands/dead_code_cmd.py @@ -2,6 +2,7 @@ from __future__ import annotations +import contextlib import json import click @@ -98,221 +99,220 @@ def dead_code_command( --repo to target a different repo. Cross-repo dead-code detection is not yet supported — run once per repo for now. """ - if fmt != "table": - silence_logs_for_machine_output() + silencer = silence_logs_for_machine_output() if fmt != "table" else contextlib.nullcontext() + with silencer: + from pathlib import Path as PathlibPath - from pathlib import Path as PathlibPath + from repowise.core.analysis.dead_code import DeadCodeAnalyzer + from repowise.core.ingestion import ASTParser, FileTraverser, GraphBuilder - from repowise.core.analysis.dead_code import DeadCodeAnalyzer - from repowise.core.ingestion import ASTParser, FileTraverser, GraphBuilder - - target = resolve_command_target( - path=path, - no_workspace_flag=no_workspace, - repo_alias=repo_alias, - ) - notices = notice_console(fmt) - target.notice(notices, command="dead-code") + target = resolve_command_target( + path=path, + no_workspace_flag=no_workspace, + repo_alias=repo_alias, + ) + notices = notice_console(fmt) + target.notice(notices, command="dead-code") - if target.is_workspace: - if target.repo_filter is not None: - picked = target.resolve_repo_alias(target.repo_filter) - if picked is None: - raise click.ClickException(f"Unknown repo alias: {target.repo_filter}") - repo_path = picked + if target.is_workspace: + if target.repo_filter is not None: + picked = target.resolve_repo_alias(target.repo_filter) + if picked is None: + raise click.ClickException(f"Unknown repo alias: {target.repo_filter}") + repo_path = picked + else: + primary = target.primary_path() + if primary is None: + raise click.ClickException("Workspace has no primary repo configured.") + repo_path = primary + notices.print("[dim] (Tip: pass --repo to analyze a different repo.)[/dim]") else: - primary = target.primary_path() - if primary is None: - raise click.ClickException("Workspace has no primary repo configured.") - repo_path = primary - notices.print("[dim] (Tip: pass --repo to analyze a different repo.)[/dim]") - else: - assert target.repo_path is not None - repo_path = target.repo_path + assert target.repo_path is not None + repo_path = target.repo_path - notices.print(f"[bold]repowise dead-code[/bold] — {repo_path}") + notices.print(f"[bold]repowise dead-code[/bold] — {repo_path}") - # Ingest — honor the persisted submodule flags so the analyzed file set - # matches what `init` indexed (a flagless traverser on a submodule-indexed - # repo would drop submodule files and skew reachability). - state = load_state(repo_path) - include_submodules = bool(state.get("include_submodules", False)) - include_nested_repos = bool(state.get("include_nested_repos", False)) + # Ingest — honor the persisted submodule flags so the analyzed file set + # matches what `init` indexed (a flagless traverser on a submodule-indexed + # repo would drop submodule files and skew reachability). + state = load_state(repo_path) + include_submodules = bool(state.get("include_submodules", False)) + include_nested_repos = bool(state.get("include_nested_repos", False)) - traverser = FileTraverser( - repo_path, - include_submodules=include_submodules, - include_nested_repos=include_nested_repos, - ) - file_infos = list(traverser.traverse()) - parser = ASTParser() - graph_builder = GraphBuilder( - repo_path, - include_submodules=include_submodules, - include_nested_repos=include_nested_repos, - ) + traverser = FileTraverser( + repo_path, + include_submodules=include_submodules, + include_nested_repos=include_nested_repos, + ) + file_infos = list(traverser.traverse()) + parser = ASTParser() + graph_builder = GraphBuilder( + repo_path, + include_submodules=include_submodules, + include_nested_repos=include_nested_repos, + ) - # Kept alongside the parse so the analyzer's marker prepasses reuse these - # bytes instead of re-reading the repo four more times. - source_map: dict[str, bytes] = {} - for fi in file_infos: - try: - source = PathlibPath(fi.abs_path).read_bytes() - parsed = parser.parse_file(fi, source) - graph_builder.add_file(parsed) - source_map[fi.path] = source - except Exception: - pass + # Kept alongside the parse so the analyzer's marker prepasses reuse these + # bytes instead of re-reading the repo four more times. + source_map: dict[str, bytes] = {} + for fi in file_infos: + try: + source = PathlibPath(fi.abs_path).read_bytes() + parsed = parser.parse_file(fi, source) + graph_builder.add_file(parsed) + source_map[fi.path] = source + except Exception: + pass - from repowise.core.ingestion import wire_tsconfig_resolver + from repowise.core.ingestion import wire_tsconfig_resolver - wire_tsconfig_resolver( - graph_builder, - repo_path, - include_submodules=include_submodules, - include_nested_repos=include_nested_repos, - ) - graph_builder.set_source_map(source_map) - graph_builder.build() + wire_tsconfig_resolver( + graph_builder, + repo_path, + include_submodules=include_submodules, + include_nested_repos=include_nested_repos, + ) + graph_builder.set_source_map(source_map) + graph_builder.build() - # Framework-aware synthetic edges (Django, Laravel, TYPO3, ...). Without - # this, convention-loaded files appear as in_degree=0 unreachable — false - # positives in the dead-code report. - try: - from repowise.core.generation.editor_files.tech_stack import detect_tech_stack + # Framework-aware synthetic edges (Django, Laravel, TYPO3, ...). Without + # this, convention-loaded files appear as in_degree=0 unreachable — false + # positives in the dead-code report. + try: + from repowise.core.generation.editor_files.tech_stack import detect_tech_stack - tech_items = detect_tech_stack(repo_path) - graph_builder.add_framework_edges([item.name for item in tech_items]) - except Exception as fw_exc: - # Silence here is the worst of the three sites: the comment above says - # these edges exist to stop false positives, so a swallowed failure - # hands the user a longer report and calls it the answer. - notices.print( - f"[yellow]Framework edge detection skipped: {fw_exc}; " - "convention-loaded files may report as unreachable.[/yellow]" - ) + tech_items = detect_tech_stack(repo_path) + graph_builder.add_framework_edges([item.name for item in tech_items]) + except Exception as fw_exc: + # Silence here is the worst of the three sites: the comment above says + # these edges exist to stop false positives, so a swallowed failure + # hands the user a longer report and calls it the answer. + notices.print( + f"[yellow]Framework edge detection skipped: {fw_exc}; " + "convention-loaded files may report as unreachable.[/yellow]" + ) - # Git metadata (best effort) - git_meta_map: dict = {} - try: - from repowise.core.ingestion.git_indexer import GitIndexer + # Git metadata (best effort) + git_meta_map: dict = {} + try: + from repowise.core.ingestion.git_indexer import GitIndexer - git_indexer = GitIndexer(repo_path) - _, metadata_list = run_async(git_indexer.index_repo("")) - git_meta_map = {m["file_path"]: m for m in metadata_list} - except Exception: - pass + git_indexer = GitIndexer(repo_path) + _, metadata_list = run_async(git_indexer.index_repo("")) + git_meta_map = {m["file_path"]: m for m in metadata_list} + except Exception: + pass - # Analyze - config: dict = { - "min_confidence": min_confidence, - "detect_unused_internals": include_internals, - "detect_zombie_packages": include_zombie_packages, - "detect_unreachable_files": not no_unreachable, - "detect_unused_exports": not no_unused_exports, - } - if kind: - # --kind overrides the individual detection flags to focus on one type - config["detect_unreachable_files"] = kind == "unreachable_file" - config["detect_unused_exports"] = kind == "unused_export" - config["detect_unused_internals"] = kind == "unused_internal" - config["detect_zombie_packages"] = kind == "zombie_package" + # Analyze + config: dict = { + "min_confidence": min_confidence, + "detect_unused_internals": include_internals, + "detect_zombie_packages": include_zombie_packages, + "detect_unreachable_files": not no_unreachable, + "detect_unused_exports": not no_unused_exports, + } + if kind: + # --kind overrides the individual detection flags to focus on one type + config["detect_unreachable_files"] = kind == "unreachable_file" + config["detect_unused_exports"] = kind == "unused_export" + config["detect_unused_internals"] = kind == "unused_internal" + config["detect_zombie_packages"] = kind == "zombie_package" - # parsed_files powers the source-scan rescues (dynamic-import markers, - # bundler resolve.alias targets, export-alias maps) — without it those - # classes false-positive on the CLI path while init stays clean. - analyzer = DeadCodeAnalyzer( - graph_builder.graph(), - git_meta_map, - parsed_files=graph_builder._parsed_files, - source_map=source_map, - repo_root=repo_path, - unindexed_source_files=[ - (skipped.path, skipped.reason) - for skipped in ( - *traverser.stats.skipped_source_files, - *traverser.stats.unknown_language_files, - ) - ], - ) - report = analyzer.analyze(config) + # parsed_files powers the source-scan rescues (dynamic-import markers, + # bundler resolve.alias targets, export-alias maps) — without it those + # classes false-positive on the CLI path while init stays clean. + analyzer = DeadCodeAnalyzer( + graph_builder.graph(), + git_meta_map, + parsed_files=graph_builder._parsed_files, + source_map=source_map, + repo_root=repo_path, + unindexed_source_files=[ + (skipped.path, skipped.reason) + for skipped in ( + *traverser.stats.skipped_source_files, + *traverser.stats.unknown_language_files, + ) + ], + ) + report = analyzer.analyze(config) - findings = report.findings - if safe_only: - findings = [f for f in findings if f.safe_to_delete] + findings = report.findings + if safe_only: + findings = [f for f in findings if f.safe_to_delete] - if fmt == "json": - output = [] - for f in findings: - output.append( - { - "kind": f.kind.value, - "file_path": f.file_path, - "symbol_name": f.symbol_name, - "confidence": f.confidence, - "reason": f.reason, - "safe_to_delete": f.safe_to_delete, - "risk_factors": f.risk_factors, - "lines": f.lines, - "primary_owner": f.primary_owner, - } - ) - click.echo(json.dumps(output, indent=2)) - return + if fmt == "json": + output = [] + for f in findings: + output.append( + { + "kind": f.kind.value, + "file_path": f.file_path, + "symbol_name": f.symbol_name, + "confidence": f.confidence, + "reason": f.reason, + "safe_to_delete": f.safe_to_delete, + "risk_factors": f.risk_factors, + "lines": f.lines, + "primary_owner": f.primary_owner, + } + ) + click.echo(json.dumps(output, indent=2)) + return + + if fmt == "md": + click.echo("# Dead Code Report\n") + click.echo(f"**Total findings:** {len(findings)}") + click.echo(f"**Cleanup-candidate lines:** {report.deletable_lines}\n") + for f in findings: + safe = " (cleanup-ready)" if f.safe_to_delete else "" + name = f"`{f.symbol_name}`" if f.symbol_name else f"`{f.file_path}`" + click.echo(f"- [{f.kind.value}] {name} — {f.reason} ({f.confidence:.0%}){safe}") + if report.hidden_below_threshold: + click.echo( + f"\n> {report.hidden_below_threshold} finding(s) hidden below threshold " + f"(confidence < {min_confidence:.2g}); " + f"pass `--min-confidence 0.0` to see them." + ) + return + + # Table format (default) + table = Table(title=f"Dead Code ({len(findings)} findings)") + table.add_column("Kind", style="cyan") + table.add_column("File / Symbol") + table.add_column("Confidence", justify="right") + table.add_column("Ready?", justify="center") + table.add_column("Lines", justify="right") + table.add_column("Reason") - if fmt == "md": - click.echo("# Dead Code Report\n") - click.echo(f"**Total findings:** {len(findings)}") - click.echo(f"**Cleanup-candidate lines:** {report.deletable_lines}\n") for f in findings: - safe = " (cleanup-ready)" if f.safe_to_delete else "" - name = f"`{f.symbol_name}`" if f.symbol_name else f"`{f.file_path}`" - click.echo(f"- [{f.kind.value}] {name} — {f.reason} ({f.confidence:.0%}){safe}") - if report.hidden_below_threshold: - click.echo( - f"\n> {report.hidden_below_threshold} finding(s) hidden below threshold " - f"(confidence < {min_confidence:.2g}); " - f"pass `--min-confidence 0.0` to see them." + name = f.symbol_name or f.file_path + safe = "[green]✓[/green]" if f.safe_to_delete else "[red]✗[/red]" + table.add_row( + f.kind.value, + name, + f"{f.confidence:.0%}", + safe, + str(f.lines), + f.reason[:60], ) - return - - # Table format (default) - table = Table(title=f"Dead Code ({len(findings)} findings)") - table.add_column("Kind", style="cyan") - table.add_column("File / Symbol") - table.add_column("Confidence", justify="right") - table.add_column("Ready?", justify="center") - table.add_column("Lines", justify="right") - table.add_column("Reason") - for f in findings: - name = f.symbol_name or f.file_path - safe = "[green]✓[/green]" if f.safe_to_delete else "[red]✗[/red]" - table.add_row( - f.kind.value, - name, - f"{f.confidence:.0%}", - safe, - str(f.lines), - f.reason[:60], + console.print(table) + # confidence_summary is a {"high": N, ...} dict; interpolating it printed a + # raw Python repr, braces and quotes included, at the end of an otherwise + # formatted report. + tiers = ", ".join( + f"{tier} {count}" + for tier in ("high", "medium", "low") + if (count := report.confidence_summary.get(tier, 0)) ) - - console.print(table) - # confidence_summary is a {"high": N, ...} dict; interpolating it printed a - # raw Python repr, braces and quotes included, at the end of an otherwise - # formatted report. - tiers = ", ".join( - f"{tier} {count}" - for tier in ("high", "medium", "low") - if (count := report.confidence_summary.get(tier, 0)) - ) - console.print( - f"\nCleanup-candidate lines: [bold]{report.deletable_lines:,}[/bold]" - + (f" ({tiers} confidence)" if tiers else "") - ) - if report.hidden_below_threshold: console.print( - f"[dim]{report.hidden_below_threshold} finding(s) hidden below " - f"threshold (confidence < {min_confidence:.2g}); " - f"pass --min-confidence 0.0 to see them.[/dim]" + f"\nCleanup-candidate lines: [bold]{report.deletable_lines:,}[/bold]" + + (f" ({tiers} confidence)" if tiers else "") ) + if report.hidden_below_threshold: + console.print( + f"[dim]{report.hidden_below_threshold} finding(s) hidden below " + f"threshold (confidence < {min_confidence:.2g}); " + f"pass --min-confidence 0.0 to see them.[/dim]" + ) diff --git a/packages/cli/src/repowise/cli/commands/doctor_cmd/command.py b/packages/cli/src/repowise/cli/commands/doctor_cmd/command.py index b2634a145..f1dd36669 100644 --- a/packages/cli/src/repowise/cli/commands/doctor_cmd/command.py +++ b/packages/cli/src/repowise/cli/commands/doctor_cmd/command.py @@ -2,6 +2,7 @@ from __future__ import annotations +import contextlib import json import click @@ -61,96 +62,95 @@ def doctor_command( "--repair is not supported with --format json (json mode is read-only)." ) - if fmt != "table": - silence_logs_for_machine_output() + silencer = silence_logs_for_machine_output() if fmt != "table" else contextlib.nullcontext() + with silencer: + status = err_console if fmt != "table" else console - status = err_console if fmt != "table" else console - - target = resolve_command_target( - path=path, - workspace_flag=workspace, - no_workspace_flag=no_workspace, - ) - target.notice(status, command="doctor") + target = resolve_command_target( + path=path, + workspace_flag=workspace, + no_workspace_flag=no_workspace, + ) + target.notice(status, command="doctor") - if fmt == "table": - # Advisory CLI update check, printed once above the repo check table(s). - _print_cli_version_status() + if fmt == "table": + # Advisory CLI update check, printed once above the repo check table(s). + _print_cli_version_status() + + if not target.is_workspace: + assert target.repo_path is not None + all_ok, checks = _run_repo_checks(target.repo_path, repair, fmt=fmt) + if fmt != "table": + payload = {"ok": all_ok, "checks": [c._asdict() for c in checks]} + click.echo(json.dumps(payload, indent=2)) + if not all_ok: + raise SystemExit(1) + return + + # Workspace mode — iterate over every entry, run workspace-level + # validation, and report a summary table at the end so the user knows + # which repos need attention. + assert target.ws_root is not None and target.ws_config is not None + ws_root = target.ws_root + ws_config = target.ws_config + + ws_issues = _run_workspace_checks(ws_root, ws_config, repair=repair, fmt=fmt) + + overall_ok = True + not_indexed: list[str] = [] + all_checks: list[DoctorCheck] = [] + for entry in ws_config.repos: + abs_path = (ws_root / entry.path).resolve() + if not abs_path.is_dir(): + continue + if not (abs_path / ".repowise").is_dir(): + not_indexed.append(entry.alias) + continue + if fmt == "table": + console.print() + console.print( + f"[bold]── {entry.alias}[/bold] " + f"[dim]({entry.path})[/dim]" + + ( + " [bold cyan](primary)[/bold cyan]" + if entry.alias == ws_config.default_repo + else "" + ) + ) + ok, checks = _run_repo_checks(abs_path, repair, fmt=fmt) + overall_ok = overall_ok and ok + all_checks.extend(DoctorCheck(f"{entry.alias}: {c.name}", c.ok, c.detail) for c in checks) - if not target.is_workspace: - assert target.repo_path is not None - all_ok, checks = _run_repo_checks(target.repo_path, repair, fmt=fmt) if fmt != "table": - payload = {"ok": all_ok, "checks": [c._asdict() for c in checks]} + all_ok = overall_ok and not ws_issues and not not_indexed + payload = { + "ok": all_ok, + "checks": [c._asdict() for c in all_checks], + "workspace": { + "checked": True, + "issues": list(ws_issues), + "not_indexed": not_indexed, + }, + } click.echo(json.dumps(payload, indent=2)) if not all_ok: raise SystemExit(1) - return - - # Workspace mode — iterate over every entry, run workspace-level - # validation, and report a summary table at the end so the user knows - # which repos need attention. - assert target.ws_root is not None and target.ws_config is not None - ws_root = target.ws_root - ws_config = target.ws_config - - ws_issues = _run_workspace_checks(ws_root, ws_config, repair=repair, fmt=fmt) - - overall_ok = True - not_indexed: list[str] = [] - all_checks: list[DoctorCheck] = [] - for entry in ws_config.repos: - abs_path = (ws_root / entry.path).resolve() - if not abs_path.is_dir(): - continue - if not (abs_path / ".repowise").is_dir(): - not_indexed.append(entry.alias) - continue - if fmt == "table": - console.print() + return + + console.print() + if not_indexed: + console.print(f"[yellow]Not indexed:[/yellow] {', '.join(not_indexed)}") + console.print(" Run [bold]repowise update --workspace[/bold] to index them.") + if ws_issues and not repair: console.print( - f"[bold]── {entry.alias}[/bold] " - f"[dim]({entry.path})[/dim]" - + ( - " [bold cyan](primary)[/bold cyan]" - if entry.alias == ws_config.default_repo - else "" - ) + f"[yellow]{len(ws_issues)} workspace-level issue(s); " + f"rerun with [bold]--repair[/bold] to attempt fixes.[/yellow]" ) - ok, checks = _run_repo_checks(abs_path, repair, fmt=fmt) - overall_ok = overall_ok and ok - all_checks.extend(DoctorCheck(f"{entry.alias}: {c.name}", c.ok, c.detail) for c in checks) - - if fmt != "table": - all_ok = overall_ok and not ws_issues and not not_indexed - payload = { - "ok": all_ok, - "checks": [c._asdict() for c in all_checks], - "workspace": { - "checked": True, - "issues": list(ws_issues), - "not_indexed": not_indexed, - }, - } - click.echo(json.dumps(payload, indent=2)) - if not all_ok: - raise SystemExit(1) - return - - console.print() - if not_indexed: - console.print(f"[yellow]Not indexed:[/yellow] {', '.join(not_indexed)}") - console.print(" Run [bold]repowise update --workspace[/bold] to index them.") - if ws_issues and not repair: - console.print( - f"[yellow]{len(ws_issues)} workspace-level issue(s); " - f"rerun with [bold]--repair[/bold] to attempt fixes.[/yellow]" - ) - workspace_clean = not ws_issues and overall_ok and not not_indexed - if workspace_clean: - console.print("[bold green]Workspace healthy.[/bold green]") - elif overall_ok and not ws_issues: - console.print("[bold yellow]All indexed repos healthy; some repos unindexed.[/bold yellow]") - else: - console.print("[bold yellow]Some checks failed across the workspace.[/bold yellow]") + workspace_clean = not ws_issues and overall_ok and not not_indexed + if workspace_clean: + console.print("[bold green]Workspace healthy.[/bold green]") + elif overall_ok and not ws_issues: + console.print("[bold yellow]All indexed repos healthy; some repos unindexed.[/bold yellow]") + else: + console.print("[bold yellow]Some checks failed across the workspace.[/bold yellow]") diff --git a/packages/cli/src/repowise/cli/commands/health_cmd/command.py b/packages/cli/src/repowise/cli/commands/health_cmd/command.py index 57b9d7a4b..42d804d6d 100644 --- a/packages/cli/src/repowise/cli/commands/health_cmd/command.py +++ b/packages/cli/src/repowise/cli/commands/health_cmd/command.py @@ -7,6 +7,7 @@ from __future__ import annotations +import contextlib import json from pathlib import Path @@ -149,298 +150,297 @@ def health_command( # Silence structlog/stdlib info+debug lines when the user asked for a # machine-readable format so stdout is pure JSON/Markdown and safe to # pipe into jq or other tools (e.g. `repowise health --format json | jq .kpis`). - if fmt != "table": - silence_logs_for_machine_output() - - # Status output goes to stderr when the user asked for a machine-readable - # format — otherwise rich's banner pollutes stdout and breaks - # `repowise health --format json | jq …` (and the CI smoke test). - status = err_console if fmt != "table" else console - - target = resolve_command_target( - path=path, no_workspace_flag=no_workspace, repo_alias=repo_alias - ) - target.notice(status, command="health") - - if target.is_workspace: - if target.repo_filter is not None: - picked = target.resolve_repo_alias(target.repo_filter) - if picked is None: - raise click.ClickException(f"Unknown repo alias: {target.repo_filter}") - repo_path = picked + silencer = silence_logs_for_machine_output() if fmt != "table" else contextlib.nullcontext() + with silencer: + # Status output goes to stderr when the user asked for a machine-readable + # format — otherwise rich's banner pollutes stdout and breaks + # `repowise health --format json | jq …` (and the CI smoke test). + status = err_console if fmt != "table" else console + + target = resolve_command_target( + path=path, no_workspace_flag=no_workspace, repo_alias=repo_alias + ) + target.notice(status, command="health") + + if target.is_workspace: + if target.repo_filter is not None: + picked = target.resolve_repo_alias(target.repo_filter) + if picked is None: + raise click.ClickException(f"Unknown repo alias: {target.repo_filter}") + repo_path = picked + else: + primary = target.primary_path() + if primary is None: + raise click.ClickException("Workspace has no primary repo configured.") + repo_path = primary else: - primary = target.primary_path() - if primary is None: - raise click.ClickException("Workspace has no primary repo configured.") - repo_path = primary - else: - assert target.repo_path is not None - repo_path = target.repo_path - - status.print(f"[bold]repowise health[/bold] — {repo_path}") - - if trend_view: - _render_trend(repo_path, fmt=fmt) - return - - # Analyze the same file set that was indexed: a repo initialized with - # --include-submodules persists the flag in state.json, and a flagless - # traverser here would silently score a different (smaller) tree. - state = load_state(repo_path) - include_submodules = bool(state.get("include_submodules", False)) - include_nested_repos = bool(state.get("include_nested_repos", False)) - # `repowise health` persists metrics into the same rows the indexer writes, - # so it has to analyze the same file set. Without the config's exclude - # patterns it scored — and overwrote rows for — files the index had - # deliberately dropped, and on a repo excluding a manifest directory it - # could write a different `module` than the index did. - exclude_patterns: list[str] = list(load_config(repo_path).get("exclude_patterns") or []) - - traverser = FileTraverser( - repo_path, - include_submodules=include_submodules, - include_nested_repos=include_nested_repos, - extra_exclude_patterns=exclude_patterns or None, - ) - file_infos = list(traverser.traverse()) - parser = ASTParser() - graph_builder = GraphBuilder( - repo_path, - include_submodules=include_submodules, - include_nested_repos=include_nested_repos, - ) - - parsed_files = [] - for fi in file_infos: + assert target.repo_path is not None + repo_path = target.repo_path + + status.print(f"[bold]repowise health[/bold] — {repo_path}") + + if trend_view: + _render_trend(repo_path, fmt=fmt) + return + + # Analyze the same file set that was indexed: a repo initialized with + # --include-submodules persists the flag in state.json, and a flagless + # traverser here would silently score a different (smaller) tree. + state = load_state(repo_path) + include_submodules = bool(state.get("include_submodules", False)) + include_nested_repos = bool(state.get("include_nested_repos", False)) + # `repowise health` persists metrics into the same rows the indexer writes, + # so it has to analyze the same file set. Without the config's exclude + # patterns it scored — and overwrote rows for — files the index had + # deliberately dropped, and on a repo excluding a manifest directory it + # could write a different `module` than the index did. + exclude_patterns: list[str] = list(load_config(repo_path).get("exclude_patterns") or []) + + traverser = FileTraverser( + repo_path, + include_submodules=include_submodules, + include_nested_repos=include_nested_repos, + extra_exclude_patterns=exclude_patterns or None, + ) + file_infos = list(traverser.traverse()) + parser = ASTParser() + graph_builder = GraphBuilder( + repo_path, + include_submodules=include_submodules, + include_nested_repos=include_nested_repos, + ) + + parsed_files = [] + for fi in file_infos: + try: + source = PathlibPath(fi.abs_path).read_bytes() + parsed = parser.parse_file(fi, source) + graph_builder.add_file(parsed) + parsed_files.append(parsed) + except Exception: + continue + + from repowise.core.ingestion import wire_tsconfig_resolver + + wire_tsconfig_resolver( + graph_builder, + repo_path, + include_submodules=include_submodules, + include_nested_repos=include_nested_repos, + ) + graph_builder.build() + + git_meta_map: dict = {} try: - source = PathlibPath(fi.abs_path).read_bytes() - parsed = parser.parse_file(fi, source) - graph_builder.add_file(parsed) - parsed_files.append(parsed) + from repowise.core.ingestion.git_indexer import GitIndexer + + git_indexer = GitIndexer(repo_path) + _, metadata_list = run_async(git_indexer.index_repo("")) + git_meta_map = {m["file_path"]: m for m in metadata_list} except Exception: - continue - - from repowise.core.ingestion import wire_tsconfig_resolver - - wire_tsconfig_resolver( - graph_builder, - repo_path, - include_submodules=include_submodules, - include_nested_repos=include_nested_repos, - ) - graph_builder.build() - - git_meta_map: dict = {} - try: - from repowise.core.ingestion.git_indexer import GitIndexer - - git_indexer = GitIndexer(repo_path) - _, metadata_list = run_async(git_indexer.index_repo("")) - git_meta_map = {m["file_path"]: m for m in metadata_list} - except Exception: - pass - - # Coverage folds into scoring from whatever `repowise coverage add` (or - # index-time ingest) persisted - no per-run flag. Ingestion lives solely - # in the `coverage` command group. - coverage_map = _load_persisted_coverage_map(repo_path) - - analyzer = HealthAnalyzer( - graph_builder.graph(), - git_meta_map=git_meta_map, - parsed_files=parsed_files, - coverage_map=coverage_map, - duplication_cache_dir=Path(repo_path) / ".repowise", - repo_root=repo_path, - ) - # Load any .repowise/health-rules.json the user keeps in the repo. - from repowise.core.analysis.health.config import HealthConfig - - health_cfg = HealthConfig.load(repo_path) - analyzer_cfg = ( - health_cfg.to_analyzer_config([pf.file_info.path for pf in parsed_files]) - if (health_cfg.disabled_biomarkers or health_cfg.rules) - else None - ) - report = analyzer.analyze(analyzer_cfg) - - # Persist health to the repo's wiki.db so the dashboard, MCP tools, and - # `repowise status` see the same numbers as this CLI run. - # - # Skip when fmt != "table" (json/md are read by scripts and CI; side - # effects are unwelcome) or when the run is filtered to a single - # file/module (those are inspection runs that shouldn't overwrite - # repo-level state). - if fmt == "table" and not file_filter and not module_filter: - _persist_health(repo_path, report=report) - - metrics = report.metrics - if file_filter: - metrics = [m for m in metrics if m.file_path == file_filter] - if module_filter: - metrics = [m for m in metrics if m.file_path.startswith(module_filter)] - if parse_scope(scope) == "production": - metrics = [m for m in metrics if not m.is_test] - # Every figure, not just the table: the flag says production, so the - # headline, the hotspot number, the worst performer and the - # distribution all have to describe that population too. - report.kpis = compute_kpis( - metrics, {p for p, m in git_meta_map.items() if m.get("is_hotspot")} + pass + + # Coverage folds into scoring from whatever `repowise coverage add` (or + # index-time ingest) persisted - no per-run flag. Ingestion lives solely + # in the `coverage` command group. + coverage_map = _load_persisted_coverage_map(repo_path) + + analyzer = HealthAnalyzer( + graph_builder.graph(), + git_meta_map=git_meta_map, + parsed_files=parsed_files, + coverage_map=coverage_map, + duplication_cache_dir=Path(repo_path) / ".repowise", + repo_root=repo_path, + ) + # Load any .repowise/health-rules.json the user keeps in the repo. + from repowise.core.analysis.health.config import HealthConfig + + health_cfg = HealthConfig.load(repo_path) + analyzer_cfg = ( + health_cfg.to_analyzer_config([pf.file_info.path for pf in parsed_files]) + if (health_cfg.disabled_biomarkers or health_cfg.rules) + else None ) - metrics_sorted = sorted(metrics, key=lambda m: m.score) - scoped_paths = {m.file_path for m in metrics} - - findings = report.findings - if file_filter: - findings = [f for f in findings if f.file_path == file_filter] - if module_filter: - findings = [f for f in findings if f.file_path.startswith(module_filter)] - if parse_scope(scope) == "production": - findings = [f for f in findings if f.file_path in scoped_paths] - - if generate_code is not None: - suggestions = getattr(report, "refactoring_suggestions", None) or [] + report = analyzer.analyze(analyzer_cfg) + + # Persist health to the repo's wiki.db so the dashboard, MCP tools, and + # `repowise status` see the same numbers as this CLI run. + # + # Skip when fmt != "table" (json/md are read by scripts and CI; side + # effects are unwelcome) or when the run is filtered to a single + # file/module (those are inspection runs that shouldn't overwrite + # repo-level state). + if fmt == "table" and not file_filter and not module_filter: + _persist_health(repo_path, report=report) + + metrics = report.metrics if file_filter: - suggestions = [s for s in suggestions if s.file_path == file_filter] + metrics = [m for m in metrics if m.file_path == file_filter] if module_filter: - suggestions = [s for s in suggestions if s.file_path.startswith(module_filter)] - # Attaches the same batched validation block the read surfaces emit; - # code generation remains explicitly requested and never auto-applies. - _load_recommendations(repo_path, suggestions, metrics_sorted) - _generate_refactoring_code(repo_path, suggestions, generate_code, fmt=fmt) - return - - if refactoring_targets: - suggestions = getattr(report, "refactoring_suggestions", None) or [] + metrics = [m for m in metrics if m.file_path.startswith(module_filter)] + if parse_scope(scope) == "production": + metrics = [m for m in metrics if not m.is_test] + # Every figure, not just the table: the flag says production, so the + # headline, the hotspot number, the worst performer and the + # distribution all have to describe that population too. + report.kpis = compute_kpis( + metrics, {p for p, m in git_meta_map.items() if m.get("is_hotspot")} + ) + metrics_sorted = sorted(metrics, key=lambda m: m.score) + scoped_paths = {m.file_path for m in metrics} + + findings = report.findings if file_filter: - suggestions = [s for s in suggestions if s.file_path == file_filter] + findings = [f for f in findings if f.file_path == file_filter] if module_filter: - suggestions = [s for s in suggestions if s.file_path.startswith(module_filter)] - recommendations = _load_recommendations(repo_path, suggestions, metrics_sorted) - _render_refactoring_targets(metrics_sorted, findings, recommendations, fmt=fmt) - return - - if badge_view: - _render_badge(report.kpis.get("average_health")) - return - - if fmt == "json": - click.echo( - json.dumps( - { - "kpis": report.kpis, - "metrics": [ - { - "file_path": m.file_path, - "score": m.score, - "max_ccn": m.max_ccn, - "max_nesting": m.max_nesting, - "nloc": m.nloc, - "has_test_file": m.has_test_file, - "line_coverage_pct": m.line_coverage_pct, - "branch_coverage_pct": m.branch_coverage_pct, - "duplication_pct": m.duplication_pct, - } - for m in metrics_sorted - ], - "findings": [ - { - "biomarker_type": f.biomarker_type, - "severity": str(f.severity), - "file_path": f.file_path, - "function_name": f.function_name, - "health_impact": f.health_impact, - "details": f.details, - "reason": f.reason, - } - for f in findings - ], - }, - indent=2, - ) - ) - return - - if fmt == "md": - click.echo("# Code Health Report\n") - for k, v in report.kpis.items(): - click.echo(f"- **{k}**: {v}") - click.echo("\n## Findings\n") - for f in findings: + findings = [f for f in findings if f.file_path.startswith(module_filter)] + if parse_scope(scope) == "production": + findings = [f for f in findings if f.file_path in scoped_paths] + + if generate_code is not None: + suggestions = getattr(report, "refactoring_suggestions", None) or [] + if file_filter: + suggestions = [s for s in suggestions if s.file_path == file_filter] + if module_filter: + suggestions = [s for s in suggestions if s.file_path.startswith(module_filter)] + # Attaches the same batched validation block the read surfaces emit; + # code generation remains explicitly requested and never auto-applies. + _load_recommendations(repo_path, suggestions, metrics_sorted) + _generate_refactoring_code(repo_path, suggestions, generate_code, fmt=fmt) + return + + if refactoring_targets: + suggestions = getattr(report, "refactoring_suggestions", None) or [] + if file_filter: + suggestions = [s for s in suggestions if s.file_path == file_filter] + if module_filter: + suggestions = [s for s in suggestions if s.file_path.startswith(module_filter)] + recommendations = _load_recommendations(repo_path, suggestions, metrics_sorted) + _render_refactoring_targets(metrics_sorted, findings, recommendations, fmt=fmt) + return + + if badge_view: + _render_badge(report.kpis.get("average_health")) + return + + if fmt == "json": click.echo( - f"- [{f.severity}] `{f.file_path}` {f.function_name or ''} " - f"- {f.reason} (impact -{f.health_impact:.2f})" + json.dumps( + { + "kpis": report.kpis, + "metrics": [ + { + "file_path": m.file_path, + "score": m.score, + "max_ccn": m.max_ccn, + "max_nesting": m.max_nesting, + "nloc": m.nloc, + "has_test_file": m.has_test_file, + "line_coverage_pct": m.line_coverage_pct, + "branch_coverage_pct": m.branch_coverage_pct, + "duplication_pct": m.duplication_pct, + } + for m in metrics_sorted + ], + "findings": [ + { + "biomarker_type": f.biomarker_type, + "severity": str(f.severity), + "file_path": f.file_path, + "function_name": f.function_name, + "health_impact": f.health_impact, + "details": f.details, + "reason": f.reason, + } + for f in findings + ], + }, + indent=2, + ) ) - return - - # Table format - from repowise.core.analysis.health.grading import ( - BAND_LABEL, - band_for, - ) - from repowise.core.analysis.health.grading import ( - distribution as health_distribution, - ) - - kpis = report.kpis - avg = kpis.get("average_health") - band_str = "" - if isinstance(avg, (int, float)): - band = band_for(float(avg)) - band_color = {"healthy": "green", "warning": "yellow", "alert": "red"}[band] - band_str = f" [[{band_color}]{BAND_LABEL[band]}[/{band_color}]]" - console.print( - f"\nCode health: [bold]{avg if avg is not None else '?'}[/bold]/10{band_str} · " - f"Hotspot: [bold]{kpis.get('hotspot_health', '?')}[/bold]/10 · " - f"Worst: [bold]{kpis.get('worst_performer_score', '?')}[/bold]/10 " - f"({kpis.get('worst_performer_path', 'n/a')})" - ) - _render_split_line(kpis) - _render_distribution_line(health_distribution(metrics)) - - _render_defect_accuracy_line(report) - - # Performance pillar section: lead with the finding COUNT + density + - # coverage (the honest signal), not the bounded /10 average. Language comes - # from the parsed files (the in-memory metrics don't carry it). - _render_performance_section( - report, - {pf.file_info.path: pf.file_info.language for pf in parsed_files}, - ) - - table = Table(title=f"Lowest-scoring files ({min(len(metrics_sorted), 20)})") - table.add_column("File", style="cyan") - table.add_column("Score", justify="right") - table.add_column("CCN", justify="right") - table.add_column("Nest", justify="right") - table.add_column("NLOC", justify="right") - table.add_column("Test?", justify="center") - for m in metrics_sorted[:20]: - score_color = "red" if m.score < 4 else "yellow" if m.score < 7 else "green" - table.add_row( - m.file_path, - f"[{score_color}]{m.score:.1f}[/{score_color}]", - str(m.max_ccn), - str(m.max_nesting), - str(m.nloc), - "✓" if m.has_test_file else "—", + return + + if fmt == "md": + click.echo("# Code Health Report\n") + for k, v in report.kpis.items(): + click.echo(f"- **{k}**: {v}") + click.echo("\n## Findings\n") + for f in findings: + click.echo( + f"- [{f.severity}] `{f.file_path}` {f.function_name or ''} " + f"- {f.reason} (impact -{f.health_impact:.2f})" + ) + return + + # Table format + from repowise.core.analysis.health.grading import ( + BAND_LABEL, + band_for, + ) + from repowise.core.analysis.health.grading import ( + distribution as health_distribution, + ) + + kpis = report.kpis + avg = kpis.get("average_health") + band_str = "" + if isinstance(avg, (int, float)): + band = band_for(float(avg)) + band_color = {"healthy": "green", "warning": "yellow", "alert": "red"}[band] + band_str = f" [[{band_color}]{BAND_LABEL[band]}[/{band_color}]]" + console.print( + f"\nCode health: [bold]{avg if avg is not None else '?'}[/bold]/10{band_str} · " + f"Hotspot: [bold]{kpis.get('hotspot_health', '?')}[/bold]/10 · " + f"Worst: [bold]{kpis.get('worst_performer_score', '?')}[/bold]/10 " + f"({kpis.get('worst_performer_path', 'n/a')})" ) - console.print(table) - - if findings: - console.print(f"\n[bold]{len(findings)}[/bold] marker findings:") - f_table = Table() - f_table.add_column("Severity", style="magenta") - f_table.add_column("Marker", style="cyan") - f_table.add_column("File") - f_table.add_column("Function") - f_table.add_column("Impact", justify="right") - for f in findings[:30]: - f_table.add_row( - str(f.severity), - f.biomarker_type, - f.file_path, - f.function_name or "-", - f"-{f.health_impact:.2f}", + _render_split_line(kpis) + _render_distribution_line(health_distribution(metrics)) + + _render_defect_accuracy_line(report) + + # Performance pillar section: lead with the finding COUNT + density + + # coverage (the honest signal), not the bounded /10 average. Language comes + # from the parsed files (the in-memory metrics don't carry it). + _render_performance_section( + report, + {pf.file_info.path: pf.file_info.language for pf in parsed_files}, + ) + + table = Table(title=f"Lowest-scoring files ({min(len(metrics_sorted), 20)})") + table.add_column("File", style="cyan") + table.add_column("Score", justify="right") + table.add_column("CCN", justify="right") + table.add_column("Nest", justify="right") + table.add_column("NLOC", justify="right") + table.add_column("Test?", justify="center") + for m in metrics_sorted[:20]: + score_color = "red" if m.score < 4 else "yellow" if m.score < 7 else "green" + table.add_row( + m.file_path, + f"[{score_color}]{m.score:.1f}[/{score_color}]", + str(m.max_ccn), + str(m.max_nesting), + str(m.nloc), + "✓" if m.has_test_file else "—", ) - console.print(f_table) + console.print(table) + + if findings: + console.print(f"\n[bold]{len(findings)}[/bold] marker findings:") + f_table = Table() + f_table.add_column("Severity", style="magenta") + f_table.add_column("Marker", style="cyan") + f_table.add_column("File") + f_table.add_column("Function") + f_table.add_column("Impact", justify="right") + for f in findings[:30]: + f_table.add_row( + str(f.severity), + f.biomarker_type, + f.file_path, + f.function_name or "-", + f"-{f.health_impact:.2f}", + ) + console.print(f_table) diff --git a/packages/cli/src/repowise/cli/commands/impacted_tests_cmd.py b/packages/cli/src/repowise/cli/commands/impacted_tests_cmd.py index 65945cfbe..6e3b54130 100644 --- a/packages/cli/src/repowise/cli/commands/impacted_tests_cmd.py +++ b/packages/cli/src/repowise/cli/commands/impacted_tests_cmd.py @@ -22,6 +22,7 @@ from __future__ import annotations +import contextlib import json import click @@ -73,21 +74,20 @@ def impacted_tests_command(revspec: str | None, repo: str | None, staged: bool, raise click.ClickException("Give a revision range or --staged, not both.") # json/list go to downstream tools; keep stdout clean of log noise. - if fmt != "table": - silence_logs_for_machine_output() + silencer = silence_logs_for_machine_output() if fmt != "table" else contextlib.nullcontext() + with silencer: + repo_path = _resolve_repo_path(repo) - repo_path = _resolve_repo_path(repo) + from repowise.core.analysis.changed_lines import changed_lines - from repowise.core.analysis.changed_lines import changed_lines + try: + changed, label = changed_lines(str(repo_path), revspec, staged=staged) + except ValueError as exc: + raise click.ClickException(str(exc)) from exc - try: - changed, label = changed_lines(str(repo_path), revspec, staged=staged) - except ValueError as exc: - raise click.ClickException(str(exc)) from exc - - result = run_async(_collect(repo_path, changed)) - result["diff"] = label - _render(result, fmt) + result = run_async(_collect(repo_path, changed)) + result["diff"] = label + _render(result, fmt) async def _collect(repo_path, changed: dict[str, set[int]]) -> dict: diff --git a/packages/cli/src/repowise/cli/commands/security_cmd.py b/packages/cli/src/repowise/cli/commands/security_cmd.py index 83a919584..0a3f9f4a3 100644 --- a/packages/cli/src/repowise/cli/commands/security_cmd.py +++ b/packages/cli/src/repowise/cli/commands/security_cmd.py @@ -10,6 +10,8 @@ from __future__ import annotations +import contextlib + import click from repowise.cli.helpers import ( @@ -104,85 +106,86 @@ def security_scan( emit_json({"scanned": False, "reason": "history-mode-not-requested"}) return - if output_format == "json": - silence_logs_for_machine_output() - - from pathlib import Path - - from repowise.core.analysis.history_scan import HistorySecurityScanner - from repowise.core.persistence import ( - create_engine, - create_session_factory, - get_session, - ) - from repowise.core.persistence.crud import ( - get_repository_by_path, - upsert_repository, + silencer = ( + silence_logs_for_machine_output() if output_format == "json" else contextlib.nullcontext() ) + with silencer: + from pathlib import Path + + from repowise.core.analysis.history_scan import HistorySecurityScanner + from repowise.core.persistence import ( + create_engine, + create_session_factory, + get_session, + ) + from repowise.core.persistence.crud import ( + get_repository_by_path, + upsert_repository, + ) - target = resolve_command_target(path=repo) - target.notice(notice_console(output_format), command="security scan --history") - - if target.is_workspace: - primary = target.primary_path() - if primary is None: - raise click.ClickException("Workspace has no primary repo configured.") - repo_path = primary - else: - assert target.repo_path is not None - repo_path = target.repo_path - - ensure_repowise_dir(repo_path) - - async def _do() -> dict: - engine = create_engine(get_db_url_for_repo(repo_path)) - sf = create_session_factory(engine) - async with get_session(sf) as session: - row = await get_repository_by_path(session, str(repo_path)) - if row is None: - row = await upsert_repository( - session, - name=repo_path.name, - local_path=str(repo_path), + target = resolve_command_target(path=repo) + target.notice(notice_console(output_format), command="security scan --history") + + if target.is_workspace: + primary = target.primary_path() + if primary is None: + raise click.ClickException("Workspace has no primary repo configured.") + repo_path = primary + else: + assert target.repo_path is not None + repo_path = target.repo_path + + ensure_repowise_dir(repo_path) + + async def _do() -> dict: + engine = create_engine(get_db_url_for_repo(repo_path)) + sf = create_session_factory(engine) + async with get_session(sf) as session: + row = await get_repository_by_path(session, str(repo_path)) + if row is None: + row = await upsert_repository( + session, + name=repo_path.name, + local_path=str(repo_path), + ) + scanner = HistorySecurityScanner(session, row.id) + summary = await scanner.scan_history( + Path(repo_path), + since=since, + to=to, + secrets_only=not all_patterns, + progress=lambda msg: ( + console.print(f"[dim]{msg}[/dim]") if output_format != "json" else None + ), ) - scanner = HistorySecurityScanner(session, row.id) - summary = await scanner.scan_history( - Path(repo_path), - since=since, - to=to, - secrets_only=not all_patterns, - progress=lambda msg: ( - console.print(f"[dim]{msg}[/dim]") if output_format != "json" else None - ), - ) - await session.commit() - return { - "commits_scanned": summary.commits_scanned, - "blobs_scanned": summary.blobs_scanned, - "files_scanned": summary.files_scanned, - "findings_inserted": summary.findings_inserted, - "by_severity": summary.by_severity, - "by_kind": summary.by_kind, - } - - result = run_async(_do()) - - if output_format == "json": - emit_json(result) - return + await session.commit() + return { + "commits_scanned": summary.commits_scanned, + "blobs_scanned": summary.blobs_scanned, + "files_scanned": summary.files_scanned, + "findings_inserted": summary.findings_inserted, + "by_severity": summary.by_severity, + "by_kind": summary.by_kind, + } + + result = run_async(_do()) - console.print(f"[bold]repowise security scan --history[/bold] — {repo_path}") - console.print(f" Commits scanned: {result['commits_scanned']}") - console.print(f" Blobs scanned: {result['blobs_scanned']}") - console.print(f" Files scanned: {result['files_scanned']}") - console.print(f" Findings stored: {result['findings_inserted']}") - if result["by_severity"]: - sev = ", ".join(f"{k}={v}" for k, v in sorted(result["by_severity"].items())) - console.print(f" By severity: {sev}") - if result["by_kind"]: - kinds = ", ".join(f"{k}={v}" for k, v in sorted(result["by_kind"].items())) - console.print(f" By kind: {kinds}") - console.print( - "\nFindings are written to the security_findings table and show up in " - "`repowise server`'s security API and UI. Re-running is idempotent." - ) + if output_format == "json": + emit_json(result) + return + + console.print(f"[bold]repowise security scan --history[/bold] — {repo_path}") + console.print(f" Commits scanned: {result['commits_scanned']}") + console.print(f" Blobs scanned: {result['blobs_scanned']}") + console.print(f" Files scanned: {result['files_scanned']}") + console.print(f" Findings stored: {result['findings_inserted']}") + if result["by_severity"]: + sev = ", ".join(f"{k}={v}" for k, v in sorted(result["by_severity"].items())) + console.print(f" By severity: {sev}") + if result["by_kind"]: + kinds = ", ".join(f"{k}={v}" for k, v in sorted(result["by_kind"].items())) + console.print(f" By kind: {kinds}") + console.print( + "\nFindings are written to the security_findings table and show up in " + "`repowise server`'s security API and UI. Re-running is idempotent." + ) diff --git a/packages/cli/src/repowise/cli/commands/update_cmd/command.py b/packages/cli/src/repowise/cli/commands/update_cmd/command.py index 56577cf53..cbfff5f3a 100644 --- a/packages/cli/src/repowise/cli/commands/update_cmd/command.py +++ b/packages/cli/src/repowise/cli/commands/update_cmd/command.py @@ -35,7 +35,7 @@ rotate_update_log_if_needed, run_async, save_state, - silence_logs_for_machine_output, + silence_logs_for_machine_output_until_close, try_acquire_update_lock, write_update_pending, ) @@ -606,7 +606,7 @@ def run_update( # into whatever runs next in the same process. emitter: JsonProgressEmitter | None = None if progress == "json": - silence_logs_for_machine_output() + silence_logs_for_machine_output_until_close() console.file = sys.stderr # Restore to None (rather than a captured file object) so `console` # goes back to resolving sys.stdout dynamically on each print, its diff --git a/packages/cli/src/repowise/cli/helpers.py b/packages/cli/src/repowise/cli/helpers.py index 5aade5f38..de2e4fa6e 100644 --- a/packages/cli/src/repowise/cli/helpers.py +++ b/packages/cli/src/repowise/cli/helpers.py @@ -77,36 +77,86 @@ def _clean_flag(value: str | None) -> bool: # Logging / structlog helpers # --------------------------------------------------------------------------- +MACHINE_OUTPUT_LOGGER_NAMES = ("httpx", "httpcore", "repowise.core", "repowise.server") -def silence_logs_for_machine_output() -> None: - """Suppress info/debug log output when stdout is machine-readable (JSON/md). +@contextlib.contextmanager +def silence_logs_for_machine_output(): + """Suppress info/debug log output while stdout is machine-readable (JSON/md). Structlog and stdlib loggers write to stdout by default. When a command emits JSON or Markdown, those lines corrupt the output for downstream consumers (e.g. ``repowise health --format json | jq .kpis``). - Call this at the top of any command that supports ``--format json`` or - ``--format md`` before the ingestion pipeline starts. + Context manager, not a bare call: logger levels and the structlog + wrapper class are process-global with no other owner, so a caller that + forgot to restore them would permanently silence its own process — the + case that mattered in practice was a test session, where the mutation + outlived the test that made it and broke unrelated caplog assertions + later in the same run (see #1976). + + Use as: + + with silence_logs_for_machine_output(): + emit_json_or_markdown(...) """ import logging - logging.getLogger("httpx").setLevel(logging.ERROR) - logging.getLogger("httpcore").setLevel(logging.ERROR) - for _name in ("repowise.core", "repowise.server"): - logging.getLogger(_name).setLevel(logging.ERROR) + loggers = [logging.getLogger(name) for name in MACHINE_OUTPUT_LOGGER_NAMES] + previous_levels = [logger.level for logger in loggers] + + previous_structlog_config: dict[str, Any] | None = None try: import structlog - # cache_logger_on_first_use=False is required: module-level - # ``structlog.get_logger`` calls snapshot the logger before configure() - # runs and would bypass this filter without it. - structlog.configure( - wrapper_class=structlog.make_filtering_bound_logger(logging.ERROR), - cache_logger_on_first_use=False, - ) + previous_structlog_config = dict(structlog.get_config()) except ImportError: pass + try: + for logger in loggers: + logger.setLevel(logging.ERROR) + if previous_structlog_config is not None: + import structlog + + # cache_logger_on_first_use=False is required: module-level + # ``structlog.get_logger`` calls snapshot the logger before + # configure() runs and would bypass this filter without it. + structlog.configure( + wrapper_class=structlog.make_filtering_bound_logger(logging.ERROR), + cache_logger_on_first_use=False, + ) + yield + finally: + for logger, level in zip(loggers, previous_levels, strict=True): + logger.setLevel(level) + if previous_structlog_config is not None: + import structlog + + structlog.configure(**previous_structlog_config) + +def silence_logs_for_machine_output_until_close() -> None: + """Enter ``silence_logs_for_machine_output`` and restore it when the + current click command finishes. + + ``silence_logs_for_machine_output`` is a context manager because it must + always restore what it mutates — but not every call site has a single + lexical block to wrap it around. An option callback (see the ``--format`` + and ``--json`` callbacks in ``output.py``) returns before the command body + even starts running, so a ``with`` block there would restore the levels + before the command does any work. Re-indenting an entire command + function's body under one ``with`` is also a large, easy-to-get-wrong + diff at call sites deep inside long functions. + + Solved the same way ``update_cmd`` already solves it for restoring + ``console.file``: register the undo against click's context instead of a + lexical scope, so it fires when the command finishes regardless of how + much code runs in between or where the call sits. + """ + ctx = click.get_current_context() + cm = silence_logs_for_machine_output() + cm.__enter__() + ctx.call_on_close(lambda: cm.__exit__(None, None, None)) + # --------------------------------------------------------------------------- # Async bridge diff --git a/packages/cli/src/repowise/cli/output.py b/packages/cli/src/repowise/cli/output.py index 3d0e628c7..be357e199 100644 --- a/packages/cli/src/repowise/cli/output.py +++ b/packages/cli/src/repowise/cli/output.py @@ -85,9 +85,9 @@ def _silence_when_machine_readable(ctx: Any, param: Any, value: str) -> str: a module that logs on import. """ if value != "table": - from repowise.cli.helpers import silence_logs_for_machine_output + from repowise.cli.helpers import silence_logs_for_machine_output_until_close - silence_logs_for_machine_output() + silence_logs_for_machine_output_until_close() return value @@ -121,9 +121,9 @@ def _silence_when_alias_selects_json(ctx: Any, param: Any, value: bool) -> bool: the one a legacy caller asked for. """ if value: - from repowise.cli.helpers import silence_logs_for_machine_output + from repowise.cli.helpers import silence_logs_for_machine_output_until_close - silence_logs_for_machine_output() + silence_logs_for_machine_output_until_close() return value diff --git a/tests/conftest.py b/tests/conftest.py index 43e023a30..1bc6518db 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -135,6 +135,40 @@ def _isolate_structlog_config(): finally: structlog.configure(**saved) +@pytest.fixture(autouse=True) +def _isolate_silenced_logger_levels(): + """Restore stdlib logger levels mutated by silence_logs_for_machine_output. + + That helper also calls ``logging.getLogger(name).setLevel(logging.ERROR)`` + on ``httpx``, ``httpcore``, ``repowise.core``, and ``repowise.server`` — + process-global state that ``_isolate_structlog_config`` (above) does not + touch, since it only snapshots structlog's own configuration. + + Without this, a test that exercises any ``--format json``/``--format md`` + command leaves those loggers raised to ERROR for the rest of the session. + A later test asserting a ``caplog`` record from a module under + ``repowise.core`` or ``repowise.server`` then reads an empty list: the + module's own logger has no level set, so its *effective* level is + inherited from the ancestor now pinned at ERROR, and + ``logger.warning(...)`` is dropped before any handler — including + ``caplog``'s — ever runs. Same failure shape as the structlog case + above: passes when the file is run alone, fails only in a full run, + depending on whether a machine-output command executed first (#1976). + + Snapshot and restore rather than reset to ``NOTSET``: a test that sets a + level on purpose keeps working, and the next test still starts clean. + """ + import logging + + from repowise.cli.helpers import MACHINE_OUTPUT_LOGGER_NAMES + + loggers = [logging.getLogger(name) for name in MACHINE_OUTPUT_LOGGER_NAMES] + previous_levels = [logger.level for logger in loggers] + try: + yield + finally: + for logger, level in zip(loggers, previous_levels, strict=True): + logger.setLevel(level) @pytest.fixture(scope="session") def repo_root() -> Path: diff --git a/tests/unit/cli/test_tool_adapter_commands.py b/tests/unit/cli/test_tool_adapter_commands.py index 83f979c2f..abad20a82 100644 --- a/tests/unit/cli/test_tool_adapter_commands.py +++ b/tests/unit/cli/test_tool_adapter_commands.py @@ -18,6 +18,7 @@ from __future__ import annotations +import contextlib import json import pytest @@ -744,9 +745,15 @@ def test_logs_are_silenced_at_every_format_not_only_the_machine_ones( answer — and inside anything reading it through ``repowise distill``. """ silenced: list = [] + + @contextlib.contextmanager + def _fake_silence(): + silenced.append(True) + yield + monkeypatch.setattr( "repowise.cli.helpers.silence_logs_for_machine_output", - lambda: silenced.append(True), + _fake_silence, ) monkeypatch.setattr("repowise.cli.tool_bridge.call_tool", lambda p, f, t: {"_meta": {}}) result = CliRunner(mix_stderr=False).invoke( diff --git a/tests/unit/cli/test_update_full_dry_run.py b/tests/unit/cli/test_update_full_dry_run.py index dd019e712..60a6713b5 100644 --- a/tests/unit/cli/test_update_full_dry_run.py +++ b/tests/unit/cli/test_update_full_dry_run.py @@ -127,7 +127,7 @@ def test_full_dry_run_emits_dry_run_machine_outcome( repo = tmp_path / "repo" _prepare_repo(repo) _patch_boundary(monkeypatch, repo) - monkeypatch.setattr(update_cmd, "silence_logs_for_machine_output", lambda: None) + monkeypatch.setattr(update_cmd, "silence_logs_for_machine_output_until_close", lambda: None) events: list[tuple[str, dict[str, Any]]] = [] diff --git a/tests/unit/test_structlog_isolation.py b/tests/unit/test_structlog_isolation.py index 492fa634d..4d84bb17f 100644 --- a/tests/unit/test_structlog_isolation.py +++ b/tests/unit/test_structlog_isolation.py @@ -1,14 +1,4 @@ -"""The global structlog config does not leak from one test into the next. - -``silence_logs_for_machine_output`` and ``configure_cli_logging`` install a -filtering bound logger at ERROR for the whole process. Without the autouse -fixture in ``tests/conftest.py`` that setting outlives the test that made it, -and every later ``capture_logs`` assertion on an ``info`` or ``warning`` reads -an empty list. Such a test passes in isolation and fails in a full run. - -The two tests below must stay in this order: the first one does the damage, -the second one proves it was undone. -""" +"""Machine-output log silencing is limited to its command's lifetime.""" from __future__ import annotations @@ -18,14 +8,15 @@ from repowise.cli.helpers import silence_logs_for_machine_output -def test_a_cli_command_silences_logs_process_wide() -> None: - silence_logs_for_machine_output() - with capture_logs() as logs: - structlog.get_logger(__name__).warning("silenced_here") - assert logs == [] +def test_machine_output_silences_logs_only_within_its_scope() -> None: + structlog.configure(cache_logger_on_first_use=True) + with silence_logs_for_machine_output(): + with capture_logs() as logs: + structlog.get_logger(__name__).warning("silenced_here") + assert logs == [] -def test_b_the_next_test_can_still_capture_a_warning() -> None: + assert structlog.get_config()["cache_logger_on_first_use"] is True with capture_logs() as logs: structlog.get_logger(__name__).warning("visible_again") assert [entry["event"] for entry in logs] == ["visible_again"]