diff --git a/aai_cli/app/context.py b/aai_cli/app/context.py index 2e734fcc..eb082ee6 100644 --- a/aai_cli/app/context.py +++ b/aai_cli/app/context.py @@ -26,6 +26,10 @@ class AppState: profile: str | None = None env: str | None = None quiet: bool = False + # Set by the root callback when config.toml is unreadable during environment + # resolution: most commands re-raise it (run_command), but `config path` — the + # command you reach for to *find* the broken file — tolerates it. + deferred_config_error: CLIError | None = None def resolve_profile(self) -> str: """The profile to act on: explicit --profile, else the active profile. @@ -64,10 +68,12 @@ def resolve_session(self) -> tuple[int, str]: # never satisfy these endpoints — they authenticate with the browser # session, not the API key — so spell out the only fix that works. raise NotAuthenticated( - "These commands need a browser login. Run 'assembly login' (without --api-key).", + "Account commands read account-level data and authenticate with your " + "browser-login session, which this profile doesn't have.", suggestion=( - "Run 'assembly login' to sign in via your browser — an API key alone " - "can't access account commands." + "Run 'assembly login' to sign in via your browser. Unlike transcription " + "commands (which work with an API key), account data like balance, usage, " + "and keys needs that session." ), ) # Registered like the API key in config.resolve_api_key: -v/-vv diagnostics @@ -197,16 +203,34 @@ def run_command( *, json: bool = False, auto_login: bool = True, + tolerate_unreadable_config: bool = False, ) -> None: - """Execute a command body, mapping CLIError to clean output + exit code.""" + """Execute a command body, mapping CLIError to clean output + exit code. + + `tolerate_unreadable_config` lets a command (only `config path`) run even when the + root callback deferred a corrupt-config error, so it can still report the file's + location; every other command re-raises that error here. + """ state: AppState = ctx.obj json_mode = output.resolve_json(explicit=json) + deferred = state.deferred_config_error + if deferred is not None and not tolerate_unreadable_config: + # The root callback couldn't read config.toml. Surface that for ordinary + # commands (which depend on it) the same way the callback used to — emit and + # exit, without telemetry/update-check, both of which would just re-parse it. + _fail(deferred, json_mode=json_mode) try: - # Inside the try so telemetry sees the raw CLIError (and its error_type) - # before it's folded into a typer.Exit below. - with telemetry.track(ctx.command_path): + if deferred is not None: + # `config path` opted in (tolerate_unreadable_config): it reports a + # contents-independent location, so run just the body and skip the + # telemetry/update-check wrappers that re-parse the broken config. fn(state, json_mode) - update_check.maybe_notify(json_mode=json_mode) + else: + # Inside the try so telemetry sees the raw CLIError (and its error_type) + # before it's folded into a typer.Exit below. + with telemetry.track(ctx.command_path): + fn(state, json_mode) + update_check.maybe_notify(json_mode=json_mode) except NotAuthenticated as err: if not auto_login or not _should_auto_login(err): _fail(err, json_mode=json_mode) diff --git a/aai_cli/app/init_exec.py b/aai_cli/app/init_exec.py index 3accf7ae..87fc29e7 100644 --- a/aai_cli/app/init_exec.py +++ b/aai_cli/app/init_exec.py @@ -141,6 +141,24 @@ def _install_step( ], will_launch +def _reject_file_ancestor(target: Path) -> None: + """Reject a target that descends through an existing file (e.g. ``somefile/app``). + + ``scaffold`` calls ``target.mkdir(parents=True)``, which raises a raw + ``NotADirectoryError`` mid-scaffold when a parent component is a regular file — + surfacing as an "Unexpected error … report a bug" line for what is really a bad + path. Catch it up front as a clean usage error instead. + """ + for ancestor in target.parents: + if ancestor.exists(): + if not ancestor.is_dir(): + raise UsageError( + f"{ancestor} is not a directory, so {target} can't be created.", + suggestion="Pick a target whose parent directories are real directories.", + ) + return + + def _resolve_target( directory: str | None, chosen: str, *, here: bool, force: bool ) -> tuple[Path, bool]: @@ -155,6 +173,7 @@ def _resolve_target( target = _resolve_dir(directory, chosen, here=here) if target.exists() and not target.is_dir(): raise UsageError(f"{target} exists and is not a directory.") + _reject_file_ancestor(target) conflict = scaffold.target_conflict(target) if conflict and not force: raise CLIError( diff --git a/aai_cli/commands/clip/_exec.py b/aai_cli/commands/clip/_exec.py index 3ab37d2b..6ca0cc86 100644 --- a/aai_cli/commands/clip/_exec.py +++ b/aai_cli/commands/clip/_exec.py @@ -193,11 +193,19 @@ def _transcript_segments( def _validate_out_dir(out_dir: Path | None) -> None: - if out_dir is not None and not out_dir.is_dir(): + if out_dir is None or out_dir.is_dir(): + return + # Distinguish a missing path from one that exists but isn't a directory: the old + # "doesn't exist" wording was misleading when --out-dir pointed at a regular file. + if out_dir.exists(): raise UsageError( - f"--out-dir doesn't exist: {out_dir}", - suggestion="Create it first, or point --out-dir at an existing directory.", + f"--out-dir is not a directory: {out_dir}", + suggestion="Point --out-dir at a directory, not a file.", ) + raise UsageError( + f"--out-dir doesn't exist: {out_dir}", + suggestion="Create it first, or point --out-dir at an existing directory.", + ) def _validate_selection(opts: ClipOptions) -> None: diff --git a/aai_cli/commands/config.py b/aai_cli/commands/config.py index 1b197866..ceedde76 100644 --- a/aai_cli/commands/config.py +++ b/aai_cli/commands/config.py @@ -112,7 +112,9 @@ def body(_state: AppState, json_mode: bool) -> None: # unwrapped (`cd "$(assembly config path | xargs dirname)"`). output.emit_text(str(file)) - run_command(ctx, body, json=json_out) + # The location is independent of the file's contents, so report it even when the + # config is unreadable — this is the command you'd use to go fix the broken file. + run_command(ctx, body, json=json_out, tolerate_unreadable_config=True) @app.command( diff --git a/aai_cli/commands/stream/__init__.py b/aai_cli/commands/stream/__init__.py index 9d558fa1..f3dd2638 100644 --- a/aai_cli/commands/stream/__init__.py +++ b/aai_cli/commands/stream/__init__.py @@ -3,6 +3,7 @@ from pathlib import Path import typer +from assemblyai import PIISubstitutionPolicy from assemblyai.streaming.v3 import Encoding, NoiseSuppressionModel, SpeechModel from aai_cli import command_registry, help_panels, options @@ -210,7 +211,7 @@ def stream( help="Comma-separated PII policies", rich_help_panel=help_panels.OPT_GUARDRAILS, ), - redact_pii_sub: str | None = typer.Option( + redact_pii_sub: PIISubstitutionPolicy | None = typer.Option( None, "--redact-pii-sub", help="Replace redacted PII with: hash or entity_name", diff --git a/aai_cli/commands/stream/_exec.py b/aai_cli/commands/stream/_exec.py index f2642e56..9f007a77 100644 --- a/aai_cli/commands/stream/_exec.py +++ b/aai_cli/commands/stream/_exec.py @@ -13,6 +13,7 @@ from dataclasses import dataclass from pathlib import Path +from assemblyai import PIISubstitutionPolicy from assemblyai.streaming.v3 import Encoding, NoiseSuppressionModel, SpeechModel from aai_cli import code_gen @@ -67,7 +68,7 @@ class StreamOptions: filter_profanity: bool | None redact_pii: bool | None redact_pii_policy: str | None - redact_pii_sub: str | None + redact_pii_sub: PIISubstitutionPolicy | None webhook_url: str | None webhook_auth_header: str | None llm_prompt: list[str] | None @@ -111,7 +112,7 @@ def base_flags(self) -> dict[str, object]: "voice_focus_threshold": self.voice_focus_threshold, "redact_pii": self.redact_pii, "redact_pii_policies": config_builder.split_csv(self.redact_pii_policy), - "redact_pii_sub": self.redact_pii_sub, + "redact_pii_sub": config_builder.enum_value(self.redact_pii_sub), "inactivity_timeout": self.inactivity_timeout, "webhook_url": self.webhook_url, "prompt": self.prompt, diff --git a/aai_cli/commands/telemetry.py b/aai_cli/commands/telemetry.py index 0e06e077..e2816096 100644 --- a/aai_cli/commands/telemetry.py +++ b/aai_cli/commands/telemetry.py @@ -92,11 +92,24 @@ def enable( def body(_state: AppState, json_mode: bool) -> None: config.set_telemetry_enabled(enabled=True) - output.emit( - {"telemetry_enabled": True}, - lambda _d: output.success("Telemetry enabled."), - json_mode=json_mode, - ) + # An env kill-switch (AAI_TELEMETRY_DISABLED / DO_NOT_TRACK) outranks the stored + # choice, so persisting "enabled" wouldn't actually turn telemetry back on while + # it's set — say so instead of an unqualified success. + source = telemetry.consent_source() + env_var = source.removeprefix("env:") if source.startswith("env:") else None + + def render(_d: dict[str, bool]) -> object: + line = output.success("Telemetry enabled.") + if env_var is None: + return line + return output.stack( + line, + output.hint( + f"Note: {env_var} is set, which keeps telemetry off until you unset it." + ), + ) + + output.emit({"telemetry_enabled": True}, render, json_mode=json_mode) run_command(ctx, body, json=json_out) diff --git a/aai_cli/commands/webhooks/__init__.py b/aai_cli/commands/webhooks/__init__.py index 0e07f416..b498d41b 100644 --- a/aai_cli/commands/webhooks/__init__.py +++ b/aai_cli/commands/webhooks/__init__.py @@ -37,7 +37,11 @@ def listen( ctx: typer.Context, port: int = typer.Option( - 8989, "--port", help="Local listener port (the first free port from here)" + 8989, + "--port", + min=0, + max=65535, + help="Local listener port (the first free port from here)", ), forward_to: str | None = typer.Option( None, "--forward-to", help="Re-POST each delivery to this URL (e.g. your local app)" diff --git a/aai_cli/core/llm.py b/aai_cli/core/llm.py index 19b5747b..663247df 100644 --- a/aai_cli/core/llm.py +++ b/aai_cli/core/llm.py @@ -5,7 +5,7 @@ from typing import TYPE_CHECKING, Any from aai_cli.core import environments -from aai_cli.core.errors import APIError, UsageError +from aai_cli.core.errors import APIError, UsageError, auth_failure if TYPE_CHECKING: from openai import OpenAI @@ -116,12 +116,18 @@ def _client(api_key: str) -> OpenAI: ) +def _is_entitlement_denial(exc: object) -> bool: + """True when a gateway 401/403 reads as a plan-entitlement block rather than a + bad key or an intercepting proxy.""" + text = f"{exc} {getattr(exc, 'body', None) or ''}".lower() + return any(hint in text for hint in _ENTITLEMENT_HINTS) + + def _denial_suggestion(exc: object) -> str: """Pick the suggestion for a gateway 401/403: point at billing only when the response actually mentions the plan entitlement, otherwise at key/network — a corporate-proxy 403 must not send users to the billing page.""" - text = f"{exc} {getattr(exc, 'body', None) or ''}".lower() - if any(hint in text for hint in _ENTITLEMENT_HINTS): + if _is_entitlement_denial(exc): return _PAID_PLAN_SUGGESTION return _ACCESS_DENIED_SUGGESTION @@ -159,9 +165,14 @@ def complete( ) except (openai.AuthenticationError, openai.PermissionDeniedError) as exc: # The gateway returns 401/403 for an invalid key, a proxy block, and a - # plan entitlement block ("no access to LLM Gateway"), so surface its - # actual message and pick the suggestion from what it says — only an - # entitlement message should point at billing. + # plan entitlement block ("no access to LLM Gateway"). A plain 401 + # (AuthenticationError) with no entitlement hint is just a rejected key, so + # surface the same clean exit-4 auth_failure transcribe gives instead of + # echoing the gateway's raw 401 body. A 403 (proxy or entitlement) keeps the + # gateway's own message and picks the suggestion from what it says — only an + # entitlement message should point at billing, never a corporate-proxy 403. + if isinstance(exc, openai.AuthenticationError) and not _is_entitlement_denial(exc): + raise auth_failure() from exc raise APIError( f"LLM Gateway access denied: {exc}", suggestion=_denial_suggestion(exc), diff --git a/aai_cli/main.py b/aai_cli/main.py index b5d7a76a..84025547 100644 --- a/aai_cli/main.py +++ b/aai_cli/main.py @@ -190,8 +190,19 @@ def main( try: environments.set_active(state.resolve_environment()) except CLIError as err: - output.emit_error(err, json_mode=json_mode) - raise typer.Exit(code=err.exit_code) from None + if err.error_type != "invalid_config": + output.emit_error(err, json_mode=json_mode) + raise typer.Exit(code=err.exit_code) from None + # A corrupt config.toml can't tell us the profile's stored env. Defer the error + # (run_command re-raises it for real commands) and fall back to the explicit + # --env or the default, so `assembly config path` can still locate the file. + # An explicit bad --env still wins — surface it now rather than the deferred one. + state.deferred_config_error = err + try: + environments.set_active(environments.resolve(state.env, None)) + except CLIError as env_err: + output.emit_error(env_err, json_mode=json_mode) + raise typer.Exit(code=env_err.exit_code) from None active_env = environments.active() _LOG.debug("environment: %s (%s)", active_env.name, active_env.api_base) for warning in (conflict_warning, state.env_override_warning()): diff --git a/scripts/check.sh b/scripts/check.sh index e238a58d..cbd8e511 100755 --- a/scripts/check.sh +++ b/scripts/check.sh @@ -200,8 +200,21 @@ echo "==> brew audit (Homebrew formula)" # Lint the formula we ship (Formula/assembly.rb) the way Homebrew's own CI does, so a # formula regression fails here instead of on the release PR. brew is macOS/Linuxbrew # only, so this self-skips where it isn't installed (CI's release path has it). +# +# Homebrew 6+ disabled `brew audit [path ...]` — a formula must be audited by NAME, +# which means it has to live in a tap. Copy ours into an ephemeral local tap, audit by +# name, then remove it (works on both macOS and Linuxbrew, old and new). The explicit +# status capture keeps the tap cleanup running under `set -e` even when the audit fails. if command -v brew >/dev/null 2>&1; then - brew audit --strict --formula Formula/assembly.rb + audit_tap="$(brew --repository)/Library/Taps/local/homebrew-aaiaudit" + mkdir -p "$audit_tap/Formula" + cp Formula/assembly.rb "$audit_tap/Formula/" + audit_status=0 + brew audit --strict --formula local/aaiaudit/assembly || audit_status=$? + rm -rf "$audit_tap" + if [ "$audit_status" -ne 0 ]; then + exit "$audit_status" + fi else echo " brew not found; skipping (Homebrew CI / release runner has it)" fi @@ -256,11 +269,22 @@ if git rev-parse --verify --quiet origin/main >/dev/null; then # though the branch itself added nothing. The merge-base only moves when the # branch itself rebases. gate_base="$(git merge-base origin/main HEAD || echo origin/main)" + + # Count hatch hits with ONE matcher on both sides so baseline and working tree compare + # apples-to-apples. Using `rg` for the working tree but `git grep -E` for the baseline + # diverged on `\b`: ERE ignores it on macOS (matching nothing) while rg honors it, so a + # pre-existing time.sleep() inflated the working count over the baseline and failed this + # gate on macOS though it passed on Linux. `git grep -P` (PCRE) handles `\b` identically + # on both platforms; `--untracked` counts newly-added (unstaged) files the way rg did. + # Patterns must be PCRE-valid (escape literal parens, e.g. `cast\(`). + hatch_base() { { git grep -hP "$1" "$gate_base" -- "${@:2}" || true; } | wc -l | tr -d '[:space:]'; } + hatch_work() { { git grep --untracked -hP "$1" -- "${@:2}" || true; } | wc -l | tr -d '[:space:]'; } + hatch_pattern='# type: ignore|# noqa|pragma: no cover' - base_hatch_count="$({ git grep -nE "$hatch_pattern" "$gate_base" -- aai_cli tests || true; } | wc -l | tr -d '[:space:]')" - work_hatch_count="$({ rg -n "$hatch_pattern" aai_cli tests || true; } | wc -l | tr -d '[:space:]')" + base_hatch_count="$(hatch_base "$hatch_pattern" aai_cli tests)" + work_hatch_count="$(hatch_work "$hatch_pattern" aai_cli tests)" if (( work_hatch_count > base_hatch_count )); then - { rg -n "$hatch_pattern" aai_cli tests || true; } | tail -n 20 + { git grep --untracked -nP "$hatch_pattern" -- aai_cli tests || true; } | tail -n 20 echo "New static-analysis ignore/no-cover escape hatch found: ${work_hatch_count} current vs ${base_hatch_count} at the merge-base with origin/main. Refactor it or update the gate explicitly." exit 1 fi @@ -273,23 +297,23 @@ if git rev-parse --verify --quiet origin/main >/dev/null; then # new one must update this gate deliberately. Scoped to tests/ — production sleeps # are fine. shortcut_pattern='pytest\.skip\(|pytest\.xfail\(|@pytest\.mark\.(skip|xfail)|\btime\.sleep\(' - base_shortcut_count="$({ git grep -nE "$shortcut_pattern" "$gate_base" -- tests || true; } | wc -l | tr -d '[:space:]')" - work_shortcut_count="$({ rg -n "$shortcut_pattern" tests || true; } | wc -l | tr -d '[:space:]')" + base_shortcut_count="$(hatch_base "$shortcut_pattern" tests)" + work_shortcut_count="$(hatch_work "$shortcut_pattern" tests)" if (( work_shortcut_count > base_shortcut_count )); then - { rg -n "$shortcut_pattern" tests || true; } | tail -n 20 + { git grep --untracked -nP "$shortcut_pattern" -- tests || true; } | tail -n 20 echo "New test skip/xfail/time.sleep found: ${work_shortcut_count} current vs ${base_shortcut_count} at the merge-base with origin/main. Fix the test (or sync properly) or update the gate explicitly." exit 1 fi - base_any_count="$({ git grep -n "Any" "$gate_base" -- aai_cli tests || true; } | wc -l | tr -d '[:space:]')" - work_any_count="$({ rg -n "Any" aai_cli tests || true; } | wc -l | tr -d '[:space:]')" + base_any_count="$(hatch_base "Any" aai_cli tests)" + work_any_count="$(hatch_work "Any" aai_cli tests)" if (( work_any_count > base_any_count )); then echo "New Any usage found: ${work_any_count} current vs ${base_any_count} at the merge-base with origin/main." exit 1 fi - base_cast_count="$({ git grep -n "cast(" "$gate_base" -- aai_cli tests || true; } | wc -l | tr -d '[:space:]')" - work_cast_count="$({ rg -n "cast\\(" aai_cli tests || true; } | wc -l | tr -d '[:space:]')" + base_cast_count="$(hatch_base 'cast\(' aai_cli tests)" + work_cast_count="$(hatch_work 'cast\(' aai_cli tests)" if (( work_cast_count > base_cast_count )); then echo "New cast() usage found: ${work_cast_count} current vs ${base_cast_count} at the merge-base with origin/main." exit 1 diff --git a/tests/__snapshots__/test_snapshots_help_run.ambr b/tests/__snapshots__/test_snapshots_help_run.ambr index 9de67337..0a15c397 100644 --- a/tests/__snapshots__/test_snapshots_help_run.ambr +++ b/tests/__snapshots__/test_snapshots_help_run.ambr @@ -651,11 +651,11 @@ │ seconds idle │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Guardrails ─────────────────────────────────────────────────────────────────╮ - │ --filter-profanity Mask profanity │ - │ --redact-pii Redact PII from turns │ - │ --redact-pii-policy TEXT Comma-separated PII policies │ - │ --redact-pii-sub TEXT Replace redacted PII with: hash or │ - │ entity_name │ + │ --filter-profanity Mask profanity │ + │ --redact-pii Redact PII from turns │ + │ --redact-pii-policy TEXT Comma-separated PII policies │ + │ --redact-pii-sub [hash|entity_name] Replace redacted PII with: │ + │ hash or entity_name │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Webhooks ───────────────────────────────────────────────────────────────────╮ │ --webhook-url TEXT Webhook URL │ @@ -1100,18 +1100,21 @@ downloads/). ╭─ Options ────────────────────────────────────────────────────────────────────╮ - │ --port INTEGER Local listener port (the first │ - │ free port from here) │ - │ [default: 8989] │ - │ --forward-to TEXT Re-POST each delivery to this │ - │ URL (e.g. your local app) │ - │ --no-tunnel Local-only: skip the cloudflared │ - │ public URL │ - │ --max-events INTEGER RANGE [x>=0] Exit after this many deliveries │ - │ (0 = run until Ctrl-C) │ - │ [default: 0] │ - │ --json -j One NDJSON record per delivery │ - │ --help Show this message and exit. │ + │ --port INTEGER RANGE Local listener port (the │ + │ [0<=x<=65535] first free port from here) │ + │ [default: 8989] │ + │ --forward-to TEXT Re-POST each delivery to │ + │ this URL (e.g. your local │ + │ app) │ + │ --no-tunnel Local-only: skip the │ + │ cloudflared public URL │ + │ --max-events INTEGER RANGE [x>=0] Exit after this many │ + │ deliveries (0 = run until │ + │ Ctrl-C) │ + │ [default: 0] │ + │ --json -j One NDJSON record per │ + │ delivery │ + │ --help Show this message and exit. │ ╰──────────────────────────────────────────────────────────────────────────────╯ Examples diff --git a/tests/test_clip_exec.py b/tests/test_clip_exec.py index aa887161..2f2f6bf7 100644 --- a/tests/test_clip_exec.py +++ b/tests/test_clip_exec.py @@ -134,6 +134,18 @@ def test_run_clip_rejects_missing_out_dir(media, tmp_path): assert "Create it first" in (exc.value.suggestion or "") +def test_run_clip_rejects_out_dir_that_is_a_file(media, tmp_path): + # A path that exists but is a regular file isn't "doesn't exist" — say so precisely. + a_file = tmp_path / "notes.txt" + a_file.write_text("x") + opts = dataclasses.replace(DEFAULTS, media=str(media), ranges=["1-2"], out_dir=a_file) + with pytest.raises(UsageError) as exc: + clip_exec.run_clip(opts, AppState(), json_mode=False) + assert "--out-dir is not a directory" in exc.value.message + assert "doesn't exist" not in exc.value.message + assert "not a file" in (exc.value.suggestion or "") + + def test_run_clip_requires_ffmpeg(media, monkeypatch): monkeypatch.setattr("shutil.which", lambda name: None) opts = dataclasses.replace(DEFAULTS, media=str(media), ranges=["1-2"]) diff --git a/tests/test_config_command.py b/tests/test_config_command.py index da4cf60e..3daeb5ac 100644 --- a/tests/test_config_command.py +++ b/tests/test_config_command.py @@ -75,6 +75,33 @@ def test_config_path_json(): assert json.loads(result.output) == {"path": str(config.config_file_path())} +def test_config_path_works_even_when_config_is_corrupt(tmp_config): + # The location is independent of the file's contents, so `config path` — the command + # you'd reach for to go fix a broken config — must still report it, not fail on it. + (tmp_config / "config.toml").write_text("this is = = not toml [[[") + result = runner.invoke(app, ["config", "path"]) + assert result.exit_code == 0 + assert result.output.strip() == str(config.config_file_path()) + + +def test_config_list_still_errors_when_config_is_corrupt(tmp_config): + # Other commands re-raise the deferred corrupt-config error (they actually read it). + (tmp_config / "config.toml").write_text("this is = = not toml [[[") + result = runner.invoke(app, ["config", "list", "--json"]) + assert result.exit_code == 2 + assert json.loads(result.output)["error"]["type"] == "invalid_config" + + +def test_bad_env_flag_wins_over_a_corrupt_config(tmp_config): + # Corrupt config defers, but an explicit bad --env is still surfaced as the + # invalid_environment usage error rather than the deferred config error. + (tmp_config / "config.toml").write_text("this is = = not toml [[[") + result = runner.invoke(app, ["--env", "bogusenv", "config", "path"]) + assert result.exit_code == 2 + assert "bogusenv" in result.output + assert "invalid TOML" not in result.output + + def test_config_list_json_is_the_full_settings_object(): config.set_api_key("staging", "sk_2") config.set_profile_env("staging", "sandbox000") diff --git a/tests/test_context.py b/tests/test_context.py index 48d9bf39..e0d0285b 100644 --- a/tests/test_context.py +++ b/tests/test_context.py @@ -8,7 +8,7 @@ from aai_cli.app.context import AppState, _interactive_session, run_command from aai_cli.auth.flow import LoginResult from aai_cli.core import config, environments -from aai_cli.core.errors import APIError, NotAuthenticated, auth_failure +from aai_cli.core.errors import APIError, CLIError, NotAuthenticated, auth_failure runner = CliRunner() @@ -331,8 +331,15 @@ def test_resolve_session_raises_when_no_session(): from aai_cli.app.context import AppState from aai_cli.core.errors import NotAuthenticated - with pytest.raises(NotAuthenticated): + with pytest.raises(NotAuthenticated) as exc: AppState().resolve_session() + # The message explains *why* account commands differ from API-key commands, so + # users aren't confused that `transcripts` works but `balance` says "not signed in". + assert "account-level data" in exc.value.message + assert "browser-login session" in exc.value.message + suggestion = exc.value.suggestion or "" + assert "sign in via your browser" in suggestion + assert "balance, usage" in suggestion def test_resolve_session_raises_when_only_account_id_missing(monkeypatch): @@ -448,14 +455,13 @@ def test_resolve_session_suggestion_never_offers_api_key_env_var(): AppState().resolve_session() assert exc.value.suggestion is not None assert "browser" in exc.value.suggestion - assert "API key alone" in exc.value.suggestion + assert "API key" in exc.value.suggestion # explains why the key won't do assert "ASSEMBLYAI_API_KEY" not in exc.value.suggestion def test_resolve_profile_rejects_invalid_explicit_profile_fast(): # Validated at resolution time (the root callback), so a typo'd --profile is a # fast exit-2 before any network round-trip, not a keyring-write-time failure. - from aai_cli.core.errors import CLIError with pytest.raises(CLIError) as exc: AppState(profile="bad name!").resolve_profile() diff --git a/tests/test_context_deferred_config.py b/tests/test_context_deferred_config.py new file mode 100644 index 00000000..007fa4d9 --- /dev/null +++ b/tests/test_context_deferred_config.py @@ -0,0 +1,60 @@ +"""`run_command`'s handling of a corrupt-config error the root callback deferred. + +Split out of test_context.py to keep modules under the 500-line gate. When config.toml +is unreadable, the callback can't resolve the profile's stored env; it defers the error +so `assembly config path` (which reports a contents-independent location) can still run, +while every other command re-raises it here. +""" + +import typer +from typer.testing import CliRunner + +from aai_cli.app.context import AppState, run_command +from aai_cli.core.errors import CLIError + +runner = CliRunner() + + +def _make_app_with_deferred(body, deferred, **run_options): + app = typer.Typer() + + @app.callback() + def cb(ctx: typer.Context): + ctx.obj = AppState(deferred_config_error=deferred) + + @app.command() + def go(ctx: typer.Context): + run_command(ctx, body, **run_options) + + return app + + +def test_run_command_reraises_deferred_config_error_for_normal_command(): + # A corrupt-config error the root callback deferred surfaces here for ordinary + # commands — they never run their body against an unreadable config. + deferred = CLIError("bad toml", error_type="invalid_config", exit_code=2) + ran = {} + + result = runner.invoke( + _make_app_with_deferred(lambda *_: ran.setdefault("body", True), deferred), ["go"] + ) + assert result.exit_code == 2 + assert "bad toml" in result.output + assert "body" not in ran # body never ran + + +def test_run_command_tolerates_deferred_config_error_when_opted_in(): + # `config path` opts in: it runs despite the deferred error (and skips the + # telemetry/update-check wrappers that would re-parse the broken config). + deferred = CLIError("bad toml", error_type="invalid_config", exit_code=2) + ran = {} + + result = runner.invoke( + _make_app_with_deferred( + lambda *_: ran.setdefault("body", True), deferred, tolerate_unreadable_config=True + ), + ["go"], + ) + assert result.exit_code == 0 + assert ran.get("body") is True + assert "bad toml" not in result.output diff --git a/tests/test_dub_exec.py b/tests/test_dub_exec.py index 2350c66e..61802675 100644 --- a/tests/test_dub_exec.py +++ b/tests/test_dub_exec.py @@ -7,6 +7,7 @@ from __future__ import annotations +import contextlib import dataclasses import os from pathlib import Path @@ -257,7 +258,11 @@ def test_validate_out_rejects_the_input_via_hard_link(media): # Two spellings of one file (mimics --out TALK.MP4 on a case-insensitive # filesystem): path comparison passes, samefile must still catch it. clone = media.parent / "TALK.MP4" - os.link(media, clone) + # On a case-insensitive filesystem (e.g. macOS default) TALK.MP4 already *is* + # talk.mp4, so the link "exists" — the two spellings resolve to one file without an + # explicit hard link, which is exactly the case samefile must still catch. + with contextlib.suppress(FileExistsError): + os.link(media, clone) with pytest.raises(UsageError) as exc: mediafile.validate_out(clone, media) assert "overwrite the input file" in exc.value.message diff --git a/tests/test_init_force_and_report.py b/tests/test_init_force_and_report.py index b5bf25f5..80a69078 100644 --- a/tests/test_init_force_and_report.py +++ b/tests/test_init_force_and_report.py @@ -32,6 +32,21 @@ def test_init_target_is_existing_file_usage_error(tmp_path, monkeypatch): assert (tmp_path / "myapp").read_text() == "I am a file" # left untouched +def test_init_target_descends_through_a_file_usage_error(tmp_path, monkeypatch): + # A target whose PARENT is a file (e.g. somefile/sub) is a clean usage error, not + # the raw "Unexpected error: [Errno 20] Not a directory … report a bug" that + # mkdir(parents=True) would otherwise hit mid-scaffold. + monkeypatch.chdir(tmp_path) + (tmp_path / "plainfile").write_text("I am a file") + # Nested one level past the file ("plainfile/sub/deeper") so the walk also covers + # the not-yet-existing intermediate before it reaches the offending file. + result = runner.invoke(app, ["init", TEMPLATE, "plainfile/sub/deeper", "--no-install"]) + assert result.exit_code == 2 + assert "is not a directory" in result.output + assert "Unexpected error" not in result.output + assert (tmp_path / "plainfile").read_text() == "I am a file" # left untouched + + def test_init_force_warns_existing_files_are_overwritten(tmp_path, monkeypatch): # --force overlays the template onto a non-empty target; the run must say so # (on stderr) instead of silently clobbering files. diff --git a/tests/test_llm.py b/tests/test_llm.py index e86eab3c..ebce9cdd 100644 --- a/tests/test_llm.py +++ b/tests/test_llm.py @@ -7,7 +7,7 @@ from openai.types.chat import ChatCompletion from aai_cli.core import environments, llm -from aai_cli.core.errors import APIError +from aai_cli.core.errors import APIError, NotAuthenticated _GATEWAY_BASE = environments.get(environments.DEFAULT_ENV).llm_gateway_base _REQUEST = httpx.Request("POST", f"{_GATEWAY_BASE}/chat/completions") @@ -65,18 +65,35 @@ def test_complete_passes_transcript_id_as_extra_body(monkeypatch): assert seen["extra_body"] == {"transcript_id": "t_42"} -def test_complete_auth_error_surfaces_gateway_message(monkeypatch): +def test_complete_bad_key_401_raises_clean_auth_failure(monkeypatch): + # A plain 401 with no entitlement hint is just a rejected key: surface the same + # clean exit-4 auth_failure transcribe gives, not the gateway's raw 401 body. err = openai.AuthenticationError( - "bad key", response=httpx.Response(401, request=_REQUEST), body=None + "Authentication error, API token missing/invalid", + response=httpx.Response(401, request=_REQUEST), + body=None, + ) + _fake_client(monkeypatch, error=err) + with pytest.raises(NotAuthenticated) as exc: + llm.complete("sk", model="m", messages=[]) + assert exc.value.exit_code == 4 + assert exc.value.rejected_key is True + # Not the raw gateway passthrough (which would leak the 401 body). + assert "access denied" not in exc.value.message + + +def test_complete_entitlement_401_still_points_at_billing(monkeypatch): + # A 401 that *does* read as an entitlement block keeps the exit-1 billing pointer + # rather than the rejected-key path. + err = openai.AuthenticationError( + "Your account has no access to the LLM Gateway.", + response=httpx.Response(401, request=_REQUEST), + body=None, ) _fake_client(monkeypatch, error=err) with pytest.raises(APIError, match="access denied") as exc: llm.complete("sk", model="m", messages=[]) - # Nothing in "bad key" mentions the plan entitlement, so the hint points at the - # key/network, not billing. - assert exc.value.suggestion is not None - assert "paid plan" not in exc.value.suggestion - assert "API key" in exc.value.suggestion and "network" in exc.value.suggestion + assert exc.value.suggestion is not None and "paid plan" in exc.value.suggestion def test_complete_entitlement_denial_suggests_paid_plan(monkeypatch): diff --git a/tests/test_stream_exec.py b/tests/test_stream_exec.py index 4c557260..8b71e2c6 100644 --- a/tests/test_stream_exec.py +++ b/tests/test_stream_exec.py @@ -112,6 +112,17 @@ def test_run_stream_validates_before_resolving_credentials(): ) +def test_redact_pii_sub_enum_maps_to_its_string_value(): + # --redact-pii-sub is an SDK enum (validated at parse time), so base_flags must + # unwrap it to the canonical string the streaming config expects, not pass the + # enum member through. + from assemblyai import PIISubstitutionPolicy + + opts = dataclasses.replace(DEFAULTS, redact_pii_sub=PIISubstitutionPolicy.hash) + assert opts.base_flags()["redact_pii_sub"] == "hash" + assert DEFAULTS.base_flags()["redact_pii_sub"] is None # unset stays None + + def test_stream_options_are_immutable(): field_name = "sample" with pytest.raises(dataclasses.FrozenInstanceError): diff --git a/tests/test_telemetry_command.py b/tests/test_telemetry_command.py index aad947d3..1bc4d253 100644 --- a/tests/test_telemetry_command.py +++ b/tests/test_telemetry_command.py @@ -128,6 +128,29 @@ def test_enable_and_disable_human(): assert "Telemetry enabled." in result.output +def test_enable_notes_env_override_when_kill_switch_set(monkeypatch): + # An env kill-switch outranks the persisted choice, so persisting "enabled" can't + # actually re-enable telemetry while it's set: say so instead of a bare success. + monkeypatch.delenv("AAI_TELEMETRY_DISABLED", raising=False) + monkeypatch.setenv("DO_NOT_TRACK", "1") + result = runner.invoke(app, ["telemetry", "enable"]) + assert result.exit_code == 0 + assert "Telemetry enabled." in result.output + assert "DO_NOT_TRACK is set" in result.output + assert "keeps telemetry off" in result.output + assert "env:" not in result.output # the env: prefix is stripped from the var name + + +def test_enable_has_no_env_note_without_a_kill_switch(monkeypatch): + # The unqualified success path: no env override active, so no caveat is appended. + monkeypatch.delenv("DO_NOT_TRACK", raising=False) + monkeypatch.delenv("AAI_TELEMETRY_DISABLED", raising=False) + result = runner.invoke(app, ["telemetry", "enable"]) + assert result.exit_code == 0 + assert "Telemetry enabled." in result.output + assert "keeps telemetry off" not in result.output + + def test_flush_is_hidden_plumbing(): # `flush` exists for dispatch() to spawn, but users shouldn't be steered to it. result = runner.invoke(app, ["telemetry", "--help"]) diff --git a/tests/test_webhook_listen.py b/tests/test_webhook_listen.py index d7bb9ee8..c735e5b0 100644 --- a/tests/test_webhook_listen.py +++ b/tests/test_webhook_listen.py @@ -96,6 +96,17 @@ def test_listen_human_mode_prints_hint_and_event_line(): assert "{" not in result.output.replace('{"ok": true}', "") # no NDJSON in human mode +@pytest.mark.parametrize("bad_port", ["-1", "99999"]) +def test_listen_rejects_out_of_range_port(bad_port): + # An out-of-range port is a user-input error (exit 2, validated at parse time), + # never an "Unexpected error … report a bug" internal failure from the socket layer. + result = runner.invoke(app, ["webhooks", "listen", "--port", bad_port]) + assert result.exit_code == 2 + # The exact bounds in the message pin the min=0/max=65535 literals against mutation. + assert "0<=x<=65535" in result.output + assert "report it" not in result.output + + # --- forwarding --------------------------------------------------------------------