diff --git a/AGENTS.md b/AGENTS.md index 67d90627..2c651933 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,11 +9,13 @@ this file, so Claude Code reads the same instructions. This project uses [uv](https://docs.astral.sh/uv/). **Run every Python tool through `uv run`** so it uses the locked environment (`pyproject.toml` + `uv.lock`), not whatever is on `PATH`: ```sh -uv sync --extra dev # create/refresh the venv with dev dependencies +uv sync # create/refresh the venv (the dev group installs by default) uv run aai --help # run the CLI from the locked environment ./scripts/check.sh # the full gate CI runs (scripts/check.sh is the source of truth) ``` +Dev tooling is a PEP 735 `[dependency-groups]` group with `default-groups = ["dev"]`, not a `[project]` extra — `uv sync --extra dev` errors. + `scripts/check.sh` is the authoritative gate; keep this list in sync with it. It runs, in order: `uv lock --check` → `ruff check` → `ruff format --check` → `mypy` → `pyright` (src strict) → `pyright` (tests) → `vulture` (dead code) → `deptry` (dependency hygiene) → `lint-imports` (import-linter architecture contracts) → max-file-length (500 lines) → `xenon` (cyclomatic complexity, max grade B / project avg A) → `swiftlint` + swift compile (macOS only, skipped elsewhere) → `markdownlint` → `prettier` (init template JS/CSS) → `shellcheck` → `actionlint` + `zizmor` (workflow lint/audit) → `gitleaks` (secret scan) → generated `--show-code` compile gate → init template contract gate → `pytest` (90% branch coverage) → `diff-cover` (100% patch coverage vs `origin/main`) → **mutation gate** (diff-scoped: mutates each changed line and reruns the tests that cover it — a surviving mutant fails the gate, so changed lines need assertions that would *fail* if the line broke, not just coverage; suppress a genuinely unassertable line with `# pragma: no mutate`) → a "no new escape hatches" diff gate (`# type: ignore` / `# noqa` / `pragma: no cover` / net-new `Any` / `cast(`) → `uv build` + `twine check --strict`. The `vulture`/`deptry`/`lint-imports`/`xenon`, patch-coverage, and mutation stages catch the failures that `ruff`+`mypy` alone won't — don't claim the gate is green until the script prints `All checks passed.` Individual tools (all via `uv run`): @@ -28,16 +30,27 @@ uv run pytest tests/test_transcribe.py -q # a single file uv run pytest tests/test_transcribe.py::test_name -q # a single test ``` +The two diff-scoped tail gates are the slowest failures to discover via the full +script; after a gate run (or any pytest run with the coverage flags below) they can +be re-run alone: + +```sh +uv run pytest -q -n auto --cov=aai_cli --cov-branch --cov-context=test --cov-report=xml # refresh coverage data +uv run diff-cover coverage.xml --compare-branch=origin/main --fail-under=100 # patch-coverage gate +uv run python scripts/mutation_gate.py origin/main # mutation gate +``` + ### Test markers -The default suite **excludes** two slow/credentialed marker sets (see `scripts/check.sh` and `pyproject.toml`): +The default suite **excludes** three slow/credentialed marker sets — `pyproject.toml`'s `addopts` carries `-m "not e2e and not install and not install_script"`, so a bare `pytest` matches what `check.sh` gates. An explicit command-line `-m` overrides it for the opt-in runs: ```sh uv run pytest -m e2e # real-API end-to-end; needs ASSEMBLYAI_API_KEY, else skips +uv run pytest -m install # installs each init template's requirements for real; needs network + uv uv run pytest -m install_script # builds a wheel and runs install.sh for real; needs network + uv/pipx ``` -`check.sh` runs `-m "not e2e and not install_script"` with a **90% branch-coverage gate** (`--cov-fail-under=90`). New code generally needs tests to clear that gate. +`check.sh` runs the default suite with a **90% branch-coverage gate** (`--cov-fail-under=90`). New code generally needs tests to clear that gate. CLI output is pinned by **syrupy snapshot tests** (`tests/__snapshots__/*.ambr`). Changing help text, tables, or rendered output will fail those tests until you regenerate them with `uv run pytest --snapshot-update` and commit the updated `.ambr` files. The auto-format hook only touches `*.py`, and pre-commit's whitespace fixers deliberately skip `tests/__snapshots__/` (syrupy's indentation must stay byte-for-byte), so never hand-edit a snapshot — always regenerate. diff --git a/aai_cli/client.py b/aai_cli/client.py index 9c996f2f..7e59d4ea 100644 --- a/aai_cli/client.py +++ b/aai_cli/client.py @@ -2,7 +2,9 @@ import contextlib import json +import re from collections.abc import Callable, Generator, Iterable +from pathlib import Path from typing import Any, Literal, Protocol import assemblyai as aai @@ -37,10 +39,14 @@ def _make_streaming_client(api_key: str) -> _StreamingClientLike: return client -def resolve_audio_source(source: str | None, *, sample: bool) -> str: +def resolve_audio_source(source: str | None, *, sample: bool, check_local: bool = True) -> str: """The audio reference to use: the hosted --sample clip, else the given path/URL. Shared by `transcribe` and `stream` so both accept a file or URL and `--sample`. + A local path that doesn't exist fails here — before any credential resolution or + request — so a typo'd filename reads as "file not found", not as an API failure. + ``--show-code`` paths pass ``check_local=False``: generating code for a file you + don't have yet is legitimate. """ if sample: return SAMPLE_AUDIO_URL @@ -49,6 +55,13 @@ def resolve_audio_source(source: str | None, *, sample: bool) -> str: "Provide an audio path or URL.", suggestion="Or pass --sample to use the hosted demo file.", ) + if check_local and not source.startswith(("http://", "https://")) and not Path(source).exists(): + raise CLIError( + f"File not found: {source}", + error_type="file_not_found", + exit_code=2, + suggestion="Check the path. For remote audio, pass an http(s):// URL.", + ) return source @@ -169,7 +182,23 @@ def select_transcript_field(transcript: Any, field: str) -> str: return _FIELD_RENDERERS.get(field, _transcript_text)(transcript) +# Transcript ids are opaque url-safe tokens. Reject anything else before it is +# interpolated into the request path: an "id" like ../v2/other would otherwise steer +# an authenticated GET at an arbitrary API route. +_TRANSCRIPT_ID_RE = re.compile(r"^[A-Za-z0-9_-]+$") + + +def validate_transcript_id(transcript_id: str) -> str: + if not _TRANSCRIPT_ID_RE.match(transcript_id): + raise UsageError( + f"{transcript_id!r} doesn't look like a transcript id.", + suggestion="Ids are letters/digits/dashes; find them with 'aai transcripts list'.", + ) + return transcript_id + + def get_transcript(api_key: str, transcript_id: str) -> aai.Transcript: + validate_transcript_id(transcript_id) _configure(api_key) with _sdk_errors(f"Could not fetch transcript {transcript_id}"): return aai.Transcript.get_by_id(transcript_id) diff --git a/aai_cli/commands/agent.py b/aai_cli/commands/agent.py index 9fea9dc7..e6240f97 100644 --- a/aai_cli/commands/agent.py +++ b/aai_cli/commands/agent.py @@ -145,10 +145,14 @@ def body(state: AppState, json_mode: bool) -> None: output.print_code(code_gen.agent(voice, system_prompt_text, greeting)) return - api_key = config.resolve_api_key(profile=state.profile) from_file = bool(source) or sample if from_file and device is not None: raise UsageError("--device applies only to microphone input.") + if from_file: + # Existence-check the clip before credentials, so a typo'd path reads as + # "file not found" instead of triggering a login. + client.resolve_audio_source(source, sample=sample) + api_key = config.resolve_api_key(profile=state.profile) renderer = AgentRenderer( json_mode=json_mode, diff --git a/aai_cli/commands/login.py b/aai_cli/commands/login.py index 4f1d8a67..f7954de2 100644 --- a/aai_cli/commands/login.py +++ b/aai_cli/commands/login.py @@ -6,7 +6,7 @@ from aai_cli import client, config, environments, help_panels, options, output from aai_cli.context import AppState, persist_browser_login, resolve_profile, run_command -from aai_cli.errors import APIError, NotAuthenticated +from aai_cli.errors import APIError from aai_cli.help_text import examples_epilog app = typer.Typer() @@ -107,9 +107,9 @@ def whoami( def body(state: AppState, json_mode: bool) -> None: profile = resolve_profile(state) - key = config.get_api_key(profile) - if not key: - raise NotAuthenticated() + # The full env -> keyring chain (raises NotAuthenticated when empty), so a CI + # box authenticated via ASSEMBLYAI_API_KEY can use whoami as a preflight check. + key = config.resolve_api_key(profile=state.profile) masked = output.mask_secret(key) env = environments.active().name reachable = client.validate_key(key) diff --git a/aai_cli/commands/stream.py b/aai_cli/commands/stream.py index 7db2757d..e413ca7d 100644 --- a/aai_cli/commands/stream.py +++ b/aai_cli/commands/stream.py @@ -162,7 +162,10 @@ def stream( # turn detection end_of_turn_confidence_threshold: float | None = typer.Option( None, - "--end-of-turn-confidence-threshold", + # Not "--end-of-turn-confidence-threshold": at 34 chars the name can't render + # un-clipped in an 80-column help screen, which made it unlearnable from --help. + # The full SDK field stays reachable via --config. + "--end-of-turn-confidence", help="End-of-turn confidence (0-1).", min=0.0, max=1.0, @@ -183,7 +186,7 @@ def stream( vad_threshold: float | None = typer.Option( None, "--vad-threshold", - help="Voice-activity threshold (0-1).", + help="Voice activity threshold (0-1).", min=0.0, max=1.0, rich_help_panel=help_panels.OPT_TURNS, @@ -191,7 +194,7 @@ def stream( format_turns: bool | None = typer.Option( None, "--format-turns/--no-format-turns", - help="Punctuate/format finalized turns.", + help="Format (punctuate) finalized turns.", rich_help_panel=help_panels.OPT_TURNS, ), include_partial_turns: bool | None = typer.Option( @@ -393,8 +396,12 @@ def body(state: AppState, json_mode: bool) -> None: output.print_code(code_gen.stream(merged, llm=gateway)) return - api_key = config.resolve_api_key(profile=state.profile) + # Validate the requested sources (including that a local file exists) before + # credentials, so a typo'd path reads as "file not found" — not as a login. validate_sources(opts, has_llm=bool(llm_prompt), text_mode=text_mode) + if opts.from_file and not opts.from_stdin: + client.resolve_audio_source(opts.source, sample=opts.sample) + api_key = config.resolve_api_key(profile=state.profile) llm_prompts = list(llm_prompt or []) session = StreamSession( diff --git a/aai_cli/commands/transcribe.py b/aai_cli/commands/transcribe.py index ae48c946..5b845947 100644 --- a/aai_cli/commands/transcribe.py +++ b/aai_cli/commands/transcribe.py @@ -392,7 +392,7 @@ def body(state: AppState, json_mode: bool) -> None: # authenticating (raw stdout, so `--show-code > script.py` runs). No # source/--sample needed — fall back to a placeholder path for a pure snippet. audio = ( - client.resolve_audio_source(source, sample=sample) + client.resolve_audio_source(source, sample=sample, check_local=False) if source or sample else "your-audio-file.mp3" ) @@ -402,6 +402,9 @@ def body(state: AppState, json_mode: bool) -> None: tc = config_builder.construct_transcription_config(merged) + # A typo'd path must read as "file not found", not trigger a login. + transcribe_exec.check_source_exists(source, sample=sample) + api_key = config.resolve_api_key(profile=state.profile) with output.status("Transcribing…", json_mode=json_mode): transcript = transcribe_exec.run_transcription( diff --git a/aai_cli/config.py b/aai_cli/config.py index ab76519f..7b9a2287 100644 --- a/aai_cli/config.py +++ b/aai_cli/config.py @@ -185,8 +185,21 @@ def set_api_key(profile: str, api_key: str) -> None: _dump(cfg) +def _keyring_get(username: str) -> str | None: + """Read a secret, treating an unusable keyring backend as "nothing stored". + + Headless machines (containers, CI, servers) routinely have no keyring backend at + all, so keyring raises NoKeyringError on every read. That state must read as "not + signed in" — ASSEMBLYAI_API_KEY still works there — never as a crash. + """ + try: + return keyring.get_password(KEYRING_SERVICE, username) + except keyring.errors.KeyringError: + return None + + def get_api_key(profile: str) -> str | None: - return keyring.get_password(KEYRING_SERVICE, profile) + return _keyring_get(profile) def get_profile_env(profile: str) -> str | None: @@ -204,7 +217,9 @@ def set_profile_env(profile: str, env: str) -> None: def clear_api_key(profile: str) -> None: - with contextlib.suppress(keyring.errors.PasswordDeleteError): + # KeyringError, not just PasswordDeleteError: with no backend at all (headless + # boxes) delete raises NoKeyringError, and "nothing stored" is already the goal. + with contextlib.suppress(keyring.errors.KeyringError): keyring.delete_password(KEYRING_SERVICE, profile) @@ -233,7 +248,7 @@ def set_session(profile: str, *, session_jwt: str, session_token: str, account_i def get_session(profile: str) -> dict[str, str] | None: """The stored {'jwt', 'token'} for a profile, or None if absent/corrupt.""" - raw = keyring.get_password(KEYRING_SERVICE, _session_username(profile)) + raw = _keyring_get(_session_username(profile)) if not raw: return None try: @@ -250,7 +265,7 @@ def get_account_id(profile: str) -> int | None: def clear_session(profile: str) -> None: - with contextlib.suppress(keyring.errors.PasswordDeleteError): + with contextlib.suppress(keyring.errors.KeyringError): keyring.delete_password(KEYRING_SERVICE, _session_username(profile)) cfg = _load() prof = cfg.profiles.get(profile) @@ -278,8 +293,8 @@ def persist_login( restored best-effort. """ _validate_profile(profile) - prior_api_key = keyring.get_password(KEYRING_SERVICE, profile) - prior_session = keyring.get_password(KEYRING_SERVICE, _session_username(profile)) + prior_api_key = _keyring_get(profile) + prior_session = _keyring_get(_session_username(profile)) prior_cfg = _load() done = False try: diff --git a/aai_cli/context.py b/aai_cli/context.py index b605c2b9..5dba568a 100644 --- a/aai_cli/context.py +++ b/aai_cli/context.py @@ -171,3 +171,20 @@ def run_command( except CLIError as err: output.emit_error(err, json_mode=json_mode) raise typer.Exit(code=err.exit_code) from None + except (typer.Exit, typer.Abort, BrokenPipeError): + # Deliberate control flow (and the closed-pipe contract handled in main.run); + # these must reach Click/the entry point untouched. + raise + except Exception as exc: + # Last resort: a bug must surface as one clean line (and the JSON error shape + # under --json), never as a raw traceback. + internal = CLIError( + f"Unexpected error: {exc}", + error_type="internal_error", + suggestion=( + "This looks like a bug in the CLI — please report it at " + "https://github.com/AssemblyAI/cli/issues." + ), + ) + output.emit_error(internal, json_mode=json_mode) + raise typer.Exit(code=internal.exit_code) from exc diff --git a/aai_cli/errors.py b/aai_cli/errors.py index ec458b53..f2a3964e 100644 --- a/aai_cli/errors.py +++ b/aai_cli/errors.py @@ -39,7 +39,8 @@ def __init__( message: str = "You're not signed in.", *, suggestion: str | None = ( - "Run 'aai onboard' for guided setup, or 'aai login' if you have an account." + "Run 'aai onboard' for guided setup, 'aai login' if you have an account, " + "or set ASSEMBLYAI_API_KEY." ), ) -> None: super().__init__( diff --git a/aai_cli/main.py b/aai_cli/main.py index de4d7537..a0ab6938 100644 --- a/aai_cli/main.py +++ b/aai_cli/main.py @@ -1,10 +1,16 @@ from __future__ import annotations import sys +from types import ModuleType from typing import TYPE_CHECKING import typer +from rich.console import RenderableType +from rich.style import StyleType +from rich.table import Table from typer import completion, rich_utils +from typer._click.exceptions import ClickException, NoSuchOption +from typer._click.utils import PacifyFlushWrapper from typer.core import TyperGroup if TYPE_CHECKING: @@ -111,6 +117,50 @@ def parse_args(self, ctx: ClickContext, args: list[str]) -> list[str]: rich_utils.STYLE_USAGE = theme.MUTED rich_utils.STYLE_USAGE_COMMAND = f"bold {theme.BRAND}" + +# Help tables put flag/command names in the leading columns and wrapping prose +# (metavar, help text) in the trailing two. Rich's width collapse only spares no_wrap +# columns, so on a narrow terminal it happily clips a flag name to "--end-of-turn-c…" — +# unlearnable from the help screen itself. Pin every column except the last two so the +# prose columns absorb the squeeze instead. +class _NoClipTable(Table): + def add_row( + self, + *renderables: RenderableType | None, + style: StyleType | None = None, + end_section: bool = False, + ) -> None: + super().add_row(*renderables, style=style, end_section=end_section) + for column in self.columns[:-2]: + column.no_wrap = True + + +def _patch_module(module: ModuleType, **attrs: object) -> None: + """Replace module attributes that are imports (not definitions) in their module — + strict mypy's no-implicit-reexport rejects plain attribute assignment for those.""" + for name, value in attrs.items(): + setattr(module, name, value) + + +# Typer's own help/error consoles must also honor the closed-pipe contract: with +# Rich's default Console, `aai --help | head -2` exits 1 via Console.on_broken_pipe. +_patch_module(rich_utils, Table=_NoClipTable, Console=theme.PipeSafeConsole) + +_format_click_error = rich_utils.rich_format_error + + +def _format_click_error_fixed(self: ClickException) -> None: + # Typer's vendored Click renders flag suggestions as a stringified 1-tuple: + # "No such option: --jsno ('(Possible options: --json)',)". Fold the suggestion + # into the message ourselves so the user sees "(Possible options: --json)". + if isinstance(self, NoSuchOption) and self.possibilities: + self.message = f"{self.message} (Possible options: {', '.join(sorted(self.possibilities))})" + self.possibilities = None + _format_click_error(self) + + +rich_utils.rich_format_error = _format_click_error_fixed + # Typer's built-in `--show-completion` help is long enough to wrap several lines in # the options panel. Trim it so it fits on fewer rows. The OptionInfo objects live on # the completion placeholder's parameter defaults; reach the (underscore-prefixed) @@ -204,7 +254,10 @@ def _offer_or_help(ctx: typer.Context, state: AppState) -> None: 'aai transcribe call.mp3 --llm "summarize action items"', ), ] - ), + ) + + "\n\n[bold]Authentication[/bold]\n\n" + "Run 'aai login', or set ASSEMBLYAI_API_KEY (used before the stored key). " + "--env or AAI_ENV selects the backend: production, sandbox000.", ) def main( ctx: typer.Context, @@ -288,3 +341,11 @@ def run() -> None: except BrokenPipeError: stdio.silence_stdout() sys.exit(0) + except SystemExit as exc: + # Typer's vendored Click handles EPIPE before our handler can see it: it wraps + # the streams in PacifyFlushWrapper and exits 1. That wrapper only ever appears + # on the closed-pipe path, so rewrite that exit to the documented success code. + if exc.code == 1 and isinstance(sys.stdout, PacifyFlushWrapper): + stdio.silence_stdout() + sys.exit(0) + raise diff --git a/aai_cli/theme.py b/aai_cli/theme.py index 00e223fe..b5a0aaba 100644 --- a/aai_cli/theme.py +++ b/aai_cli/theme.py @@ -84,9 +84,22 @@ _WARN = {"queued", "processing", "in_progress", "running"} +class PipeSafeConsole(Console): + """A Console honoring the CLI's closed-pipe contract. + + Rich's default ``on_broken_pipe`` redirects stdout to devnull and raises + ``SystemExit(1)``, so ``aai … | head`` would report failure. Re-raise the + ``BrokenPipeError`` instead so the entry point (``main.run``) can treat a closed + downstream pipe as success. + """ + + def on_broken_pipe(self) -> None: + raise BrokenPipeError() + + def make_console(file: IO[str] | None = None, **kwargs: Any) -> Console: """Build a Console with the AssemblyAI theme attached so `aai.*` names resolve.""" - return Console(file=file, theme=THEME, **kwargs) + return PipeSafeConsole(file=file, theme=THEME, **kwargs) def speaker_style(speaker: object) -> str: diff --git a/aai_cli/transcribe_exec.py b/aai_cli/transcribe_exec.py index b0367c50..5ab0a4e8 100644 --- a/aai_cli/transcribe_exec.py +++ b/aai_cli/transcribe_exec.py @@ -41,6 +41,15 @@ def out_payload( return client.select_transcript_field(transcript, choices.TranscriptOutput.text) +def check_source_exists(source: str | None, *, sample: bool) -> None: + """Resolve (and existence-check) the audio reference before credential resolution. + + Stdin (``-``) is exempt: its bytes are buffered at transcription time. + """ + if source != "-": + client.resolve_audio_source(source, sample=sample) + + def run_transcription( api_key: str, source: str | None, diff --git a/pyproject.toml b/pyproject.toml index e7047623..ca5308f8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -121,7 +121,11 @@ testpaths = ["tests"] # that bind a loopback server (OAuth callback, dev server, tunnel) opt back in with # `@pytest.mark.allow_hosts([...])`. The e2e/install suites drive the CLI as a # subprocess, whose sockets this in-process patch never sees, so they're unaffected. -addopts = "--disable-socket --allow-unix-socket" +# -m: a bare `pytest` runs the same suite check.sh gates — the e2e/install/ +# install_script marker suites are slow, need network/credentials, and fail +# confusingly in sandboxes otherwise. An explicit command-line -m (e.g. +# `pytest -m e2e`) overrides this one, so the opt-in runs keep working. +addopts = "--disable-socket --allow-unix-socket -m 'not e2e and not install and not install_script'" # Treat warnings as errors so a deprecation or resource leak introduced in a change # fails the build instead of scrolling past. Listed-later filters take precedence, so # the targeted ignores below override the blanket `error`. diff --git a/tests/__snapshots__/test_cli_output_snapshots.ambr b/tests/__snapshots__/test_cli_output_snapshots.ambr index 37f577f1..dfe6b85b 100644 --- a/tests/__snapshots__/test_cli_output_snapshots.ambr +++ b/tests/__snapshots__/test_cli_output_snapshots.ambr @@ -695,19 +695,25 @@ │ (repeatable). │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Turn Detection ─────────────────────────────────────────────────────────────╮ - │ --end-of-turn-co… FLOAT RANGE End-of-turn │ - │ [0.0<=x<=1.0] confidence (0-1). │ - │ --min-turn-silen… INTEGER Min silence to │ - │ end a turn (ms). │ - │ --max-turn-silen… INTEGER Max silence │ - │ before ending a │ - │ turn (ms). │ - │ --vad-threshold FLOAT RANGE Voice-activity │ - │ [0.0<=x<=1.0] threshold (0-1). │ - │ --format-turns --no-format-tur… Punctuate/format │ - │ finalized turns. │ - │ --include-partia… Emit partial │ - │ turns. │ + │ --end-of-turn-confidence FLOAT RANGE End-of-turn │ + │ [0.0<=x<=1.0 confidence │ + │ ] (0-1). │ + │ --min-turn-silence INTEGER Min silence │ + │ to end a turn │ + │ (ms). │ + │ --max-turn-silence INTEGER Max silence │ + │ before ending │ + │ a turn (ms). │ + │ --vad-threshold FLOAT RANGE Voice │ + │ [0.0<=x<=1.0 activity │ + │ ] threshold │ + │ (0-1). │ + │ --format-turns --no-format-turns Format │ + │ (punctuate) │ + │ finalized │ + │ turns. │ + │ --include-partial-turns Emit partial │ + │ turns. │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Speakers & Channels ────────────────────────────────────────────────────────╮ │ --speaker-labels Diarize speakers. With system │ @@ -717,11 +723,11 @@ │ --max-speakers INTEGER RANGE [x>=1] Max speakers. │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Features ───────────────────────────────────────────────────────────────────╮ - │ --voice-focus [near-field|far-field] Voice focus (noise │ - │ suppression model). │ - │ --voice-focus-thresho… FLOAT RANGE Voice-focus threshold │ - │ [0.0<=x<=1.0] (0-1). │ - │ --inactivity-timeout INTEGER Auto-close after N │ + │ --voice-focus [near-field|far-field Voice focus (noise │ + │ ] suppression model). │ + │ --voice-focus-threshold FLOAT RANGE Voice-focus threshold │ + │ [0.0<=x<=1.0] (0-1). │ + │ --inactivity-timeout INTEGER Auto-close after N │ │ seconds idle. │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Guardrails ─────────────────────────────────────────────────────────────────╮ @@ -843,22 +849,23 @@ │ channel separately. │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Guardrails ─────────────────────────────────────────────────────────────────╮ - │ --redact-pii Redact PII from the │ - │ transcript. │ - │ --redact-pii-policy TEXT Comma-separated PII │ - │ policies (e.g. │ - │ person_name,...). │ - │ --redact-pii-sub [hash|entity_name] How to replace │ - │ redacted PII. │ - │ --redact-pii-audio Also redact audio. │ - │ --filter-profanity Mask profanity. │ - │ --content-safety Detect sensitive │ - │ content. │ - │ --content-safety-conf… INTEGER RANGE Content-safety │ - │ [25<=x<=100] confidence threshold │ - │ (25-100). │ - │ --speech-threshold FLOAT RANGE Minimum proportion of │ - │ [0.0<=x<=1.0] speech required (0-1). │ + │ --redact-pii Redact PII from the │ + │ transcript. │ + │ --redact-pii-policy TEXT Comma-separated PII │ + │ policies (e.g. │ + │ person_name,...). │ + │ --redact-pii-sub [hash|entity_name] How to replace │ + │ redacted PII. │ + │ --redact-pii-audio Also redact audio. │ + │ --filter-profanity Mask profanity. │ + │ --content-safety Detect sensitive │ + │ content. │ + │ --content-safety-confidence INTEGER RANGE Content-safety │ + │ [25<=x<=100] confidence threshold │ + │ (25-100). │ + │ --speech-threshold FLOAT RANGE Minimum proportion │ + │ [0.0<=x<=1.0] of speech required │ + │ (0-1). │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Analysis ───────────────────────────────────────────────────────────────────╮ │ --summarization Summarize the │ @@ -1040,8 +1047,8 @@ # name: test_error_human_render_matches_snapshot[not_authenticated] ''' Error: You're not signed in. - Suggestion: Run 'aai onboard' for guided setup, or 'aai login' if you have an - account. + Suggestion: Run 'aai onboard' for guided setup, 'aai login' if you have an + account, or set ASSEMBLYAI_API_KEY. ''' # --- @@ -1073,7 +1080,7 @@ # --- # name: test_error_json_render_matches_snapshot[not_authenticated] ''' - {"error": {"type": "not_authenticated", "message": "You're not signed in.", "suggestion": "Run 'aai onboard' for guided setup, or 'aai login' if you have an account."}} + {"error": {"type": "not_authenticated", "message": "You're not signed in.", "suggestion": "Run 'aai onboard' for guided setup, 'aai login' if you have an account, or set ASSEMBLYAI_API_KEY."}} ''' # --- diff --git a/tests/test_client.py b/tests/test_client.py index 51f40a47..13d90311 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -91,17 +91,6 @@ def test_list_transcripts_rejected_key_becomes_not_authenticated(mocker): client.list_transcripts("sk_bad") -def test_resolve_audio_source_sample_explicit_and_missing(): - from aai_cli.errors import UsageError - - assert client.resolve_audio_source(None, sample=True) == client.SAMPLE_AUDIO_URL - assert client.resolve_audio_source("clip.mp3", sample=False) == "clip.mp3" - with pytest.raises(UsageError) as exc: - client.resolve_audio_source(None, sample=False) - assert exc.value.message == "Provide an audio path or URL." - assert "--sample" in (exc.value.suggestion or "") - - def test_transcribe_blocks_and_returns_transcript(mocker): fake_transcript = mocker.MagicMock() fake_transcript.status = client.aai.TranscriptStatus.completed diff --git a/tests/test_config.py b/tests/test_config.py index 2cf3ca01..ae52ba39 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -271,3 +271,43 @@ def test_load_returns_independent_copies(): hit = config._load() # cache hit: must also be a *deep* copy hit.profiles.clear() assert "default" in config._load().profiles + + +def test_get_api_key_treats_broken_keyring_backend_as_no_key(monkeypatch): + # Headless boxes (containers, CI) have no keyring backend at all; reads raise + # NoKeyringError there. That must read as "not signed in", never as a crash. + import keyring + import keyring.errors + + def no_backend(service, username): + raise keyring.errors.NoKeyringError("no recommended backend") + + monkeypatch.setattr(keyring, "get_password", no_backend) + assert config.get_api_key("default") is None + assert config.get_session("default") is None + with pytest.raises(NotAuthenticated): + config.resolve_api_key() + + +def test_resolve_api_key_env_var_works_without_keyring_backend(monkeypatch): + import keyring + import keyring.errors + + def no_backend(service, username): + raise keyring.errors.NoKeyringError("no recommended backend") + + monkeypatch.setattr(keyring, "get_password", no_backend) + monkeypatch.setenv("ASSEMBLYAI_API_KEY", "from_env") + assert config.resolve_api_key() == "from_env" + + +def test_clear_credentials_without_keyring_backend_is_silent(monkeypatch): + import keyring + import keyring.errors + + def no_backend(service, username): + raise keyring.errors.NoKeyringError("no recommended backend") + + monkeypatch.setattr(keyring, "delete_password", no_backend) + config.clear_api_key("default") + config.clear_session("default") diff --git a/tests/test_context.py b/tests/test_context.py index 00bb4bb4..84d39564 100644 --- a/tests/test_context.py +++ b/tests/test_context.py @@ -272,3 +272,44 @@ def test_appstate_methods_are_the_single_source_of_truth(): def test_appstate_environment_falls_back_to_default(): assert AppState().resolve_environment().name == environments.DEFAULT_ENV + + +def test_run_command_maps_unexpected_exception_to_clean_internal_error(): + # The last-resort handler: a bug surfaces as one clean line, never a traceback. + def body(state, json_mode): + raise ValueError("kaboom") + + result = runner.invoke(_make_app(body), ["go"]) + assert result.exit_code == 1 + assert "Unexpected error: kaboom" in result.output + assert "Traceback" not in result.output + assert "report it" in result.output + + +def test_run_command_unexpected_exception_keeps_json_error_shape(): + import json + + def body(state, json_mode): + raise ValueError("kaboom") + + result = runner.invoke(_make_app(body, json=True), ["go"]) + assert result.exit_code == 1 + assert json.loads(result.output)["error"]["type"] == "internal_error" + + +def test_run_command_lets_typer_exit_pass_through(): + def body(state, json_mode): + raise typer.Exit(code=7) + + result = runner.invoke(_make_app(body), ["go"]) + assert result.exit_code == 7 + assert "Unexpected error" not in result.output + + +def test_run_command_lets_broken_pipe_propagate(): + # The closed-pipe contract is owned by main.run(); run_command must not eat it. + def body(state, json_mode): + raise BrokenPipeError() + + result = runner.invoke(_make_app(body), ["go"], standalone_mode=False) + assert isinstance(result.exception, BrokenPipeError) diff --git a/tests/test_errors.py b/tests/test_errors.py index 80bf8d45..4d2da228 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -6,9 +6,9 @@ def test_not_authenticated_defaults(): assert err.exit_code == 4 assert err.error_type == "not_authenticated" assert err.message == "You're not signed in." - assert ( - err.suggestion - == "Run 'aai onboard' for guided setup, or 'aai login' if you have an account." + assert err.suggestion == ( + "Run 'aai onboard' for guided setup, 'aai login' if you have an account, " + "or set ASSEMBLYAI_API_KEY." ) diff --git a/tests/test_help_rendering.py b/tests/test_help_rendering.py new file mode 100644 index 00000000..eef39823 --- /dev/null +++ b/tests/test_help_rendering.py @@ -0,0 +1,78 @@ +"""Rendering guards for the help screens and Click error formatting. + +These pin the two patches main.py applies to Typer's rich rendering: flag-name +columns must never be clipped to an unreadable "--end-of-turn-c…" at a standard +80-column terminal, and unknown-flag suggestions must not leak a tuple repr. +""" + +import re + +import pytest +from typer.testing import CliRunner + +from aai_cli.main import app +from tests._cli_tree import leaf_command_argvs + +runner = CliRunner() + +# Matches SGR (color/style) ANSI escape sequences. CI runners force color on (Rich +# enables it under GITHUB_ACTIONS), which interleaves style codes mid-message — +# e.g. the option highlighter styles "--zzqq" inside "No such option: --zzqq" — +# so every text assertion here runs on the color-free render. +_ANSI_SGR = re.compile(r"\x1b\[[0-9;]*m") + +# A long-option token cut off by Rich's ellipsis overflow, e.g. "--end-of-turn-c…". +_CLIPPED_FLAG = re.compile(r"--[\w-]*…") + + +def _plain(text: str) -> str: + return _ANSI_SGR.sub("", text) + + +@pytest.mark.parametrize("argv", leaf_command_argvs(), ids=" ".join) +def test_no_flag_name_is_clipped_at_80_columns(argv): + result = runner.invoke(app, [*argv, "--help"], env={"COLUMNS": "80"}) + assert result.exit_code == 0 + plain = _plain(result.output) + assert not _CLIPPED_FLAG.search(plain), _CLIPPED_FLAG.search(plain) + + +def test_root_help_documents_authentication(): + # The shared "how do I authenticate / pick a backend" line: the env vars must be + # discoverable from the CLI itself, not only from external docs. + result = runner.invoke(app, ["--help"]) + assert result.exit_code == 0 + plain = _plain(result.output) + assert "ASSEMBLYAI_API_KEY" in plain + assert "AAI_ENV" in plain + + +def test_unknown_flag_suggestion_renders_clean(): + # Vendored Click formats this as a stringified 1-tuple ("('(Possible options: + # --json)',)"); main.py folds the suggestion into the message instead. + result = runner.invoke(app, ["transcribe", "x.wav", "--jsno"]) + assert result.exit_code == 2 + plain = _plain(result.output) + assert "(Possible options: --json)" in plain + assert "',)" not in plain + + +def test_unknown_flag_without_suggestion_renders_plain(): + result = runner.invoke(app, ["transcribe", "x.wav", "--zzqq"]) + assert result.exit_code == 2 + plain = _plain(result.output) + assert "No such option: --zzqq" in plain + assert "Possible options" not in plain + + +def test_noclip_table_pins_leading_columns_and_passes_row_args_through(): + from aai_cli.main import _NoClipTable + + table = _NoClipTable() + table.add_row("--flag", "META", "help text") + # Only the trailing two (prose) columns may wrap/shrink. + assert [c.no_wrap for c in table.columns] == [True, False, False] + # add_row keyword defaults must match rich's (end_section=False unless asked). + assert table.rows[-1].end_section is False + table.add_row("--other", "META", "more", end_section=True) + assert table.rows[-1].end_section is True diff --git a/tests/test_login.py b/tests/test_login.py index a0069b8a..2a86c68b 100644 --- a/tests/test_login.py +++ b/tests/test_login.py @@ -289,3 +289,15 @@ def test_whoami_renders_human_table_rejected_key(mocker): assert result.exit_code == 0 assert "key rejected" in result.output assert "none" in result.output + + +def test_whoami_honors_env_api_key(monkeypatch, mocker): + # A CI box authenticated only via ASSEMBLYAI_API_KEY (no keyring entry) must be + # able to use whoami as a preflight check. + monkeypatch.setenv("ASSEMBLYAI_API_KEY", "sk_env_1234567890") + mocker.patch("aai_cli.commands.login.client.validate_key", autospec=True, return_value=True) + result = runner.invoke(app, ["whoami", "--json"]) + assert result.exit_code == 0 + data = json.loads(result.output) + assert data["reachable"] is True + assert "sk_env_1234567890" not in result.output # still masked diff --git a/tests/test_main_module.py b/tests/test_main_module.py index f316fcc1..ef56bbdf 100644 --- a/tests/test_main_module.py +++ b/tests/test_main_module.py @@ -69,3 +69,31 @@ def test_python_dash_m_version(): ) assert result.returncode == 0 assert result.stdout.strip() # prints something (the version) + + +def test_run_converts_click_epipe_exit_to_success(monkeypatch): + """Typer's vendored Click swallows EPIPE itself (PacifyFlushWrapper + exit 1); + run() must still honor the closed-pipe-is-success contract.""" + from typer._click.utils import PacifyFlushWrapper + + def epipe_path(*a, **k): + monkeypatch.setattr(sys, "stdout", PacifyFlushWrapper(sys.stdout)) + raise SystemExit(1) + + monkeypatch.setattr(main_mod, "app", epipe_path) + monkeypatch.setattr("aai_cli.stdio.silence_stdout", lambda: None) + with pytest.raises(SystemExit) as exc: + main_mod.run() + assert exc.value.code == 0 + + +def test_run_keeps_exit_1_when_stdout_is_not_pacified(monkeypatch): + """A real failure exit (code 1, untouched stdout) must never be rewritten.""" + + def failure(*a, **k): + raise SystemExit(1) + + monkeypatch.setattr(main_mod, "app", failure) + with pytest.raises(SystemExit) as exc: + main_mod.run() + assert exc.value.code == 1 diff --git a/tests/test_source_validation.py b/tests/test_source_validation.py new file mode 100644 index 00000000..83016826 --- /dev/null +++ b/tests/test_source_validation.py @@ -0,0 +1,116 @@ +"""Input validation that must run before credentials or any request. + +A typo'd local path has to read as "file not found" (never trigger a login or an +upload attempt), and a transcript "id" is interpolated into the API path so anything +non-token-shaped is rejected before an authenticated GET can be steered elsewhere. +""" + +import pytest +from typer.testing import CliRunner + +from aai_cli import client, config +from aai_cli.errors import CLIError, UsageError +from aai_cli.main import app + +runner = CliRunner() + + +def test_resolve_audio_source_sample_explicit_and_missing(tmp_path): + assert client.resolve_audio_source(None, sample=True) == client.SAMPLE_AUDIO_URL + clip = tmp_path / "clip.mp3" + clip.write_bytes(b"fake") + assert client.resolve_audio_source(str(clip), sample=False) == str(clip) + with pytest.raises(UsageError) as exc: + client.resolve_audio_source(None, sample=False) + assert exc.value.message == "Provide an audio path or URL." + assert "--sample" in (exc.value.suggestion or "") + + +def test_resolve_audio_source_missing_local_file_fails_cleanly(): + with pytest.raises(CLIError) as exc: + client.resolve_audio_source("no-such-clip.mp3", sample=False) + assert exc.value.error_type == "file_not_found" + assert exc.value.exit_code == 2 + assert exc.value.message == "File not found: no-such-clip.mp3" + + +def test_resolve_audio_source_skips_existence_check_for_urls_and_show_code(): + # URLs are not local paths; --show-code legitimately generates code for a file + # the user does not have yet. + url = "https://example.com/a.mp3" + assert client.resolve_audio_source(url, sample=False) == url + assert ( + client.resolve_audio_source("future.mp3", sample=False, check_local=False) == "future.mp3" + ) + + +def test_validate_transcript_id_rejects_path_segments(): + assert client.validate_transcript_id("t_42-abc") == "t_42-abc" + for bad in ("../../etc/passwd", "", "a/b", "id?x=1"): + with pytest.raises(UsageError): + client.validate_transcript_id(bad) + + +def test_get_transcript_validates_id_before_request(mocker): + get_by_id = mocker.patch.object(client.aai.Transcript, "get_by_id", autospec=True) + with pytest.raises(UsageError): + client.get_transcript("sk", "../../etc/passwd") + get_by_id.assert_not_called() + + +def test_transcripts_get_rejects_path_traversal_id(): + config.set_api_key("default", "sk_live") + result = runner.invoke(app, ["transcripts", "get", "../../etc/passwd"]) + assert result.exit_code == 2 + assert "doesn't look like a transcript id" in result.output + + +def test_transcribe_missing_file_fails_before_credentials(mocker): + # No key is configured: the path check must fire first, so the user sees + # "file not found" instead of a login prompt (or a keyring error). + tx = mocker.patch("aai_cli.commands.transcribe.client.transcribe", autospec=True) + result = runner.invoke(app, ["transcribe", "missing.wav"]) + assert result.exit_code == 2 + assert "File not found: missing.wav" in result.output + assert "starting browser login" not in result.output + tx.assert_not_called() + + +def test_transcribe_requires_source(): + config.set_api_key("default", "sk_live") + result = runner.invoke(app, ["transcribe"]) + assert result.exit_code == 2 + + +def test_transcribe_empty_stdin_exits_2(): + config.set_api_key("default", "sk_live") + result = runner.invoke(app, ["transcribe", "-"], input=b"") + assert result.exit_code == 2 # nothing piped -> usage error + + +def test_stream_missing_file_fails_before_credentials(monkeypatch): + called = {"stream": False} + monkeypatch.setattr( + "aai_cli.commands.stream.client.stream_audio", + lambda *a, **k: called.__setitem__("stream", True), + ) + result = runner.invoke(app, ["stream", "missing.wav"]) + assert result.exit_code == 2 + assert "File not found: missing.wav" in result.output + assert "starting browser login" not in result.output + assert called["stream"] is False + + +def test_agent_missing_file_fails_before_credentials(): + result = runner.invoke(app, ["agent", "missing.wav"]) + assert result.exit_code == 2 + assert "File not found: missing.wav" in result.output + assert "starting browser login" not in result.output + + +def test_show_code_does_not_require_local_file_to_exist(): + # Generating code for audio you don't have yet is legitimate (check_local=False). + result = runner.invoke(app, ["transcribe", "missing.wav", "--show-code"]) + assert result.exit_code == 0 + assert "missing.wav" in result.output + assert "import assemblyai" in result.output diff --git a/tests/test_theme.py b/tests/test_theme.py index d300b49b..a3819262 100644 --- a/tests/test_theme.py +++ b/tests/test_theme.py @@ -70,3 +70,21 @@ def test_output_console_is_themed_and_error_is_styled(monkeypatch): assert "Error:" in out assert "boom" in out assert "\x1b[" in out # themed error emits ANSI on a forced-color console + + +def test_pipe_safe_console_reraises_broken_pipe(): + # Rich's default on_broken_pipe converts EPIPE to SystemExit(1); the CLI's + # consoles must re-raise so main.run() can treat a closed pipe as success. + import io + + import pytest + + from aai_cli import theme + + class BrokenFile(io.StringIO): + def write(self, s): + raise BrokenPipeError + + console = theme.make_console(file=BrokenFile()) + with pytest.raises(BrokenPipeError): + console.print("hello") diff --git a/tests/test_transcribe.py b/tests/test_transcribe.py index 0f5aeae8..86cb80ff 100644 --- a/tests/test_transcribe.py +++ b/tests/test_transcribe.py @@ -1,5 +1,6 @@ import json +import pytest from typer.testing import CliRunner from aai_cli import config @@ -9,6 +10,14 @@ runner = CliRunner() +@pytest.fixture(autouse=True) +def audio_file(tmp_path, monkeypatch): + # The command checks the local path exists before resolving credentials, so the + # "audio.mp3" the tests pass must be a real file; run each test in its own cwd. + monkeypatch.chdir(tmp_path) + (tmp_path / "audio.mp3").write_bytes(b"fake-audio") + + def _auth(): config.set_api_key("default", "sk_live") @@ -57,12 +66,6 @@ def test_transcribe_sample_prints_text(mocker): assert audio_arg.endswith("wildfires.mp3") -def test_transcribe_requires_source(): - _auth() - result = runner.invoke(app, ["transcribe"]) - assert result.exit_code == 2 - - def test_transcribe_passes_speaker_labels(mocker): _auth() tx = mocker.patch( @@ -163,12 +166,6 @@ def fake_transcribe(api_key, audio, *, config): assert seen["bytes"] == b"RIFFfake-wav-bytes" -def test_transcribe_empty_stdin_exits_2(): - _auth() - result = runner.invoke(app, ["transcribe", "-"], input=b"") - assert result.exit_code == 2 # nothing piped -> usage error - - def test_transcribe_status_renders_enum_value(mocker): import assemblyai as aai diff --git a/tests/test_transcribe_out.py b/tests/test_transcribe_out.py index 45582411..2f170117 100644 --- a/tests/test_transcribe_out.py +++ b/tests/test_transcribe_out.py @@ -1,6 +1,7 @@ import json from unittest.mock import MagicMock, patch +import pytest from typer.testing import CliRunner from aai_cli import config @@ -8,6 +9,15 @@ runner = CliRunner() + +@pytest.fixture(autouse=True) +def audio_file(tmp_path, monkeypatch): + # The command checks the local path exists before resolving credentials, so the + # "audio.mp3" the tests pass must be a real file; run each test in its own cwd. + monkeypatch.chdir(tmp_path) + (tmp_path / "audio.mp3").write_bytes(b"fake-audio") + + _TRANSCRIBE = "aai_cli.commands.transcribe.client.transcribe"