From 5b0624134731d0e4979abb3f491e8aea4c3466ab Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 22:45:51 +0000 Subject: [PATCH 1/3] cli: align with Typer's bundled agent skill Reviewed src/icebergsca/cli/main.py against the typer skill shipped at typer/.agents/skills/typer/SKILL.md. The explicit Typer app, Annotated declarations and absence of Click-era settings already matched; these are the gaps. - --scope and --ecosystem took a single comma-separated string, so their help showed and listed no choices, unlike --format and unlike --exclude in the same command. They now take a repeatable list and advertise their members in the metavar. Comma-separated values still parse, so every documented invocation is unchanged. - The sbom docstring used RST double backticks, which the default Rich markup mode renders literally. Uses Rich markup instead. - rich_markup_mode is now stated rather than inferred, so help output does not change shape with what else is installed. - sbom's -v/-q had no help text; the cache sub-app had no no_args_is_help, so a bare `icebergsca cache` errored where a bare `icebergsca` helps. - Floor typer at 0.26, the release that vendored Click. Below it, an unrelated Click upgrade governs parsing and exit codes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CJSEXcpbfSLbYqHc2t2WY2 --- pyproject.toml | 4 +- .../.agents/skills/icebergsca/SKILL.md | 3 + src/icebergsca/cli/main.py | 67 +++++++++++++------ tests/test_cli.py | 23 +++++++ uv.lock | 2 +- website/docs/cli.md | 4 +- 6 files changed, 80 insertions(+), 23 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index fe1003f..6eec289 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,7 +38,9 @@ classifiers = [ "Topic :: Software Development :: Quality Assurance", ] dependencies = [ - "typer>=0.15", + # 0.26 is where Typer vendored Click. Below that floor an unrelated Click + # upgrade in the same environment can change how this CLI parses and exits. + "typer>=0.26", "rich>=13.0", "httpx>=0.27", "packaging>=24.0", diff --git a/src/icebergsca/.agents/skills/icebergsca/SKILL.md b/src/icebergsca/.agents/skills/icebergsca/SKILL.md index 9cf3c84..a2edcb4 100644 --- a/src/icebergsca/.agents/skills/icebergsca/SKILL.md +++ b/src/icebergsca/.agents/skills/icebergsca/SKILL.md @@ -170,6 +170,9 @@ icebergsca scan . --include-dev # everything icebergsca scan . --scope runtime # only what ships ``` +`--scope` and `--ecosystem` are repeatable as well as comma-separated: +`--scope runtime --scope build` and `--scope runtime,build` are the same request. + ## Narrowing a scan ```bash diff --git a/src/icebergsca/cli/main.py b/src/icebergsca/cli/main.py index e29e5e5..2762409 100644 --- a/src/icebergsca/cli/main.py +++ b/src/icebergsca/cli/main.py @@ -35,8 +35,9 @@ class ExitCode(IntEnum): vulnerabilities has done its job and exits :attr:`OK`; severity gating arrives later as an explicit ``--fail-on`` flag. - :attr:`USAGE` is 2 rather than 1 because Click reserves that code for usage - errors, and remapping it would mean overriding framework internals for nothing. + :attr:`USAGE` is 2 rather than 1 because Typer's vendored Click reserves that + code for usage errors, and remapping it would mean overriding framework + internals for nothing. """ OK = 0 @@ -51,6 +52,9 @@ class ExitCode(IntEnum): help="Software composition analysis: find dependencies, check them against OSV.", no_args_is_help=True, add_completion=False, + # Stated rather than left to the default so that help output does not change + # shape depending on what else happens to be installed. + rich_markup_mode="rich", ) logger = logging.getLogger("icebergsca") @@ -77,22 +81,34 @@ def _configure_logging(verbosity: int, quiet: bool) -> None: ) -def _parse_csv_option( - value: str | None, name: str, choices: type[EnumT] +def _choices_metavar(choices: type[EnumT]) -> str: + """Render an enum the way Typer renders one it converts itself.""" + return f"<{'|'.join(member.value for member in choices)}>" + + +def _parse_enum_option( + values: list[str] | None, name: str, choices: type[EnumT] ) -> list[EnumT] | None: - """Split a comma-separated flag into enum members, reporting bad values clearly.""" - if not value: + """Resolve a repeatable, comma-separated flag into enum members. + + Both ``--scope runtime,dev`` and ``--scope runtime --scope dev`` are accepted. + Typer would convert a ``list[Scope]`` on its own, but not split on commas, and + the comma form is the one the documentation has always shown. + """ + if not values: return None parsed: list[EnumT] = [] valid = {member.value for member in choices} - for item in (part.strip() for part in value.split(",")): + for item in (part.strip() for value in values for part in value.split(",")): if not item: continue if item not in valid: raise typer.BadParameter( f"unknown {name} '{item}' — choose from: {', '.join(sorted(valid))}" ) - parsed.append(choices(item)) + member = choices(item) + if member not in parsed: + parsed.append(member) return parsed or None @@ -142,15 +158,21 @@ def scan_command( ), ] = False, scopes: Annotated[ - str | None, + list[str] | None, typer.Option( "--scope", - help="Comma-separated scopes to include, overriding --include-dev.", + metavar=_choices_metavar(Scope), + help="Scope to include, overriding --include-dev. " + "Repeatable, or comma-separated.", ), ] = None, ecosystems: Annotated[ - str | None, - typer.Option("--ecosystem", help="Comma-separated ecosystems to restrict to."), + list[str] | None, + typer.Option( + "--ecosystem", + metavar=_choices_metavar(EcosystemId), + help="Ecosystem to restrict to. Repeatable, or comma-separated.", + ), ] = None, exclude: Annotated[ list[str] | None, @@ -202,8 +224,8 @@ def scan_command( """Scan a project for dependencies and known vulnerabilities.""" _configure_logging(verbose, quiet) - selected_scopes = _parse_csv_option(scopes, "scope", Scope) - selected_ecosystems = _parse_csv_option(ecosystems, "ecosystem", EcosystemId) + selected_scopes = _parse_enum_option(scopes, "scope", Scope) + selected_ecosystems = _parse_enum_option(ecosystems, "ecosystem", EcosystemId) if selected_scopes is not None: scope_set = frozenset(selected_scopes) @@ -297,13 +319,17 @@ def sbom_command( "and no network calls to OSV.", ), ] = False, - verbose: Annotated[int, typer.Option("-v", "--verbose", count=True)] = 0, - quiet: Annotated[bool, typer.Option("-q", "--quiet")] = False, + verbose: Annotated[ + int, typer.Option("-v", "--verbose", count=True, help="Increase log verbosity.") + ] = 0, + quiet: Annotated[ + bool, typer.Option("-q", "--quiet", help="Only log errors.") + ] = False, ) -> None: """Emit a CycloneDX 1.6 SBOM. - Equivalent to ``scan --format cyclonedx``, except that it defaults to components - only — an SBOM is often wanted for inventory rather than for findings. + Equivalent to [bold]scan --format cyclonedx[/bold], except that it defaults to + components only — an SBOM is often wanted for inventory rather than for findings. """ _configure_logging(verbose, quiet) @@ -340,7 +366,10 @@ def sbom_command( # cache # --------------------------------------------------------------------------- -cache_app = typer.Typer(help="Inspect and manage the on-disk OSV cache.") +cache_app = typer.Typer( + help="Inspect and manage the on-disk OSV cache.", + no_args_is_help=True, +) app.add_typer(cache_app, name="cache") diff --git a/tests/test_cli.py b/tests/test_cli.py index 9aeba30..fd9df32 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -99,6 +99,29 @@ def test_ecosystem_filter(tmp_path: Path) -> None: assert {m["ecosystem"] for m in document["manifests"]} == {"PyPI"} +def test_scope_flag_is_repeatable_as_well_as_comma_separated(tmp_path: Path) -> None: + (tmp_path / "pyproject.toml").write_text( + '[project]\nname = "x"\nversion = "1"\ndependencies = ["httpx>=0.27"]\n\n' + '[project.optional-dependencies]\ndev = ["ruff>=0.9"]\n' + ) + argv = ["scan", str(tmp_path), "--format", "json"] + repeated = runner.invoke(app, [*argv, "--scope", "runtime", "--scope", "dev"]) + comma = runner.invoke(app, [*argv, "--scope", "runtime,dev"]) + for result in (repeated, comma): + names = { + e["package"]["name"] for e in json.loads(result.stdout)["dependencies"] + } + assert names == {"httpx", "ruff"} + + +def test_enum_choices_are_listed_in_help() -> None: + """A bare ```` metavar tells a reader nothing about what is accepted.""" + # Wide enough that Rich does not wrap the metavar column mid-value. + result = runner.invoke(app, ["scan", "--help"], env={"COLUMNS": "200"}) + assert "" in result.stdout + assert "" in result.stdout + + def test_exclude_glob(tmp_path: Path) -> None: project(tmp_path) (tmp_path / "sub").mkdir() diff --git a/uv.lock b/uv.lock index 15f8575..e7d3553 100644 --- a/uv.lock +++ b/uv.lock @@ -285,7 +285,7 @@ requires-dist = [ { name = "pyyaml", specifier = ">=6.0" }, { name = "rich", specifier = ">=13.0" }, { name = "ruff", marker = "extra == 'dev'", specifier = ">=0.9" }, - { name = "typer", specifier = ">=0.15" }, + { name = "typer", specifier = ">=0.26" }, { name = "types-defusedxml", marker = "extra == 'dev'", specifier = ">=0.7" }, { name = "types-jsonschema", marker = "extra == 'dev'", specifier = ">=4.0" }, { name = "types-pyyaml", marker = "extra == 'dev'", specifier = ">=6.0" }, diff --git a/website/docs/cli.md b/website/docs/cli.md index d955a90..8604eac 100644 --- a/website/docs/cli.md +++ b/website/docs/cli.md @@ -25,12 +25,12 @@ icebergsca scan | Flag | Effect | |---|---| -| `--ecosystem ` | Comma-separated ecosystems to restrict to: `pypi`, `npm`, `maven`, `go`, `cargo`, `nuget`, `rubygems` | +| `--ecosystem ` | Ecosystems to restrict to: `pypi`, `npm`, `maven`, `go`, `cargo`, `nuget`, `rubygems`. Repeatable, or comma-separated | | `--exclude ` | Glob to skip. Repeatable | | `--max-depth ` | Maximum directory depth to walk | | `--follow-symlinks` | Follow symlinked directories | | `--include-dev` | Include dev and test dependencies, which are excluded by default | -| `--scope ` | Comma-separated scopes to include, overriding `--include-dev`: `runtime`, `dev`, `test`, `build`, `optional` | +| `--scope ` | Scopes to include, overriding `--include-dev`: `runtime`, `dev`, `test`, `build`, `optional`. Repeatable, or comma-separated | ```bash icebergsca scan . --ecosystem pypi,npm --exclude 'tests/fixtures/**' From 16933f69ebaaf0183cc2c2d5da8f747446613dbb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 22:50:07 +0000 Subject: [PATCH 2/3] cli: let Typer convert --scope and --ecosystem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No deployments to keep compatible, so the comma-separated form goes and the options become plain list[Scope] and list[EcosystemId]. Typer derives the choices metavar, validates the values and rejects a bad one as a usage error by itself, which is exactly what _parse_enum_option and _choices_metavar were reimplementing — both are now gone, along with the EnumT TypeVar. Help output and exit codes are unchanged. Docs and the bundled skill now show the repeated form. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CJSEXcpbfSLbYqHc2t2WY2 --- README.md | 2 +- .../.agents/skills/icebergsca/SKILL.md | 10 ++-- src/icebergsca/cli/main.py | 58 +++---------------- tests/test_cli.py | 17 +++--- website/docs/cli.md | 6 +- website/docs/ecosystems.md | 4 +- 6 files changed, 27 insertions(+), 70 deletions(-) diff --git a/README.md b/README.md index 9b90c90..0220cec 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ the same command against a newer tag with `--force`. icebergsca scan ./myproject icebergsca scan ./myproject --format json --output report.json icebergsca scan ./myproject --include-dev -icebergsca scan ./myproject --ecosystem pypi,npm --exclude 'fixtures/**' +icebergsca scan ./myproject --ecosystem pypi --ecosystem npm --exclude 'fixtures/**' icebergsca scan ./requirements.txt # a single file works too icebergsca sbom ./myproject # CycloneDX 1.6, components only diff --git a/src/icebergsca/.agents/skills/icebergsca/SKILL.md b/src/icebergsca/.agents/skills/icebergsca/SKILL.md index a2edcb4..9775f9e 100644 --- a/src/icebergsca/.agents/skills/icebergsca/SKILL.md +++ b/src/icebergsca/.agents/skills/icebergsca/SKILL.md @@ -170,15 +170,15 @@ icebergsca scan . --include-dev # everything icebergsca scan . --scope runtime # only what ships ``` -`--scope` and `--ecosystem` are repeatable as well as comma-separated: -`--scope runtime --scope build` and `--scope runtime,build` are the same request. +`--scope` and `--ecosystem` take one value each and are repeated to widen the +selection: `--scope runtime --scope build`. ## Narrowing a scan ```bash -icebergsca scan . --ecosystem pypi,npm # restrict ecosystems -icebergsca scan . --exclude 'tests/fixtures/**' # skip paths -icebergsca scan ./requirements.txt # a single file +icebergsca scan . --ecosystem pypi --ecosystem npm # restrict ecosystems +icebergsca scan . --exclude 'tests/fixtures/**' # skip paths +icebergsca scan ./requirements.txt # a single file ``` Deliberately vulnerable test fixtures are a common source of noise. Exclude them rather than diff --git a/src/icebergsca/cli/main.py b/src/icebergsca/cli/main.py index 2762409..2634160 100644 --- a/src/icebergsca/cli/main.py +++ b/src/icebergsca/cli/main.py @@ -15,7 +15,7 @@ from contextlib import contextmanager from enum import IntEnum from pathlib import Path -from typing import Annotated, NoReturn, TypeVar +from typing import Annotated, NoReturn import typer @@ -59,8 +59,6 @@ class ExitCode(IntEnum): logger = logging.getLogger("icebergsca") -EnumT = TypeVar("EnumT", Scope, EcosystemId) - def _fail(message: str) -> NoReturn: typer.secho(f"error: {message}", fg=typer.colors.RED, err=True) @@ -81,37 +79,6 @@ def _configure_logging(verbosity: int, quiet: bool) -> None: ) -def _choices_metavar(choices: type[EnumT]) -> str: - """Render an enum the way Typer renders one it converts itself.""" - return f"<{'|'.join(member.value for member in choices)}>" - - -def _parse_enum_option( - values: list[str] | None, name: str, choices: type[EnumT] -) -> list[EnumT] | None: - """Resolve a repeatable, comma-separated flag into enum members. - - Both ``--scope runtime,dev`` and ``--scope runtime --scope dev`` are accepted. - Typer would convert a ``list[Scope]`` on its own, but not split on commas, and - the comma form is the one the documentation has always shown. - """ - if not values: - return None - parsed: list[EnumT] = [] - valid = {member.value for member in choices} - for item in (part.strip() for value in values for part in value.split(",")): - if not item: - continue - if item not in valid: - raise typer.BadParameter( - f"unknown {name} '{item}' — choose from: {', '.join(sorted(valid))}" - ) - member = choices(item) - if member not in parsed: - parsed.append(member) - return parsed or None - - def _version_callback(value: bool) -> None: if value: typer.echo(f"icebergsca {__version__}") @@ -158,21 +125,15 @@ def scan_command( ), ] = False, scopes: Annotated[ - list[str] | None, + list[Scope] | None, typer.Option( "--scope", - metavar=_choices_metavar(Scope), - help="Scope to include, overriding --include-dev. " - "Repeatable, or comma-separated.", + help="Scope to include, overriding --include-dev. Repeatable.", ), ] = None, ecosystems: Annotated[ - list[str] | None, - typer.Option( - "--ecosystem", - metavar=_choices_metavar(EcosystemId), - help="Ecosystem to restrict to. Repeatable, or comma-separated.", - ), + list[EcosystemId] | None, + typer.Option("--ecosystem", help="Ecosystem to restrict to. Repeatable."), ] = None, exclude: Annotated[ list[str] | None, @@ -224,11 +185,8 @@ def scan_command( """Scan a project for dependencies and known vulnerabilities.""" _configure_logging(verbose, quiet) - selected_scopes = _parse_enum_option(scopes, "scope", Scope) - selected_ecosystems = _parse_enum_option(ecosystems, "ecosystem", EcosystemId) - - if selected_scopes is not None: - scope_set = frozenset(selected_scopes) + if scopes: + scope_set = frozenset(scopes) elif include_dev: scope_set = frozenset(Scope) else: @@ -242,7 +200,7 @@ def scan_command( exclude=tuple(exclude or ()), max_depth=max_depth, follow_symlinks=follow_symlinks, - ecosystems=frozenset(selected_ecosystems) if selected_ecosystems else None, + ecosystems=frozenset(ecosystems) if ecosystems else None, ), scopes=scope_set, offline=offline, diff --git a/tests/test_cli.py b/tests/test_cli.py index fd9df32..9f53f7a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -99,19 +99,18 @@ def test_ecosystem_filter(tmp_path: Path) -> None: assert {m["ecosystem"] for m in document["manifests"]} == {"PyPI"} -def test_scope_flag_is_repeatable_as_well_as_comma_separated(tmp_path: Path) -> None: +def test_scope_flag_is_repeatable(tmp_path: Path) -> None: (tmp_path / "pyproject.toml").write_text( '[project]\nname = "x"\nversion = "1"\ndependencies = ["httpx>=0.27"]\n\n' '[project.optional-dependencies]\ndev = ["ruff>=0.9"]\n' ) - argv = ["scan", str(tmp_path), "--format", "json"] - repeated = runner.invoke(app, [*argv, "--scope", "runtime", "--scope", "dev"]) - comma = runner.invoke(app, [*argv, "--scope", "runtime,dev"]) - for result in (repeated, comma): - names = { - e["package"]["name"] for e in json.loads(result.stdout)["dependencies"] - } - assert names == {"httpx", "ruff"} + result = runner.invoke( + app, + ["scan", str(tmp_path), "--format", "json", "--scope", "runtime"] + + ["--scope", "dev"], + ) + names = {e["package"]["name"] for e in json.loads(result.stdout)["dependencies"]} + assert names == {"httpx", "ruff"} def test_enum_choices_are_listed_in_help() -> None: diff --git a/website/docs/cli.md b/website/docs/cli.md index 8604eac..78e6073 100644 --- a/website/docs/cli.md +++ b/website/docs/cli.md @@ -25,15 +25,15 @@ icebergsca scan | Flag | Effect | |---|---| -| `--ecosystem ` | Ecosystems to restrict to: `pypi`, `npm`, `maven`, `go`, `cargo`, `nuget`, `rubygems`. Repeatable, or comma-separated | +| `--ecosystem ` | Ecosystem to restrict to: `pypi`, `npm`, `maven`, `go`, `cargo`, `nuget`, `rubygems`. Repeatable | | `--exclude ` | Glob to skip. Repeatable | | `--max-depth ` | Maximum directory depth to walk | | `--follow-symlinks` | Follow symlinked directories | | `--include-dev` | Include dev and test dependencies, which are excluded by default | -| `--scope ` | Scopes to include, overriding `--include-dev`: `runtime`, `dev`, `test`, `build`, `optional`. Repeatable, or comma-separated | +| `--scope ` | Scope to include, overriding `--include-dev`: `runtime`, `dev`, `test`, `build`, `optional`. Repeatable | ```bash -icebergsca scan . --ecosystem pypi,npm --exclude 'tests/fixtures/**' +icebergsca scan . --ecosystem pypi --ecosystem npm --exclude 'tests/fixtures/**' icebergsca scan . --scope runtime # only what ships icebergsca scan ./requirements.txt # a single file ``` diff --git a/website/docs/ecosystems.md b/website/docs/ecosystems.md index 80b9e74..ea0ec80 100644 --- a/website/docs/ecosystems.md +++ b/website/docs/ecosystems.md @@ -18,10 +18,10 @@ filename, so a file only has to be where a build tool would put it. | .NET | `nuget` | `*.csproj`, `packages.config` | `packages.lock.json` | yes | | Ruby | `rubygems` | `Gemfile`, `*.gemspec` | `Gemfile.lock` | yes | -Restrict a scan with a comma-separated list: +Restrict a scan by repeating the flag: ```bash -icebergsca scan . --ecosystem pypi,npm +icebergsca scan . --ecosystem pypi --ecosystem npm ``` !!! note "Three vocabularies that genuinely disagree" From 425f3de35ab52e24fed803fa711cd394b7d7d58c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 23:09:07 +0000 Subject: [PATCH 3/3] tests: strip ANSI before asserting on rendered help MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_enum_choices_are_listed_in_help passed locally and failed on all four CI Pythons. Rich counts GitHub Actions as a terminal, so help output is bare off CI and styled on it — and it styles each choice in a metavar separately, putting escape codes between every value: <\x1b[1;33mpypi\x1b[0m|\x1b[1;33mnpm\x1b[0m|... so the plain substring is never present there. Strips the codes first. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CJSEXcpbfSLbYqHc2t2WY2 --- tests/test_cli.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 9f53f7a..bee7f73 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import re from pathlib import Path import pytest @@ -14,6 +15,19 @@ runner = CliRunner() +_ANSI = re.compile(r"\x1b\[[0-9;]*m") + + +def plain(text: str) -> str: + """Strip the styling Rich applies when it thinks it is writing to a terminal. + + Rich treats CI as a terminal, so help output is bare locally but styled under + GitHub Actions — and it styles each choice in a metavar separately, which puts + escape codes between every value. Anything asserting on rendered help has to + strip them or it passes only off CI. + """ + return _ANSI.sub("", text) + def project(tmp_path: Path) -> Path: (tmp_path / "requirements.txt").write_text("requests==2.31.0\nflask>=2.0\n") @@ -117,8 +131,9 @@ def test_enum_choices_are_listed_in_help() -> None: """A bare ```` metavar tells a reader nothing about what is accepted.""" # Wide enough that Rich does not wrap the metavar column mid-value. result = runner.invoke(app, ["scan", "--help"], env={"COLUMNS": "200"}) - assert "" in result.stdout - assert "" in result.stdout + rendered = plain(result.stdout) + assert "" in rendered + assert "" in rendered def test_exclude_glob(tmp_path: Path) -> None: