diff --git a/.gitignore b/.gitignore index 2865af01..7e1cf203 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,8 @@ htmlcov/ # Editor/agent local artifacts: keep personal settings local, but track the # team-shared bits (.claude/settings.json, agents/, skills/). .claude/settings.local.json +# Temporary git worktrees created for isolated subagents +.claude/worktrees/ # Local scratch scripts (often contain live keys) transcribe/ diff --git a/aai_cli/agent/session.py b/aai_cli/agent/session.py index 27de5de2..e55125f4 100644 --- a/aai_cli/agent/session.py +++ b/aai_cli/agent/session.py @@ -11,6 +11,7 @@ from aai_cli import environments from aai_cli import ws as wsutil from aai_cli.errors import APIError, CLIError, NotAuthenticated +from aai_cli.streaming import diagnostics def ws_url() -> str: @@ -205,11 +206,19 @@ def _send_audio_loop(ws: _WebSocket, session: VoiceAgentSession, mic: _IO) -> No def _open_ws(connect: _Connect, api_key: str) -> _WebSocket: - """Open the Voice Agent socket, mapping a connect failure to a clean CLIError.""" + """Open the Voice Agent socket, mapping a connect failure to a clean CLIError. + + A rejected handshake (HTTP 401/403) gets the shared actionable suggestion + (whoami / environment / network); anything else keeps the wsutil mapping. + """ + message = "Could not connect to the voice agent" try: return connect(ws_url(), additional_headers={"Authorization": f"Bearer {api_key}"}) except Exception as exc: - raise wsutil.auth_or_api_error(exc, "Could not connect to the voice agent") from exc + rejected = diagnostics.handshake_error(exc, message, host=environments.active().agents_host) + if rejected is not None: + raise rejected from exc + raise wsutil.auth_or_api_error(exc, message) from exc def _session_update_message(config: AgentRunConfig) -> str: diff --git a/aai_cli/agent/voices.py b/aai_cli/agent/voices.py index 955bb21a..91aa1690 100644 --- a/aai_cli/agent/voices.py +++ b/aai_cli/agent/voices.py @@ -1,54 +1,74 @@ from __future__ import annotations +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Voice: + """A known Voice Agent voice id and the language group it belongs to.""" + + name: str + language: str + + +ENGLISH = "English" +MULTILINGUAL = "Multilingual" + # Known Voice Agent voice IDs (from the Voice Agent quickstart). The server is # the source of truth; this list backs --list-voices and catches obvious typos. -VOICES: list[str] = [ - # English - "ivy", - "james", - "tyler", - "winter", - "sam", - "mia", - "bella", - "david", - "jack", - "kyle", - "helen", - "martha", - "river", - "emma", - "victor", - "eleanor", - "sophie", - "oliver", - # Multilingual - "arjun", - "ethan", - "dmitri", - "lukas", - "lena", - "pierre", - "mina", - "ren", - "mei", - "joon", - "giulia", - "luca", - "lucia", - "hana", - "mateo", - "diego", +VOICES: list[Voice] = [ + Voice("ivy", ENGLISH), + Voice("james", ENGLISH), + Voice("tyler", ENGLISH), + Voice("winter", ENGLISH), + Voice("sam", ENGLISH), + Voice("mia", ENGLISH), + Voice("bella", ENGLISH), + Voice("david", ENGLISH), + Voice("jack", ENGLISH), + Voice("kyle", ENGLISH), + Voice("helen", ENGLISH), + Voice("martha", ENGLISH), + Voice("river", ENGLISH), + Voice("emma", ENGLISH), + Voice("victor", ENGLISH), + Voice("eleanor", ENGLISH), + Voice("sophie", ENGLISH), + Voice("oliver", ENGLISH), + Voice("arjun", MULTILINGUAL), + Voice("ethan", MULTILINGUAL), + Voice("dmitri", MULTILINGUAL), + Voice("lukas", MULTILINGUAL), + Voice("lena", MULTILINGUAL), + Voice("pierre", MULTILINGUAL), + Voice("mina", MULTILINGUAL), + Voice("ren", MULTILINGUAL), + Voice("mei", MULTILINGUAL), + Voice("joon", MULTILINGUAL), + Voice("giulia", MULTILINGUAL), + Voice("luca", MULTILINGUAL), + Voice("lucia", MULTILINGUAL), + Voice("hana", MULTILINGUAL), + Voice("mateo", MULTILINGUAL), + Voice("diego", MULTILINGUAL), ] +# The plain ids, for membership checks and completion. +VOICE_NAMES: list[str] = [voice.name for voice in VOICES] + DEFAULT_VOICE = "ivy" def format_voice_list() -> str: - """Human-readable, newline-separated voice IDs for --list-voices.""" - return "\n".join(VOICES) + """Human-readable voice IDs for --list-voices, grouped by language.""" + groups = dict.fromkeys(voice.language for voice in VOICES) + blocks: list[str] = [] + for language in groups: + names = "\n".join(f" {voice.name}" for voice in VOICES if voice.language == language) + blocks.append(f"{language}:\n{names}") + return "\n\n".join(blocks) def complete_voice(incomplete: str) -> list[str]: """Shell-completion callback for ``--voice``: known voice ids matching the prefix.""" - return [v for v in VOICES if v.startswith(incomplete)] + return [name for name in VOICE_NAMES if name.startswith(incomplete)] diff --git a/aai_cli/auth/flow.py b/aai_cli/auth/flow.py index 792517ad..02235bad 100644 --- a/aai_cli/auth/flow.py +++ b/aai_cli/auth/flow.py @@ -1,5 +1,7 @@ from __future__ import annotations +import json +import sys import webbrowser from dataclasses import dataclass @@ -84,16 +86,42 @@ def _parse[T](adapter: TypeAdapter[T], data: object) -> T: ) from exc -def _open_browser(url: str) -> None: +def _note(*, json_mode: bool, human: str, hint: str, url: str | None = None) -> None: + """One stderr progress note for the login flow. + + Humans get Rich prose; ``--json`` mode gets one ``{"hint": …}`` object per note, + keeping stderr machine-readable (the same contract as ``output.emit_warning``). + """ + if json_mode: + payload: dict[str, object] = {"hint": hint} + if url is not None: + payload["url"] = url + sys.stderr.write(json.dumps(payload) + "\n") + else: + output.error_console.print(human) + + +def _open_browser(url: str, *, json_mode: bool) -> None: """Open the system browser, falling back to printing the URL.""" - output.error_console.print( - f"Opening your browser to sign in:\n [aai.url]{escape(url)}[/aai.url]" + _note( + json_mode=json_mode, + human=f"Opening your browser to sign in:\n [aai.url]{escape(url)}[/aai.url]", + hint="Opening your browser to sign in.", + url=url, ) try: - webbrowser.open(url) + opened = webbrowser.open(url) except Exception: # noqa: BLE001 - opening a browser is best-effort - output.error_console.print( - "[aai.muted]Could not open a browser; open the URL above manually.[/aai.muted]" + opened = False + # webbrowser.open returns False — without raising — on headless boxes with no + # usable browser, so the fallback must fire on the boolean too; otherwise the + # user sits out the 120s timeout with no hint that nothing opened. + if not opened: + _note( + json_mode=json_mode, + human="[aai.muted]Could not open a browser; open the URL above manually.[/aai.muted]", + hint="Could not open a browser; open the URL manually.", + url=url, ) @@ -135,16 +163,23 @@ def find_or_create_cli_key(account_id: int, session_jwt: str) -> str: return _parse(_CREATED_TOKEN, created).api_key -def run_login_flow() -> LoginResult: +def run_login_flow(*, json_mode: bool = False) -> LoginResult: """Drive the full browser + AMS login and return a LoginResult.""" # Bind the loopback callback server *before* opening the browser: if the port is # taken, fail cleanly now instead of stranding the user mid-OAuth in a flow that # can never call back. capture = _start_capture() - _open_browser(discovery.build_start_url()) - output.error_console.print( - "[aai.muted]Waiting up to 2 minutes for you to finish signing in…[/aai.muted]\n" - "[aai.muted]No browser here? Run 'assembly login --api-key ' instead.[/aai.muted]" + _open_browser(discovery.build_start_url(), json_mode=json_mode) + _note( + json_mode=json_mode, + human=( + "[aai.muted]Waiting up to 2 minutes for you to finish signing in…[/aai.muted]\n" + "[aai.muted]No browser here? Run 'assembly login --api-key ' instead.[/aai.muted]" + ), + hint=( + "Waiting up to 2 minutes for you to finish signing in. " + "No browser here? Run 'assembly login --api-key ' instead." + ), ) result = capture.wait() @@ -167,10 +202,11 @@ def run_login_flow() -> LoginResult: ) org = disc.organizations[0] if len(disc.organizations) > 1: - output.error_console.print( - f"[aai.muted]Found {len(disc.organizations)} organizations; signing in to " - f"'{org.organization_name or org.organization_id}'.[/aai.muted]" + chosen = ( + f"Found {len(disc.organizations)} organizations; signing in to " + f"'{org.organization_name or org.organization_id}'." ) + _note(json_mode=json_mode, human=f"[aai.muted]{chosen}[/aai.muted]", hint=chosen) # `exchange` already returns the signed-in account, so read the id from it # rather than making a second GET /v1/auth round-trip. diff --git a/aai_cli/client.py b/aai_cli/client.py index dab743d7..48086ef5 100644 --- a/aai_cli/client.py +++ b/aai_cli/client.py @@ -17,6 +17,7 @@ from aai_cli import environments, jsonshape, stdio from aai_cli.errors import APIError, CLIError, UsageError, auth_failure, is_auth_failure +from aai_cli.streaming.diagnostics import handshake_error, silence_streaming_logging SAMPLE_AUDIO_URL = "https://assembly.ai/wildfires.mp3" _StreamHandler = Callable[[Any, Any], object] @@ -104,7 +105,14 @@ def _sdk_errors(message: str) -> Generator[None]: except Exception as exc: if is_auth_failure(exc): raise auth_failure() from exc - raise APIError(f"{message}: {exc}") from exc + # Compact the reason (httpx-backed SDK errors embed a multi-line + # `Request: <…>` repr) and drop the SDK's own "failed to …" preamble when + # `message` already says the same thing, so the error reads once, cleanly. + reason = _SDK_PREAMBLE_RE.sub("", _compact_reason(exc)) + raise APIError( + f"{message}: {reason}", + suggestion="Check your network and try again.", + ) from exc def _list_transcript_params(limit: int) -> aai.ListTranscriptParameters: @@ -124,6 +132,13 @@ def _list_transcript_params(limit: int) -> aai.ListTranscriptParameters: # httpx-backed SDK errors embed a multi-line repr ("…\nReason: …\nRequest: "). _REQUEST_REPR_RE = re.compile(r"Request: <[^>]*>") +# The SDK prefixes transcript-history errors with its own "failed to retrieve +# transcript(s) …:" preamble, doubling up with the wrapper's message ("Could not +# list transcripts: failed to retrieve transcripts: …"). Strip just that known +# preamble; transcript ids are url-safe tokens, so the optional id segment can +# never swallow a colon-bearing reason (e.g. a URL). +_SDK_PREAMBLE_RE = re.compile(r"^failed to retrieve transcripts?(?: [A-Za-z0-9_-]+)?: ") + def _compact_reason(exc: object) -> str: """``str(exc)`` as a single clean line: drop the trailing ``Request: <…>`` repr and @@ -246,6 +261,20 @@ def get_transcript(api_key: str, transcript_id: str) -> aai.Transcript: return aai.Transcript.get_by_id(transcript_id) +def _streaming_run_error(error: object) -> CLIError: + """Classify a recorded streaming Error event into the CLIError to raise. + + A rejected handshake (HTTP 401/403) gets an actionable suggestion: bare + "Streaming error: WebSocket handshake rejected (HTTP 403)" left users cold. + """ + rejected = handshake_error(error, "Streaming error", host=environments.active().streaming_host) + if rejected is not None: + return rejected + if is_auth_failure(error): + return auth_failure() + return APIError(f"Streaming error: {error}") + + def stream_audio( api_key: str, source: Iterable[bytes], @@ -260,6 +289,9 @@ def stream_audio( Forwards Begin/Turn/Termination events to the callbacks; raises APIError on a stream error. `params` is a fully-built StreamingParameters (sample_rate/speech_model/etc). """ + # The SDK's reader thread and websockets both log connection failures at ERROR; + # with no logging configured those lines would duplicate the CLIError on stderr. + silence_streaming_logging() sc = _make_streaming_client(api_key) def _guard(cb: Callable[[Any], Any]) -> _StreamHandler: @@ -299,6 +331,4 @@ def _record_error(_client: object, error: object) -> None: sc.disconnect(terminate=True) if errors: - if is_auth_failure(errors[0]): - raise auth_failure() - raise APIError(f"Streaming error: {errors[0]}") + raise _streaming_run_error(errors[0]) diff --git a/aai_cli/commands/account.py b/aai_cli/commands/account.py index 8e8e7722..17e7573b 100644 --- a/aai_cli/commands/account.py +++ b/aai_cli/commands/account.py @@ -14,18 +14,26 @@ from aai_cli.help_text import examples_epilog -def _utc_day_start(day: str) -> str: - """Render a ``YYYY-MM-DD`` date as a tz-aware UTC ISO-8601 timestamp. +def _parse_day(day: str) -> date: + try: + return date.fromisoformat(day) + except ValueError as exc: + raise UsageError(f"Invalid date {day!r}; expected YYYY-MM-DD.") from exc + + +def _utc_day_start(day: date) -> str: + """Render a date as a tz-aware UTC ISO-8601 timestamp. The AMS billing endpoint compares the bounds against tz-aware datetimes and rejects naive ones ("can't compare offset-naive and offset-aware datetimes"), so the wire value always carries an explicit ``+00:00`` offset. """ - try: - parsed = date.fromisoformat(day) - except ValueError as exc: - raise UsageError(f"Invalid date {day!r}; expected YYYY-MM-DD.") from exc - return datetime(parsed.year, parsed.month, parsed.day, tzinfo=UTC).isoformat() + return datetime(day.year, day.month, day.day, tzinfo=UTC).isoformat() + + +# The AMS usage endpoint's recognized window sizes; anything else is silently +# misinterpreted server-side, so reject it client-side as a usage error. +_USAGE_WINDOWS = ("day", "week", "month") def _format_usage_number(value: object) -> str: @@ -149,7 +157,9 @@ def usage( None, "--start", help="Start date (YYYY-MM-DD). Default: 30d ago." ), end: str | None = typer.Option(None, "--end", help="End date (YYYY-MM-DD). Default: today."), - window: str | None = typer.Option(None, "--window", help="Window size, e.g. 'day' or 'month'."), + window: str | None = typer.Option( + None, "--window", help="Window size: 'day', 'week', or 'month'." + ), include_zero: bool = typer.Option( False, "--include-zero", @@ -161,11 +171,23 @@ def usage( """Show usage over a date range (defaults to the last 30 days).""" def body(state: AppState, json_mode: bool) -> None: - # Parse/validate the date flags before any session resolution or network - # work, so a bad --start/--end is a fast usage error even when not logged in. + # Parse/validate the flags before any session resolution or network work, + # so a bad --start/--end/--window is a fast usage error even when not logged in. today = datetime.now(UTC).date() - start_date = _utc_day_start(start or (today - timedelta(days=30)).isoformat()) - end_date = _utc_day_start(end or today.isoformat()) + start_day = _parse_day(start) if start else today - timedelta(days=30) + end_day = _parse_day(end) if end else today + if end_day < start_day: + raise UsageError( + f"--end {end_day.isoformat()} is before --start {start_day.isoformat()}.", + suggestion="Pick an end date on or after the start date.", + ) + if window is not None and window not in _USAGE_WINDOWS: + raise UsageError( + f"Invalid --window {window!r}.", + suggestion=f"Use one of: {', '.join(_USAGE_WINDOWS)}.", + ) + start_date = _utc_day_start(start_day) + end_date = _utc_day_start(end_day) _, jwt = resolve_session(state) data = ams.get_usage(jwt, start_date, end_date, window) diff --git a/aai_cli/commands/agent.py b/aai_cli/commands/agent.py index ece92e95..8cddb027 100644 --- a/aai_cli/commands/agent.py +++ b/aai_cli/commands/agent.py @@ -15,7 +15,13 @@ AgentRunConfig, run_session, ) -from aai_cli.agent.voices import DEFAULT_VOICE, VOICES, complete_voice, format_voice_list +from aai_cli.agent.voices import ( + DEFAULT_VOICE, + VOICE_NAMES, + VOICES, + complete_voice, + format_voice_list, +) from aai_cli.context import AppState, run_command from aai_cli.errors import CLIError, UsageError from aai_cli.help_text import examples_epilog @@ -69,7 +75,8 @@ def _open_audio( def _emit_voice_list(_state: AppState, json_mode: bool) -> None: """--list-voices body, routed through run_command so --json yields a machine-readable array instead of the human list; needs no auth.""" - output.emit(VOICES, lambda _voices: format_voice_list(), json_mode=json_mode) + payload = [{"name": voice.name, "language": voice.language} for voice in VOICES] + output.emit(payload, lambda _voices: format_voice_list(), json_mode=json_mode) @app.command( @@ -129,12 +136,13 @@ def agent( ) -> None: """Have a live two-way voice conversation with an AssemblyAI voice agent. - Use headphones: the mic stays open while the agent speaks, so on speakers it would - hear itself and loop. Pass an audio file/URL (or --sample) to speak a recorded clip to - the agent instead of the microphone; the session then ends after the agent's reply. + Use headphones: the mic stays open while the agent speaks, so on + speakers it would hear itself and loop. Pass an audio file/URL (or + --sample) to speak a recorded clip to the agent instead of the + microphone; the session then ends after the agent's reply. - This only runs a conversation in the terminal — it writes no code. To build - a voice agent app, run 'assembly init voice-agent' instead. + This only runs a conversation in the terminal — it writes no code. To + build a voice agent app, run 'assembly init voice-agent' instead. """ if list_voices: @@ -144,7 +152,7 @@ def agent( def body(state: AppState, json_mode: bool) -> None: validate_output_flags(json_mode=json_mode, output_field=output_field) text_mode, json_mode = output.stream_output_modes(output_field, json_mode=json_mode) - if voice not in VOICES: + if voice not in VOICE_NAMES: raise UsageError( f"Unknown voice {voice!r}.", suggestion="Run 'assembly agent --list-voices' to see the options.", diff --git a/aai_cli/commands/audit.py b/aai_cli/commands/audit.py index b99c6367..6d3674c1 100644 --- a/aai_cli/commands/audit.py +++ b/aai_cli/commands/audit.py @@ -87,7 +87,7 @@ def _audit_rows(payload: Mapping[str, object]) -> list[dict[str, object]]: ) def audit( ctx: typer.Context, - limit: int = typer.Option(20, "--limit", help="How many entries to show."), + limit: int = typer.Option(20, "--limit", min=1, help="How many entries to show."), action: str | None = typer.Option(None, "--action", help="Filter by raw action name."), resource: str | None = typer.Option(None, "--resource", help="Filter by raw resource type."), include_logins: bool = typer.Option( diff --git a/aai_cli/commands/deploy.py b/aai_cli/commands/deploy.py index c2388442..b7dade95 100644 --- a/aai_cli/commands/deploy.py +++ b/aai_cli/commands/deploy.py @@ -9,7 +9,7 @@ import typer -from aai_cli import help_panels, output +from aai_cli import help_panels, options, output from aai_cli.context import AppState, run_command from aai_cli.errors import CLIError, UsageError from aai_cli.help_text import examples_epilog @@ -105,7 +105,7 @@ def _confirmed(target: Target, *, assume_yes: bool) -> bool: return typer.confirm(f"Deploy this project to {target.name}?") -def run_deploy(*, target: Target, prod: bool, assume_yes: bool) -> None: +def run_deploy(*, target: Target, prod: bool, assume_yes: bool, json_mode: bool) -> None: """Confirm, then run the target's deploy command in the current directory.""" if prod and not target.supports_prod: raise UsageError( @@ -117,7 +117,8 @@ def run_deploy(*, target: Target, prod: bool, assume_yes: bool) -> None: procfile.require_procfile(Path.cwd()) _require_cli(target) if not _confirmed(target, assume_yes=assume_yes): - output.console.print("Aborted.") + aborted = {"status": "aborted", "target": target.name} + output.emit(aborted, lambda _d: "Aborted.", json_mode=json_mode) return result = subprocess.run(target.command(prod=prod), cwd=Path.cwd(), check=False) if result.returncode: @@ -144,6 +145,7 @@ def deploy( railway: bool = typer.Option(False, "--railway", help="Deploy to Railway."), fly: bool = typer.Option(False, "--fly", help="Deploy to Fly.io."), assume_yes: bool = typer.Option(False, "--yes", "-y", help="Skip the confirmation prompt."), + json_out: bool = options.json_option(), ) -> None: """Deploy the current project to Vercel (default), Railway, or Fly.io. @@ -152,8 +154,10 @@ def deploy( (Render deploys from a connected Git repo — see the project README.) """ - def body(_state: AppState, _json_mode: bool) -> None: + def body(_state: AppState, json_mode: bool) -> None: selected = [t for t, on in ((VERCEL, vercel), (RAILWAY, railway), (FLY, fly)) if on] - run_deploy(target=_resolve_target(selected), prod=prod, assume_yes=assume_yes) + run_deploy( + target=_resolve_target(selected), prod=prod, assume_yes=assume_yes, json_mode=json_mode + ) - run_command(ctx, body) + run_command(ctx, body, json=json_out) diff --git a/aai_cli/commands/dev.py b/aai_cli/commands/dev.py index f1b5c941..e6ba43f6 100644 --- a/aai_cli/commands/dev.py +++ b/aai_cli/commands/dev.py @@ -17,12 +17,15 @@ app = typer.Typer() -def run_dev(*, port: int, host: str, no_install: bool, no_open: bool, json_mode: bool) -> None: +def run_dev( + *, port: int, host: str, no_install: bool, no_open: bool, json_mode: bool, quiet: bool +) -> None: """Boot the project's Procfile `web:` process locally, with live reload.""" target = Path.cwd() use_uv = runner.has_uv() chosen_port = runner.find_free_port(port) + devserver.notify_port_change(port, chosen_port, json_mode=json_mode, quiet=quiet) env = {**os.environ, "PORT": str(chosen_port)} # Resolves the start command AND validates we're inside a scaffolded project. web = procfile.web_argv(target, env=env) @@ -82,7 +85,14 @@ def dev( if needed, then starts the FastAPI server with live reload and opens the browser. """ - def body(_state: AppState, json_mode: bool) -> None: - run_dev(port=port, host=host, no_install=no_install, no_open=no_open, json_mode=json_mode) + def body(state: AppState, json_mode: bool) -> None: + run_dev( + port=port, + host=host, + no_install=no_install, + no_open=no_open, + json_mode=json_mode, + quiet=state.quiet, + ) run_command(ctx, body, json=json_out) diff --git a/aai_cli/commands/doctor.py b/aai_cli/commands/doctor.py index 6629f4d4..9c621d6a 100644 --- a/aai_cli/commands/doctor.py +++ b/aai_cli/commands/doctor.py @@ -111,10 +111,12 @@ def _check_api_key(profile: str) -> Check: ) if valid: return _check("api-key", "ok", "API key is valid and AssemblyAI is reachable.") + # validate_key collapses every auth-shaped failure (401, 403, proxy "forbidden") + # to False, so don't claim a specific status code we never saw. return _check( "api-key", "fail", - "API key was rejected (HTTP 401/403).", + "API key was rejected by the server.", fix="Run 'assembly login' with a valid key.", affects=["everything"], ) diff --git a/aai_cli/commands/evaluate.py b/aai_cli/commands/evaluate.py index 54f7c263..0c96c821 100644 --- a/aai_cli/commands/evaluate.py +++ b/aai_cli/commands/evaluate.py @@ -18,6 +18,7 @@ from aai_cli import client, config, der, eval_data, help_panels, jsonshape, options, output, wer from aai_cli.context import AppState, run_command +from aai_cli.errors import CLIError, NotAuthenticated, UsageError from aai_cli.help_text import examples_epilog app = typer.Typer() @@ -56,6 +57,11 @@ class _ItemResult: speakers: der.DerScore | None +def _failed_result(item: eval_data.EvalItem, err: CLIError) -> _ItemResult: + """A row whose transcription failed: the error rides along, no scores pooled.""" + return _ItemResult(row={"item": item.item_id, "error": err.message}, words=None, speakers=None) + + def _score_item( item: eval_data.EvalItem, transcript: aai.Transcript, *, collar: float ) -> _ItemResult: @@ -71,6 +77,63 @@ def _score_item( return _ItemResult(row=row, words=words, speakers=speakers) +def _pooled_metrics(results: list[_ItemResult]) -> dict[str, object]: + """The summary scores pooled over the scored rows (failed rows carry none).""" + metrics: dict[str, object] = {} + word_scores = [result.words for result in results if result.words is not None] + if word_scores: + total = wer.pooled(word_scores) + metrics.update({"words": total.words, "errors": total.errors, "wer": total.wer}) + der_scores = [result.speakers for result in results if result.speakers is not None] + if der_scores: + pooled = der.pooled(der_scores) + metrics["der"] = pooled.der + metrics["der_breakdown"] = { + "missed": pooled.missed / pooled.total, + "false_alarm": pooled.false_alarm / pooled.total, + "confusion": pooled.confusion / pooled.total, + } + return metrics + + +def _transcribe_one( + api_key: str, item: eval_data.EvalItem, config: aai.TranscriptionConfig +) -> aai.Transcript | CLIError: + """One item's outcome: its transcript, or the CLIError it failed with. + + A bad item must not discard the other (paid) items, so per-item failures + are recorded rather than raised — except ``NotAuthenticated`` (one rejected + key fails every row identically) and non-CLIError bugs, which propagate and + abort the run. + """ + try: + return client.transcribe(api_key, item.audio, config=config) + except NotAuthenticated: + raise + except CLIError as err: + return err + + +def _concurrent_transcripts( + api_key: str, + items: list[eval_data.EvalItem], + *, + transcription_config: aai.TranscriptionConfig, + concurrency: int, +) -> list[aai.Transcript | CLIError]: + with ThreadPoolExecutor(max_workers=concurrency) as pool: + futures = [ + pool.submit(_transcribe_one, api_key, item, transcription_config) for item in items + ] + for future in as_completed(futures): + if (exc := future.exception()) is not None: + # Only aborting failures escape _transcribe_one: drop the + # not-yet-started items rather than burn an API call on each. + pool.shutdown(cancel_futures=True) + raise exc + return [future.result() for future in futures] + + def _transcripts( api_key: str, items: list[eval_data.EvalItem], @@ -79,42 +142,31 @@ def _transcripts( concurrency: int, json_mode: bool, quiet: bool, -) -> list[aai.Transcript]: - """Each item's transcript, in dataset order. +) -> list[aai.Transcript | CLIError]: + """Each item's transcript — or the CLIError it failed with — in dataset order. Sequential by default, with a per-item spinner; ``--concurrency`` fans the - API calls out across a thread pool (the transcribe-batch pattern: the first - worker error drops the not-yet-started items and re-raises). + API calls out across a thread pool (see ``_transcribe_one`` for which + failures are per-item outcomes and which abort the run). """ if concurrency == 1: - transcripts: list[aai.Transcript] = [] + outcomes: list[aai.Transcript | CLIError] = [] for index, item in enumerate(items, start=1): with output.status( f"[{index}/{len(items)}] Transcribing {item.item_id}…", json_mode=json_mode, quiet=quiet, ): - transcripts.append( - client.transcribe(api_key, item.audio, config=transcription_config) - ) - return transcripts - with ( - output.status( - f"Transcribing {len(items)} items (concurrency {concurrency})…", - json_mode=json_mode, - quiet=quiet, - ), - ThreadPoolExecutor(max_workers=concurrency) as pool, + outcomes.append(_transcribe_one(api_key, item, transcription_config)) + return outcomes + with output.status( + f"Transcribing {len(items)} items (concurrency {concurrency})…", + json_mode=json_mode, + quiet=quiet, ): - futures = [ - pool.submit(client.transcribe, api_key, item.audio, config=transcription_config) - for item in items - ] - for future in as_completed(futures): - if (exc := future.exception()) is not None: - pool.shutdown(cancel_futures=True) - raise exc - return [future.result() for future in futures] + return _concurrent_transcripts( + api_key, items, transcription_config=transcription_config, concurrency=concurrency + ) def _payload( @@ -126,19 +178,10 @@ def _payload( "items": len(results), "rows": [result.row for result in results], } - word_scores = [result.words for result in results if result.words is not None] - if word_scores: - total = wer.pooled(word_scores) - payload.update({"words": total.words, "errors": total.errors, "wer": total.wer}) - der_scores = [result.speakers for result in results if result.speakers is not None] - if der_scores: - pooled = der.pooled(der_scores) - payload["der"] = pooled.der - payload["der_breakdown"] = { - "missed": pooled.missed / pooled.total, - "false_alarm": pooled.false_alarm / pooled.total, - "confusion": pooled.confusion / pooled.total, - } + payload.update(_pooled_metrics(results)) + failed = sum(1 for result in results if "error" in result.row) + if failed: + payload["failed"] = failed return payload @@ -160,21 +203,34 @@ def _summary(payload: dict[str, object]) -> str: return output.heading(" ".join(parts)) +def _cell(row: dict[str, object], key: str) -> str: + """The row's value as table text — blank when absent (e.g. a failed row's scores).""" + return str(row[key]) if key in row else "" + + +def _pct_cell(row: dict[str, object], key: str) -> str: + return _pct(row[key]) if key in row else "" + + def _render(payload: dict[str, object]) -> RenderableType: has_wer = "wer" in payload has_der = "der" in payload + has_failed = "failed" in payload columns = [ "ITEM", *(["WORDS", "ERRORS", "WER"] if has_wer else []), *(["DER"] if has_der else []), + *(["ERROR"] if has_failed else []), ] table = output.data_table(*columns) for row in jsonshape.mapping_list(payload.get("rows")): cells = [str(row.get("item"))] if has_wer: - cells += [str(row.get("words")), str(row.get("errors")), _pct(row.get("wer"))] + cells += [_cell(row, "words"), _cell(row, "errors"), _pct_cell(row, "wer")] if has_der: - cells.append(_pct(row.get("der"))) + cells.append(_pct_cell(row, "der")) + if has_failed: + cells.append(_cell(row, "error")) table.add_row(*cells) model = payload.get("speech_model") or "default model" return output.stack( @@ -244,11 +300,12 @@ def evaluate( "--speaker-labels", help="Diarize and also score DER against the dataset's reference speaker turns (speakers/timestamps_start/timestamps_end columns, in seconds).", ), - collar: float = typer.Option( - 1.0, + collar: float | None = typer.Option( + None, "--collar", min=0.0, - help="DER forgiveness (seconds) around each reference turn boundary.", + help="DER forgiveness (seconds) around each reference turn boundary " + "(default: 1.0; needs --speaker-labels).", ), concurrency: int = typer.Option( 1, @@ -281,6 +338,14 @@ def evaluate( """ def body(state: AppState, json_mode: bool) -> None: + if collar is not None and not speaker_labels: + raise UsageError( + "--collar only applies when diarization is being scored.", + suggestion="Add --speaker-labels.", + ) + # Resolve credentials before any dataset download: a signed-out user must + # not pull the whole dataset only to fail at the first transcription. + api_key = config.resolve_api_key(profile=state.profile) data = eval_data.load( dataset, split=split, @@ -290,13 +355,12 @@ def body(state: AppState, json_mode: bool) -> None: limit=limit, with_speakers=speaker_labels, ) - api_key = config.resolve_api_key(profile=state.profile) transcription_config = aai.TranscriptionConfig( speech_models=[speech_model.value] if speech_model else None, language_code=language_code, speaker_labels=speaker_labels or None, ) - transcripts = _transcripts( + outcomes = _transcripts( api_key, data.items, transcription_config=transcription_config, @@ -305,13 +369,23 @@ def body(state: AppState, json_mode: bool) -> None: quiet=state.quiet, ) results = [ - _score_item(item, transcript, collar=collar) - for item, transcript in zip( + _failed_result(item, outcome) + if isinstance(outcome, CLIError) + else _score_item(item, outcome, collar=collar if collar is not None else 1.0) + for item, outcome in zip( data.items, - transcripts, - strict=True, # pragma: no mutate (defensive invariant; _transcripts returns one transcript per item) + outcomes, + strict=True, # pragma: no mutate (defensive invariant; _transcripts returns one outcome per item) ) ] - output.emit(_payload(data.label, speech_model, results), _render, json_mode=json_mode) + payload = _payload(data.label, speech_model, results) + output.emit(payload, _render, json_mode=json_mode) + failed = jsonshape.as_int(payload.get("failed")) + if failed: + raise CLIError( + f"{failed} of {len(results)} items failed to transcribe.", + error_type="eval_failed", + suggestion="The summary covers only the items that transcribed.", + ) run_command(ctx, body, json=json_out) diff --git a/aai_cli/commands/init.py b/aai_cli/commands/init.py index 6d262e6e..e58b7ed1 100644 --- a/aai_cli/commands/init.py +++ b/aai_cli/commands/init.py @@ -9,10 +9,12 @@ from aai_cli import __version__, environments, help_panels, options, output, steps from aai_cli.context import AppState, run_command -from aai_cli.errors import CLIError +from aai_cli.errors import CLIError, UsageError from aai_cli.help_text import examples_epilog from aai_cli.init import keys, runner, scaffold, templates +_DEFAULT_PORT = 3000 + # Single-command sub-typer flattened to `assembly init` (the exact pattern `assembly transcribe` # uses): one @app.command() named `init`, registered via app.add_typer(init.app) with # no name. Bare `assembly init` runs the command with template=None -> the interactive picker. @@ -118,8 +120,11 @@ def _install_step( ], will_launch -def _resolve_target(directory: str | None, chosen: str, *, here: bool, force: bool) -> Path: - """Resolve the target directory and reject --here+DIRECTORY or a non-empty conflict.""" +def _resolve_target( + directory: str | None, chosen: str, *, here: bool, force: bool +) -> tuple[Path, bool]: + """Resolve the target directory, rejecting --here+DIRECTORY, an existing file, or + a non-empty conflict. Returns the target and whether --force is overlaying it.""" if here and directory: raise CLIError( "Pass either a DIRECTORY or --here, not both.", @@ -127,29 +132,51 @@ def _resolve_target(directory: str | None, chosen: str, *, here: bool, force: bo exit_code=1, ) target = _resolve_dir(directory, chosen, here=here) - if scaffold.target_conflict(target) and not force: + if target.exists() and not target.is_dir(): + raise UsageError(f"{target} exists and is not a directory.") + conflict = scaffold.target_conflict(target) + if conflict and not force: raise CLIError( f"{target} already exists and is not empty. " f"Use --force to overwrite or pick another directory.", error_type="usage_error", exit_code=1, ) - return target + return target, conflict -def _scaffold_report(chosen: str, target: Path, api_key: str | None) -> list[steps.Step]: +def _key_row(api_key: str | None, key_source: str | None, preserved: str | None) -> steps.Step: + """The report's `key` row — emitted symmetrically whether a key resolved or not.""" + if api_key is not None: + return {"name": "key", "status": "written", "detail": f"from {key_source}"} + if preserved is not None: + return {"name": "key", "status": "kept", "detail": "existing .env key preserved"} + return { + "name": "key", + "status": "skipped", + "detail": "no API key found; wrote a placeholder to .env (run `assembly login`)", + } + + +def _scaffold_report( + chosen: str, + target: Path, + *, + api_key: str | None, + key_source: str | None, + preserved: str | None, +) -> list[steps.Step]: """Write the template to `target` and return the opening report rows.""" - scaffold.scaffold(chosen, target, api_key=api_key, env_vars=_active_env_vars()) - report: list[steps.Step] = [{"name": "scaffold", "status": "created", "detail": str(target)}] - if api_key is None: - report.append( - { - "name": "key", - "status": "skipped", - "detail": "no API key found; wrote a placeholder to .env (run `assembly login`)", - } - ) - return report + scaffold.scaffold(chosen, target, api_key=api_key or preserved, env_vars=_active_env_vars()) + return [ + {"name": "scaffold", "status": "created", "detail": str(target)}, + _key_row(api_key, key_source, preserved), + ] + + +def _dev_hint(port: int) -> str: + """The `assembly dev` invocation matching the chosen port (the default needs no flag).""" + return "assembly dev" if port == _DEFAULT_PORT else f"assembly dev --port {port}" def launch_app(target: Path, *, port: int, use_uv: bool, no_open: bool, json_mode: bool) -> None: @@ -170,6 +197,37 @@ def launch_app(target: Path, *, port: int, use_uv: bool, no_open: bool, json_mod raise typer.Exit(code=code) +def _build_report( + state: AppState, chosen: str, target: Path, *, no_install: bool, use_uv: bool, port: int +) -> tuple[list[steps.Step], bool]: + """Scaffold and assemble the report rows; returns them plus whether to launch.""" + api_key, key_source = keys.resolve_optional_api_key(profile=state.profile) + # A configured (non-placeholder) .env key must survive a re-scaffold when no key + # resolves — otherwise --force would silently reset it to the placeholder. + preserved = scaffold.existing_env_key(target) if api_key is None else None + effective_key = api_key or preserved + report = _scaffold_report( + chosen, target, api_key=api_key, key_source=key_source, preserved=preserved + ) + + install_rows, will_launch = _install_step( + target, no_install=no_install, api_key=effective_key, use_uv=use_uv + ) + report.extend(install_rows) + + # Deps are installed but there's no key, so the server can't start — say so + # rather than exiting silently. + if not no_install and effective_key is None: + report.append( + { + "name": "launch", + "status": "skipped", + "detail": f"no API key; run `assembly login`, then: cd {target} && {_dev_hint(port)}", + } + ) + return report, will_launch + + def run_init( state: AppState, *, @@ -189,34 +247,27 @@ def run_init( running dev server mid-flow — it stops after install and leaves the run command as a hint (the wizard calls `launch_app` itself once its remaining sections are done). """ + chosen = _resolve_template(template) + target, overwriting = _resolve_target(directory, chosen, here=here, force=force) if not json_mode: - # Vercel-style banner at the top of the run. Decoration goes to stderr (data → - # stdout): it must never pollute a piped stdout, even on an error path. + # Vercel-style banner, printed only once validation passes so pure error runs + # (unknown template, conflicting target) stay undecorated like the sibling + # commands. Decoration goes to stderr (data → stdout): it must never pollute + # a piped stdout. output.error_console.print( f"[aai.heading]AssemblyAI CLI[/aai.heading] [aai.muted]{__version__}[/aai.muted]" ) - chosen = _resolve_template(template) - target = _resolve_target(directory, chosen, here=here, force=force) - - api_key = keys.resolve_optional_api_key(profile=state.profile) - report = _scaffold_report(chosen, target, api_key) + if overwriting: + output.emit_warning( + f"--force: overwriting existing files in {target} " + "(the template is overlaid; files not in the template are kept).", + json_mode=json_mode, + ) use_uv = runner.has_uv() - install_rows, will_launch = _install_step( - target, no_install=no_install, api_key=api_key, use_uv=use_uv + report, will_launch = _build_report( + state, chosen, target, no_install=no_install, use_uv=use_uv, port=port ) - report.extend(install_rows) - - # Deps are installed but there's no key, so the server can't start — say so - # rather than exiting silently. - if not no_install and api_key is None: - report.append( - { - "name": "launch", - "status": "skipped", - "detail": f"no API key; run `assembly login`, then: cd {target} && assembly dev", - } - ) output.emit(report, lambda d: steps.render_steps(d, heading="Setup"), json_mode=json_mode) if any(s["status"] == "failed" for s in report): @@ -227,7 +278,7 @@ def run_init( elif not json_mode: # Scaffolded but not launched (no key, or --no-install, or launch=False): leave the # user with the one command that starts their app, the way `vercel`/`supabase` sign off. - output.console.print(output.hint(f"Run `cd {escape(str(target))} && assembly dev`.")) + output.console.print(output.hint(f"Run `cd {escape(str(target))} && {_dev_hint(port)}`.")) return target @@ -267,9 +318,16 @@ def init( no_open: bool = typer.Option( False, "--no-open", help="Install + launch, but don't open the browser." ), - force: bool = typer.Option(False, "--force", help="Overwrite a non-empty target directory."), + force: bool = typer.Option( + False, + "--force", + help=( + "Overwrite a non-empty target directory (overlays the template; " + "files not in the template are kept)." + ), + ), here: bool = typer.Option(False, "--here", help="Scaffold into the current directory."), - port: int = typer.Option(3000, "--port", help="Local server port."), + port: int = typer.Option(_DEFAULT_PORT, "--port", help="Local server port."), json_out: bool = options.json_option(), ) -> None: """Scaffold a new project from a template, then launch it. diff --git a/aai_cli/commands/keys.py b/aai_cli/commands/keys.py index 676c5c14..053997e3 100644 --- a/aai_cli/commands/keys.py +++ b/aai_cli/commands/keys.py @@ -6,7 +6,7 @@ from aai_cli import jsonshape, options, output from aai_cli.auth import ams from aai_cli.context import AppState, resolve_session, run_command -from aai_cli.errors import APIError +from aai_cli.errors import APIError, UsageError from aai_cli.help_text import examples_epilog app = typer.Typer(help="List, create, and rename your AssemblyAI API keys.", no_args_is_help=True) @@ -119,6 +119,13 @@ def create( """Create a new API key. Prints the key value once — copy it now.""" def body(state: AppState, json_mode: bool) -> None: + # Validate locally before any auth/network work: an empty or whitespace-only + # label would otherwise cost a session resolution plus an AMS round-trip. + if not name.strip(): + raise UsageError( + "--name must not be empty.", + suggestion="Pass a label for the key, e.g. --name ci-pipeline.", + ) account_id, jwt = resolve_session(state) pid = project_id if project_id is not None else _default_project_id(account_id, jwt) created = ams.create_token(account_id, pid, name, jwt) diff --git a/aai_cli/commands/llm.py b/aai_cli/commands/llm.py index c9ab75d4..944db6b9 100644 --- a/aai_cli/commands/llm.py +++ b/aai_cli/commands/llm.py @@ -1,9 +1,11 @@ from __future__ import annotations +from collections.abc import Callable + import typer from rich.markup import escape -from aai_cli import choices, config, help_panels, options, output, stdio +from aai_cli import choices, client, config, help_panels, options, output, stdio from aai_cli import llm as gateway from aai_cli.context import AppState, run_command from aai_cli.errors import UsageError @@ -48,6 +50,44 @@ def _emit_model_list(_state: AppState, json_mode: bool) -> None: output.emit(list(gateway.KNOWN_MODELS), "\n".join, json_mode=json_mode) +def _list_models_body( + output_field: choices.TextOrJson | None, +) -> Callable[[AppState, bool], None]: + """The --list-models command body: rejects -o (it only applies to one-shot + mode, mirroring how --follow rejects it) before printing the known models.""" + + def body(state: AppState, json_mode: bool) -> None: + if output_field is not None: + raise UsageError( + "--output applies to one-shot mode; --list-models prints the plain " + "list (use --json for a machine-readable array)." + ) + _emit_model_list(state, json_mode) + + return body + + +def _stdin_transcript_text( + state: AppState, json_mode: bool, transcript_id: str | None +) -> str | None: + """Resolve the inline transcript text for one-shot mode. + + Text piped on stdin becomes the content the prompt operates on, unless an + explicit --transcript-id is given — that injects server-side and takes + priority, so piped text is ignored with a visible warning (suppressed by + --quiet, structured under --json). + """ + if transcript_id is None: + return stdio.piped_stdin_text() + # Same cheap local id check as `transcripts get`, before auth or network. + client.validate_transcript_id(transcript_id) + if stdio.stdin_is_piped() and not state.quiet: + output.emit_warning( + "Ignoring piped stdin; --transcript-id takes priority.", json_mode=json_mode + ) + return None + + @app.command( rich_help_panel=help_panels.TRANSCRIPTION, epilog=examples_epilog( @@ -94,7 +134,7 @@ def llm( help="Print one field of the result: text (just the answer, pipe-friendly) or json.", ), max_tokens: int = typer.Option( - gateway.DEFAULT_MAX_TOKENS, "--max-tokens", help="Max tokens to generate." + gateway.DEFAULT_MAX_TOKENS, "--max-tokens", help="Max tokens to generate.", min=1 ), list_models: bool = typer.Option(False, "--list-models", help="Print known models and exit."), json_out: bool = options.json_option("Output raw JSON (one object per turn in --follow mode)."), @@ -106,7 +146,7 @@ def llm( """ if list_models: - run_command(ctx, _emit_model_list, json=json_out) + run_command(ctx, _list_models_body(output_field), json=json_out) return def follow_body(state: AppState, json_mode: bool) -> None: @@ -144,10 +184,8 @@ def body(state: AppState, json_mode: bool) -> None: suggestion="Or pass --list-models to see available models.", ) prompt_text = prompt + stdin_text = _stdin_transcript_text(state, json_mode, transcript_id) api_key = config.resolve_api_key(profile=state.profile) - # Text piped on stdin becomes the content the prompt operates on, unless an - # explicit --transcript-id is given (that injects server-side and takes priority). - stdin_text = stdio.piped_stdin_text() if not transcript_id else None messages = gateway.build_messages( prompt_text, system=system, transcript_id=transcript_id, transcript_text=stdin_text ) diff --git a/aai_cli/commands/login.py b/aai_cli/commands/login.py index d8a8e318..edc72f80 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, UsageError +from aai_cli.errors import APIError, CLIError, UsageError from aai_cli.help_text import examples_epilog app = typer.Typer() @@ -31,9 +31,7 @@ def login( def body(state: AppState, json_mode: bool) -> None: profile = resolve_profile(state) env = environments.active().name - if api_key is None: - persist_browser_login(profile, env) - elif not api_key.strip(): + if api_key is not None and not api_key.strip(): # An explicitly-passed empty/whitespace key (e.g. --api-key "$UNSET_VAR") # must fail loudly, not silently fall into the browser flow as if the # flag had never been passed. @@ -44,6 +42,21 @@ def body(state: AppState, json_mode: bool) -> None: "(check that the shell variable you expanded is set)." ), ) + # Both login paths persist to the OS keyring, so probe it before any + # browser/network work: completing the whole OAuth dance only to fail on + # the final keyring write is the worst place to discover a headless box. + if not config.keyring_usable(): + raise CLIError( + "Your OS keyring isn't usable, so login can't store credentials.", + error_type="keyring_unusable", + exit_code=2, + suggestion=( + "Set ASSEMBLYAI_API_KEY instead (login can't store a key without " + "a keyring), or unlock/install an OS keyring and retry." + ), + ) + if api_key is None: + persist_browser_login(profile, env, json_mode=json_mode) else: # Non-interactive escape hatch for CI/automation: no AMS session is # obtained, so account self-service commands won't work for this profile. @@ -106,17 +119,26 @@ def logout( def body(state: AppState, json_mode: bool) -> None: profile = resolve_profile(state) + # Look before clearing so the report is truthful: "Signed out" on a fresh + # machine (or a typo'd --profile) would claim something happened when + # nothing was stored. Still exit 0 either way — logout is idempotent. + had_key = config.get_api_key(profile) is not None + had_session = config.get_session(profile) is not None + cleared = had_key or had_session config.clear_api_key(profile) config.clear_session(profile) - output.emit( - {"logged_out": True, "profile": profile}, - lambda _d: ( - output.success(f"Signed out of {escape(profile)}.") - + "\n" - + output.hint("Run `assembly login` to sign back in.") - ), - json_mode=json_mode, - ) + + def render(_d: dict[str, object]) -> object: + if cleared: + return ( + output.success(f"Signed out of {escape(profile)}.") + + "\n" + + output.hint("Run `assembly login` to sign back in.") + ) + return output.muted(f"No stored credentials for '{profile}' — nothing to clear.") + + data: dict[str, object] = {"logged_out": True, "profile": profile, "cleared": cleared} + output.emit(data, render, json_mode=json_mode) # auto_login=False: signing out while signed out must not start a sign-in. run_command(ctx, body, json=json_out, auto_login=False) @@ -143,19 +165,32 @@ def body(state: AppState, json_mode: bool) -> None: key = config.resolve_api_key(profile=state.profile) masked = output.mask_secret(key) env = environments.active().name - reachable = client.validate_key(key) + # A network failure must not suppress the local table: profile, env, masked + # key, and session are all known offline. reachable=None marks "couldn't + # check" — distinct from False, which means the server rejected the key. + network_error: APIError | None = None + reachable: bool | None + try: + reachable = client.validate_key(key) + except APIError as exc: + reachable = None + network_error = exc session_label = "stored" if config.get_session(profile) else "none" account_id = config.get_account_id(profile) + def status_cell() -> str: + if reachable: + return output.success("reachable") + if reachable is None: + return output.warn("unreachable (network error)") + return output.fail("key rejected") + def render(_d: dict[str, object]) -> Table: table = output.detail_table() table.add_row("Profile", escape(profile)) table.add_row("Env", escape(env)) table.add_row("API key", escape(masked)) - table.add_row( - "Status", - output.success("reachable") if reachable else output.fail("key rejected"), - ) + table.add_row("Status", status_cell()) table.add_row("Account", escape(str(account_id)) if account_id else "—") table.add_row("Session", escape(session_label)) return table @@ -169,6 +204,17 @@ def render(_d: dict[str, object]) -> Table: "session": session_label, } output.emit(data, render, json_mode=json_mode) + if network_error is not None: + # Couldn't validate ≠ rejected: surface the network failure with its own + # suggestion and exit code 1 (api_error), keeping the auth exit 4 reserved + # for a key the server actually refused. + raise APIError( + network_error.message, + suggestion=( + "Check your network connection and retry — the key was not rejected; " + "it just couldn't be validated." + ), + ) if not reachable: # A rejected key must fail the command (exit 4, the auth code used by # NotAuthenticated) so CI can use whoami as a preflight check; the diff --git a/aai_cli/commands/onboard.py b/aai_cli/commands/onboard.py index 39158305..340769a6 100644 --- a/aai_cli/commands/onboard.py +++ b/aai_cli/commands/onboard.py @@ -6,6 +6,7 @@ from aai_cli import help_panels, options, output from aai_cli.context import AppState, resolve_profile, run_command +from aai_cli.errors import CLIError from aai_cli.help_text import examples_epilog from aai_cli.onboard import wizard from aai_cli.onboard.prompter import InteractivePrompter, NonInteractivePrompter, Prompter @@ -46,9 +47,19 @@ def onboard( def body(state: AppState, json_mode: bool) -> None: profile = resolve_profile(state) wiz_ctx = WizardContext(state=state, profile=profile, json_mode=json_mode) - forced = non_interactive or output.is_agentic() + # --json also forces non-interactive: a machine-output run can't block on + # prompts, and the interactive prompter would write prose onto the JSON stdout. + forced = non_interactive or output.is_agentic() or json_mode code = wizard.run_onboarding(build_prompter(non_interactive=forced), wiz_ctx) if code != 0: + if json_mode: + # The standard {"error": …} envelope on stderr; the wizard already + # emitted its JSON section summary on stdout. + raise CLIError( + "Onboarding did not complete.", + error_type="onboarding_incomplete", + exit_code=code, + ) raise typer.Exit(code=code) # auto_login=False: the wizard owns the sign-in step itself. diff --git a/aai_cli/commands/sessions.py b/aai_cli/commands/sessions.py index b432f28d..266211f2 100644 --- a/aai_cli/commands/sessions.py +++ b/aai_cli/commands/sessions.py @@ -1,5 +1,7 @@ from __future__ import annotations +import enum + import typer from rich.markup import escape from rich.table import Table @@ -30,6 +32,16 @@ def _session_rows(value: object) -> list[dict[str, object]]: return jsonshape.mapping_list(value) +class SessionStatus(enum.StrEnum): + """Closed value set for ``sessions list --status`` (the API's lifecycle states), + so a typo is rejected with Typer's choices error instead of silently filtering + nothing (mirrors the ``choices.TranscriptOutput`` pattern).""" + + created = "created" + completed = "completed" + error = "error" + + @app.command( name="list", epilog=examples_epilog( @@ -49,9 +61,9 @@ def _session_rows(value: object) -> list[dict[str, object]]: ) def list_( ctx: typer.Context, - limit: int = typer.Option(10, "--limit", help="How many sessions to show."), - status: str | None = typer.Option( - None, "--status", help="Filter: created, completed, or error." + limit: int = typer.Option(10, "--limit", help="How many sessions to show.", min=1), + status: SessionStatus | None = typer.Option( + None, "--status", help="Only show sessions with this status." ), json_out: bool = options.json_option(), ) -> None: @@ -59,7 +71,9 @@ def list_( def body(state: AppState, json_mode: bool) -> None: _, jwt = resolve_session(state) - payload = ams.list_streaming(jwt, limit=limit, status=status) + payload = ams.list_streaming( + jwt, limit=limit, status=None if status is None else status.value + ) rows = _session_rows(payload.get("data")) def render(data: list[dict[str, object]]) -> object: diff --git a/aai_cli/commands/share.py b/aai_cli/commands/share.py index 33a86119..5e151c35 100644 --- a/aai_cli/commands/share.py +++ b/aai_cli/commands/share.py @@ -58,12 +58,13 @@ def _terminate(proc: subprocess.Popen[str] | None) -> None: proc.terminate() -def run_share(*, port: int, no_install: bool, json_mode: bool) -> None: +def run_share(*, port: int, no_install: bool, json_mode: bool, quiet: bool) -> None: """Boot the app and expose it on a public cloudflared quick-tunnel URL.""" target = Path.cwd() use_uv = runner.has_uv() chosen_port = runner.find_free_port(port) + devserver.notify_port_change(port, chosen_port, json_mode=json_mode, quiet=quiet) env = {**os.environ, "PORT": str(chosen_port)} web = procfile.web_argv(target, env=env) # validates we're in a scaffolded project _require_cloudflared() @@ -77,6 +78,8 @@ def run_share(*, port: int, no_install: bool, json_mode: bool) -> None: server = runner.spawn(devserver.dev_command(target, web, use_uv=use_uv), cwd=target, env=env) proxy: subprocess.Popen[str] | None = None + log_path: Path | None = None + keep_log = False try: if not runner.wait_for_port(chosen_port): raise CLIError( @@ -95,10 +98,14 @@ def run_share(*, port: int, no_install: bool, json_mode: bool) -> None: ) public = tunnel.await_url(log_path) if public is None: + # Keep the captured cloudflared output: it's the only evidence of why + # the tunnel never came up. + keep_log = True raise CLIError( "cloudflared didn't report a tunnel URL in time.", error_type="tunnel_error", exit_code=1, + suggestion=f"cloudflared's output was kept at {log_path} — check it for errors.", ) payload: dict[str, object] = { "url": public, @@ -112,6 +119,8 @@ def run_share(*, port: int, no_install: bool, json_mode: bool) -> None: finally: _terminate(proxy) _terminate(server) + if log_path is not None and not keep_log: + log_path.unlink(missing_ok=True) @app.command( @@ -140,7 +149,7 @@ def share( https://developers.cloudflare.com/cloudflare-one/connections/connect-networks/downloads/). """ - def body(_state: AppState, json_mode: bool) -> None: - run_share(port=port, no_install=no_install, json_mode=json_mode) + def body(state: AppState, json_mode: bool) -> None: + run_share(port=port, no_install=no_install, json_mode=json_mode, quiet=state.quiet) run_command(ctx, body, json=json_out) diff --git a/aai_cli/commands/speak.py b/aai_cli/commands/speak.py index d4a9a3e5..54dd8f2d 100644 --- a/aai_cli/commands/speak.py +++ b/aai_cli/commands/speak.py @@ -167,6 +167,7 @@ def _speak_dialogue( @app.command( rich_help_panel=help_panels.TRANSCRIPTION, + # --sandbox is a root flag, so it must come before the subcommand in every example. epilog=examples_epilog( [ ("Speak text aloud (sandbox only)", 'assembly --sandbox speak "Hello there, friend."'), @@ -195,11 +196,15 @@ def speak( voice: list[str] = typer.Option( [], "--voice", - help="Voice id, or SPEAKER=VOICE for diarized input (repeatable, e.g. --voice A=jane).", + help="Voice id (e.g. jane, michael, mary, paul, eve, george), or SPEAKER=VOICE " + "for diarized input (repeatable, e.g. --voice A=jane).", ), language: str = typer.Option(DEFAULT_LANGUAGE, "--language", help="Language of the text."), sample_rate: int | None = typer.Option( - None, "--sample-rate", help="Output sample rate in Hz. Server default if omitted." + None, + "--sample-rate", + help="Output sample rate in Hz (positive). Server default if omitted.", + min=1, ), out: Path | None = typer.Option( None, "--out", help="Write a WAV file instead of playing through the speakers." @@ -208,11 +213,12 @@ def speak( ) -> None: """Synthesize speech from text with AssemblyAI streaming TTS (sandbox only). - Plays the audio through your speakers by default, or writes a WAV with --out. - Speaker-labeled input (from 'assembly transcribe --speaker-labels') is detected - automatically: the labels are stripped and each speaker gets a different - voice. This feature only exists in the sandbox today — run it as - 'assembly --sandbox speak' (--sandbox goes before the subcommand). + Plays the audio through your speakers by default, or writes a WAV with + --out. Speaker-labeled input (from 'assembly transcribe + --speaker-labels') is detected automatically: the labels are stripped + and each speaker gets a different voice. This feature only exists in + the sandbox today — run it as 'assembly --sandbox speak' (--sandbox + goes before the subcommand). """ def body(state: AppState, json_mode: bool) -> None: @@ -221,7 +227,8 @@ def body(state: AppState, json_mode: bool) -> None: "assembly speak is only available in the sandbox.", error_type="unsupported_environment", exit_code=2, - suggestion="Re-run as 'assembly --sandbox speak …'.", + suggestion="Re-run as: assembly --sandbox speak … " + "(--sandbox goes before the command; or use --env sandbox000).", ) spoken = _read_text(text) api_key = config.resolve_api_key(profile=state.profile) @@ -239,6 +246,13 @@ def body(state: AppState, json_mode: bool) -> None: quiet=state.quiet, ) else: + if overrides: + # Mirror the inverse warning in _speak_dialogue: never drop a + # requested voice mapping silently. + output.emit_warning( + "Ignoring --voice SPEAKER=VOICE mappings; input has no speaker labels.", + json_mode=json_mode, + ) _speak_single( api_key, spoken, diff --git a/aai_cli/commands/stream.py b/aai_cli/commands/stream.py index e7f8f62f..5c14ea2a 100644 --- a/aai_cli/commands/stream.py +++ b/aai_cli/commands/stream.py @@ -109,7 +109,9 @@ def stream( sample_rate: int | None = typer.Option( None, "--sample-rate", - help="Force a microphone capture rate in Hz (default: device native).", + help="Audio rate in Hz (positive): capture rate for the mic, or the declared " + "rate of raw PCM on stdin (default: device native / 16000).", + min=1, rich_help_panel=help_panels.OPT_CAPTURE, ), device: int | None = typer.Option( diff --git a/aai_cli/commands/telemetry.py b/aai_cli/commands/telemetry.py index 2c5ab0f8..c8077201 100644 --- a/aai_cli/commands/telemetry.py +++ b/aai_cli/commands/telemetry.py @@ -14,7 +14,10 @@ from aai_cli.context import AppState, run_command from aai_cli.help_text import examples_epilog -app = typer.Typer(help="Anonymous usage telemetry: status, enable, disable.") +app = typer.Typer( + help="Anonymous usage telemetry: status, enable, disable.", + no_args_is_help=True, +) def _consent_label() -> str: @@ -39,6 +42,7 @@ def body(_state: AppState, json_mode: bool) -> None: data: dict[str, object] = { "enabled": telemetry.is_enabled(), "consent": _consent_label(), + "source": telemetry.consent_source(), "token_configured": bool(telemetry.client_token()), } @@ -49,11 +53,17 @@ def render(d: dict[str, object]) -> object: else output.muted("Telemetry is disabled.") ) detail = output.muted( - f"Consent: {d['consent']}. Intake token configured: " + f"Consent: {d['consent']} (source: {d['source']}). Intake token configured: " f"{'yes' if d['token_configured'] else 'no'}." ) - hint = output.hint( - "Opt out any time: 'assembly telemetry disable' or AAI_TELEMETRY_DISABLED=1." + # The hint points the way the user can actually move: opt out while + # enabled, re-enable once disabled — never the direction they're already in. + hint = ( + output.hint( + "Opt out any time: 'assembly telemetry disable' or AAI_TELEMETRY_DISABLED=1." + ) + if d["enabled"] + else output.hint("Re-enable with 'assembly telemetry enable'.") ) return output.stack(state_line, detail, hint) diff --git a/aai_cli/commands/transcribe.py b/aai_cli/commands/transcribe.py index c1832c8b..cd5b1d74 100644 --- a/aai_cli/commands/transcribe.py +++ b/aai_cli/commands/transcribe.py @@ -83,7 +83,9 @@ def transcribe( temperature: float | None = typer.Option( None, "--temperature", - help="Speech model temperature.", + help="Speech model temperature (0 most deterministic, 1 least).", + min=0.0, + max=1.0, rich_help_panel=help_panels.OPT_MODEL, ), prompt: str | None = typer.Option( @@ -253,7 +255,11 @@ def transcribe( rich_help_panel=help_panels.OPT_CUSTOMIZATION, ), audio_end: int | None = typer.Option( - None, "--audio-end", help="End offset in ms.", rich_help_panel=help_panels.OPT_CUSTOMIZATION + None, + "--audio-end", + help="End offset in ms.", + min=0, + rich_help_panel=help_panels.OPT_CUSTOMIZATION, ), download_sections: list[str] | None = typer.Option( None, @@ -354,8 +360,7 @@ def transcribe( transcribe many sources concurrently. Each source gets a .aai.json sidecar with the full result, and a re-run skips sources already transcribed. - Curated flags cover common features; --config KEY=VALUE and --config-file reach - every other field. Analysis (summary, chapters, ...) renders in human mode. + Curated flags cover common features; --config KEY=VALUE and --config-file reach every other field. Analysis (summary, chapters, ...) renders in human mode. """ def body(state: AppState, json_mode: bool) -> None: @@ -409,6 +414,8 @@ def body(state: AppState, json_mode: bool) -> None: flags.update(config_builder.auth_header_flags(webhook_auth_header)) transcribe_exec.validate_out_with_llm(out, llm_prompt) + transcribe_exec.validate_out_path(out) + transcribe_exec.validate_json_with_output(output_field, json_mode=json_mode) merged = config_builder.merge_transcribe_config( flags=flags, overrides=config_kv, config_file=config_file @@ -457,6 +464,7 @@ def body(state: AppState, json_mode: bool) -> None: # A typo'd path must read as "file not found", not trigger a login. transcribe_exec.check_source_exists(source, sample=sample) + transcribe_exec.warn_unrecognized_extension(source, json_mode=json_mode, quiet=state.quiet) api_key = config.resolve_api_key(profile=state.profile) with output.status("Transcribing…", json_mode=json_mode, quiet=state.quiet): diff --git a/aai_cli/commands/transcripts.py b/aai_cli/commands/transcripts.py index f07cbe84..9d0f9b5d 100644 --- a/aai_cli/commands/transcripts.py +++ b/aai_cli/commands/transcripts.py @@ -11,6 +11,51 @@ app = typer.Typer(help="Browse and fetch past transcripts.", no_args_is_help=True) +# `list` is registered before `get` so the subcommand help lists them in that +# order, matching `assembly sessions --help`. +@app.command( + name="list", + epilog=examples_epilog( + [ + ("List your recent transcripts", "assembly transcripts list"), + ("Show more at once", "assembly transcripts list --limit 50"), + ("Grab the latest transcript id", "assembly transcripts list --json | jq -r '.[0].id'"), + ( + "Summarize your latest transcript", + 'assembly llm "summarize" --transcript-id ' + "$(assembly transcripts list --json | jq -r '.[0].id')", + ), + ] + ), +) +def list_( + ctx: typer.Context, + limit: int = typer.Option(10, "--limit", help="How many transcripts to show.", min=1), + json_out: bool = options.json_option(), +) -> None: + """List recent transcripts.""" + + def body(state: AppState, json_mode: bool) -> None: + api_key = config.resolve_api_key(profile=state.profile) + rows = client.list_transcripts(api_key, limit=limit) + + def render(data: list[dict[str, object]]) -> object: + if not data: + return output.muted("No transcripts yet.") + table = output.data_table("id", "status", "created (UTC)") + for row in data: + table.add_row( + escape(str(row["id"])), + theme.status_text(str(row["status"])), + escape(timeparse.format_utc_datetime(row.get("created"))), + ) + return table + + output.emit(rows, render, json_mode=json_mode) + + run_command(ctx, body, json=json_out) + + @app.command( epilog=examples_epilog( [ @@ -35,6 +80,9 @@ def get( """Fetch a past transcript by id and print its text.""" def body(state: AppState, json_mode: bool) -> None: + # Cheap local id validation first: a malformed id is a usage error whether + # or not the user is signed in, so it must not trigger auth/login first. + client.validate_transcript_id(transcript_id) api_key = config.resolve_api_key(profile=state.profile) transcript = client.get_transcript(api_key, transcript_id) if client.status_str(transcript) == "error": @@ -58,46 +106,3 @@ def body(state: AppState, json_mode: bool) -> None: ) run_command(ctx, body, json=json_out) - - -@app.command( - name="list", - epilog=examples_epilog( - [ - ("List your recent transcripts", "assembly transcripts list"), - ("Show more at once", "assembly transcripts list --limit 50"), - ("Grab the latest transcript id", "assembly transcripts list --json | jq -r '.[0].id'"), - ( - "Summarize your latest transcript", - 'assembly llm "summarize" --transcript-id ' - "$(assembly transcripts list --json | jq -r '.[0].id')", - ), - ] - ), -) -def list_( - ctx: typer.Context, - limit: int = typer.Option(10, "--limit", help="How many transcripts to show.", min=1), - json_out: bool = options.json_option(), -) -> None: - """List recent transcripts.""" - - def body(state: AppState, json_mode: bool) -> None: - api_key = config.resolve_api_key(profile=state.profile) - rows = client.list_transcripts(api_key, limit=limit) - - def render(data: list[dict[str, object]]) -> object: - if not data: - return output.muted("No transcripts yet.") - table = output.data_table("id", "status", "created (UTC)") - for row in data: - table.add_row( - escape(str(row["id"])), - theme.status_text(str(row["status"])), - escape(timeparse.format_utc_datetime(row.get("created"))), - ) - return table - - output.emit(rows, render, json_mode=json_mode) - - run_command(ctx, body, json=json_out) diff --git a/aai_cli/config.py b/aai_cli/config.py index efe626c1..6c69559a 100644 --- a/aai_cli/config.py +++ b/aai_cli/config.py @@ -61,6 +61,16 @@ class StoredSession(BaseModel): token: str = "" +def validate_profile(name: str) -> None: + """Reject profile names that aren't simple identifiers. + + Public so resolution-time callers (``context.AppState.resolve_profile``) can + fail fast on a typo'd ``--profile`` before any network work, instead of only + tripping over it at keyring-write time. + """ + _validate_profile(name) + + def _validate_profile(name: str) -> None: if not _PROFILE_RE.match(name): from aai_cli.errors import CLIError @@ -356,6 +366,13 @@ def persist_login( _dump(prior_cfg) +def has_device_id() -> bool: + """Whether the anonymous telemetry device id has been minted yet, without + minting one — lets telemetry detect the true first run for its one-time + collection disclosure.""" + return _load().device_id is not None + + def get_device_id() -> str: """A stable anonymous install id for telemetry: a random UUID minted locally on first use and persisted in config.toml. Carries nothing derivable from the @@ -395,8 +412,12 @@ def set_update_cache(*, last_check: float, latest_version: str | None) -> None: def resolve_api_key(*, profile: str | None = None, api_key_flag: str | None = None) -> str: + # Values are stripped at every tier: a whitespace-only key (e.g. a botched + # `export ASSEMBLYAI_API_KEY=' '`) must read as "no key" (the clean exit-4 + # not-signed-in path), not get sent as an illegal HTTP header byte string. if api_key_flag is not None: - if not api_key_flag: + flag_key = api_key_flag.strip() + if not flag_key: from aai_cli.errors import CLIError raise CLIError( @@ -405,12 +426,12 @@ def resolve_api_key(*, profile: str | None = None, api_key_flag: str | None = No exit_code=2, suggestion="Pass a non-empty key, e.g. --api-key sk_...", ) - return api_key_flag - env_key = os.environ.get(ENV_API_KEY) + return flag_key + env_key = (os.environ.get(ENV_API_KEY) or "").strip() if env_key: return env_key profile = profile or get_active_profile() - stored = get_api_key(profile) + stored = (get_api_key(profile) or "").strip() if stored: return stored raise NotAuthenticated() diff --git a/aai_cli/context.py b/aai_cli/context.py index 4ef0ebaf..1d04e9f1 100644 --- a/aai_cli/context.py +++ b/aai_cli/context.py @@ -30,8 +30,16 @@ class AppState: quiet: bool = False def resolve_profile(self) -> str: - """The profile to act on: explicit --profile, else the active profile.""" - return self.profile or config.get_active_profile() + """The profile to act on: explicit --profile, else the active profile. + + An explicit ``--profile`` is validated here, at resolution time, so a typo'd + name is a fast usage error in the root callback — before any network + round-trip — instead of only failing at keyring-write time after the work. + """ + if self.profile is not None: + config.validate_profile(self.profile) + return self.profile + return config.get_active_profile() def resolve_environment(self) -> Environment: """The backend environment: --env > AAI_ENV > the profile's stored env > default.""" @@ -51,8 +59,15 @@ def resolve_session(self) -> tuple[int, str]: session = config.get_session(profile) account_id = config.get_account_id(profile) if session is None or account_id is None: + # The inherited default suggestion offers ASSEMBLYAI_API_KEY, which can + # 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)." + "These commands need a browser login. Run 'assembly login' (without --api-key).", + suggestion=( + "Run 'assembly login' to sign in via your browser — an API key alone " + "can't access account commands." + ), ) return account_id, session["jwt"] @@ -98,9 +113,13 @@ def env_override_warning(state: AppState) -> str | None: return state.env_override_warning() -def persist_browser_login(profile: str, env: str) -> None: - """Run the browser login flow and persist its credentials for `profile`/`env`.""" - result = run_login_flow() +def persist_browser_login(profile: str, env: str, *, json_mode: bool = False) -> None: + """Run the browser login flow and persist its credentials for `profile`/`env`. + + ``json_mode`` keeps the flow's stderr progress notes machine-readable under + ``--json`` (each becomes a ``{"hint": …}`` object instead of prose). + """ + result = run_login_flow(json_mode=json_mode) config.persist_login( profile, api_key=result.api_key, @@ -111,8 +130,8 @@ def persist_browser_login(profile: str, env: str) -> None: ) -def _persist_browser_login(state: AppState) -> None: - persist_browser_login(state.resolve_profile(), environments.active().name) +def _persist_browser_login(state: AppState, *, json_mode: bool) -> None: + persist_browser_login(state.resolve_profile(), environments.active().name, json_mode=json_mode) def _login_persistence_error(exc: object) -> APIError: @@ -164,7 +183,7 @@ def _auto_login_and_exit(state: AppState, *, json_mode: bool) -> NoReturn: output.error_console.print( "[aai.muted]Not signed in; starting browser login.[/aai.muted]" ) - _persist_browser_login(state) + _persist_browser_login(state, json_mode=json_mode) except CLIError as login_err: output.emit_error(login_err, json_mode=json_mode) raise typer.Exit(code=login_err.exit_code) from None diff --git a/aai_cli/eval_data.py b/aai_cli/eval_data.py index 021e8072..128e79f2 100644 --- a/aai_cli/eval_data.py +++ b/aai_cli/eval_data.py @@ -22,19 +22,13 @@ import csv import json -import os import re from dataclasses import dataclass -from http import HTTPStatus from pathlib import Path -import httpx2 as httpx - -from aai_cli import der, jsonshape, wer +from aai_cli import der, eval_hf_api, jsonshape, wer from aai_cli.errors import APIError, CLIError, UsageError -_DATASETS_SERVER = "https://datasets-server.huggingface.co" -_TIMEOUT = 30.0 # pragma: no mutate (request timeout; nothing observable to assert) _MANIFEST_SUFFIXES = (".csv", ".jsonl") # Hub ids are `name` or `namespace/name`; rejecting anything else keeps a typo'd # local path from being sent to the hub as if it were a dataset id. @@ -157,8 +151,10 @@ def _pick_column( for candidate in candidates: if candidate in available: return candidate + noun = candidates[0] + article = "an" if noun[0] in "aeiou" else "a" raise UsageError( - f"Could not find a {candidates[0]} column (columns: {', '.join(available)}).", + f"Could not find {article} {noun} column (columns: {', '.join(available)}).", suggestion=f"Name it with {flag}.", ) @@ -261,6 +257,13 @@ def _load_manifest( exit_code=2, suggestion="Pass a .csv/.jsonl manifest path, or a Hugging Face dataset id.", ) + if path.suffix not in _MANIFEST_SUFFIXES: + # Other suffixes (.parquet, .txt, …) would be parsed as JSONL and fail with a + # confusing "line 1 is not valid JSON" — name the real constraint instead. + raise UsageError( + f"Manifests must be .csv or .jsonl; got '{path.name}'.", + suggestion="Convert the manifest, or pass a Hugging Face dataset id.", + ) rows = _manifest_rows(path) if not rows: raise UsageError(f"Manifest {path.name} has no rows.") @@ -315,100 +318,6 @@ def _manifest_item( # ------------------------------------------------------- Hugging Face datasets -def _error_detail(resp: httpx.Response) -> str: - try: - body: object = resp.json() - except ValueError: - return resp.text - mapping = jsonshape.as_mapping(body) - if mapping is not None and "error" in mapping: - return str(mapping["error"]) - return resp.text - - -def _checked_payload(resp: httpx.Response, *, dataset: str) -> dict[str, object]: - if resp.status_code in (401, 403): - raise APIError( - f"Hugging Face denied access to '{dataset}' (HTTP {resp.status_code}).", - suggestion="Gated or private dataset? Set HF_TOKEN to a token that has access.", - ) - if resp.status_code == HTTPStatus.NOT_FOUND: - raise UsageError( - f"Hugging Face dataset '{dataset}' was not found: {_error_detail(resp)}", - suggestion="Check the dataset id, e.g. 'distil-whisper/meanwhile'.", - ) - if resp.status_code != HTTPStatus.OK: - raise APIError( - f"Hugging Face datasets server error (HTTP {resp.status_code}): {_error_detail(resp)}" - ) - try: - data: object = resp.json() - except ValueError as exc: - raise APIError("Hugging Face datasets server returned invalid JSON.") from exc - mapping = jsonshape.as_mapping(data) - if mapping is None: - raise APIError( - "Hugging Face datasets server returned unexpected JSON (expected an object)." - ) - return mapping - - -def _fetch_json(endpoint: str, params: dict[str, str | int], *, dataset: str) -> dict[str, object]: - token = os.environ.get("HF_TOKEN") - headers = {"authorization": f"Bearer {token}"} if token else {} - try: - with httpx.Client(base_url=_DATASETS_SERVER, timeout=_TIMEOUT, headers=headers) as client: - resp = client.get(endpoint, params=params) - except httpx.HTTPError as exc: - raise APIError(f"Could not reach the Hugging Face datasets server: {exc}") from exc - return _checked_payload(resp, dataset=dataset) - - -def _split_entries(dataset: str) -> list[dict[str, object]]: - payload = _fetch_json("/splits", {"dataset": dataset}, dataset=dataset) - entries = jsonshape.mapping_list(payload.get("splits")) - if not entries: - raise APIError(f"Hugging Face reports no splits for '{dataset}'.") - return entries - - -def _pick_subset(entries: list[dict[str, object]], subset: str | None, dataset: str) -> str: - configs = list(dict.fromkeys(str(entry.get("config")) for entry in entries)) - if subset is not None: - if subset in configs: - return subset - raise UsageError(f"'{dataset}' has no subset '{subset}' (subsets: {', '.join(configs)}).") - if len(configs) == 1: - return configs[0] - if "default" in configs: - return "default" - raise UsageError( - f"'{dataset}' has multiple subsets: {', '.join(configs)}.", - suggestion="Pick one with --subset.", - ) - - -def _pick_split( - entries: list[dict[str, object]], config: str, split: str | None, dataset: str -) -> str: - splits = [str(entry.get("split")) for entry in entries if str(entry.get("config")) == config] - if split is not None: - if split in splits: - return split - raise UsageError( - f"'{dataset}' has no '{split}' split in subset '{config}' " - f"(splits: {', '.join(splits)})." - ) - if "test" in splits: - return "test" - if len(splits) == 1: - return splits[0] - raise UsageError( - f"'{dataset}' has several splits in subset '{config}': {', '.join(splits)}.", - suggestion="Pick one with --split (eval sets usually score 'test').", - ) - - def _audio_source(cell: object, *, column: str, item_id: str) -> str: """The audio URL out of a datasets-server cell: a bare string, or the first ``src`` of the ``[{"src": …, "type": …}]`` shape audio columns render as.""" @@ -457,10 +366,10 @@ def _load_hf( f"'{dataset}' is neither a local .csv/.jsonl manifest nor a Hugging Face dataset id.", suggestion="Pass a manifest path, or an id like 'distil-whisper/meanwhile'.", ) - entries = _split_entries(dataset) - config = _pick_subset(entries, subset, dataset) - split_name = _pick_split(entries, config, split, dataset) - payload = _fetch_json( + entries = eval_hf_api.split_entries(dataset) + config = eval_hf_api.pick_subset(entries, subset, dataset) + split_name = eval_hf_api.pick_split(entries, config, split, dataset) + payload = eval_hf_api.fetch_json( "/rows", {"dataset": dataset, "config": config, "split": split_name, "offset": 0, "length": limit}, dataset=dataset, diff --git a/aai_cli/eval_hf_api.py b/aai_cli/eval_hf_api.py new file mode 100644 index 00000000..367e5d2c --- /dev/null +++ b/aai_cli/eval_hf_api.py @@ -0,0 +1,137 @@ +"""The Hugging Face datasets-server REST client behind `assembly eval`. + +Thin httpx wrappers (split discovery, subset/split selection, row fetching) +with the error translation `eval_data` relies on: auth/gating denials get an +``HF_TOKEN`` hint only when the response body actually reads like gating, and +everything else surfaces the server's own detail verbatim. +""" + +from __future__ import annotations + +import os +from http import HTTPStatus + +import httpx2 as httpx + +from aai_cli import jsonshape +from aai_cli.errors import APIError, UsageError + +_DATASETS_SERVER = "https://datasets-server.huggingface.co" +_TIMEOUT = 30.0 # pragma: no mutate (request timeout; nothing observable to assert) + + +def _error_detail(resp: httpx.Response) -> str: + try: + body: object = resp.json() + except ValueError: + return resp.text + mapping = jsonshape.as_mapping(body) + if mapping is not None and "error" in mapping: + return str(mapping["error"]) + return resp.text + + +# A 401/403 body that mentions one of these reads like HF auth/gating, where a token +# can actually help; anything else (e.g. a sandbox proxy's "Host not in allowlist") +# gets the body verbatim instead of a misleading HF_TOKEN hint. +_GATING_HINTS = ("gated", "private", "auth", "token") + + +def _looks_gating_related(detail: str) -> bool: + lowered = detail.lower() + return not detail or any(hint in lowered for hint in _GATING_HINTS) + + +def _denied_access_error(resp: httpx.Response, *, dataset: str) -> APIError: + detail = _error_detail(resp) + message = f"Hugging Face denied access to '{dataset}' (HTTP {resp.status_code})" + if detail: + message += f": {detail}" + return APIError( + message, + suggestion=( + "Gated or private dataset? Set HF_TOKEN to a token that has access." + if _looks_gating_related(detail) + else None + ), + ) + + +def _checked_payload(resp: httpx.Response, *, dataset: str) -> dict[str, object]: + if resp.status_code in (401, 403): + raise _denied_access_error(resp, dataset=dataset) + if resp.status_code == HTTPStatus.NOT_FOUND: + raise UsageError( + f"Hugging Face dataset '{dataset}' was not found: {_error_detail(resp)}", + suggestion="Check the dataset id, e.g. 'distil-whisper/meanwhile'.", + ) + if resp.status_code != HTTPStatus.OK: + raise APIError( + f"Hugging Face datasets server error (HTTP {resp.status_code}): {_error_detail(resp)}" + ) + try: + data: object = resp.json() + except ValueError as exc: + raise APIError("Hugging Face datasets server returned invalid JSON.") from exc + mapping = jsonshape.as_mapping(data) + if mapping is None: + raise APIError( + "Hugging Face datasets server returned unexpected JSON (expected an object)." + ) + return mapping + + +def fetch_json(endpoint: str, params: dict[str, str | int], *, dataset: str) -> dict[str, object]: + token = os.environ.get("HF_TOKEN") + headers = {"authorization": f"Bearer {token}"} if token else {} + try: + with httpx.Client(base_url=_DATASETS_SERVER, timeout=_TIMEOUT, headers=headers) as client: + resp = client.get(endpoint, params=params) + except httpx.HTTPError as exc: + raise APIError(f"Could not reach the Hugging Face datasets server: {exc}") from exc + return _checked_payload(resp, dataset=dataset) + + +def split_entries(dataset: str) -> list[dict[str, object]]: + payload = fetch_json("/splits", {"dataset": dataset}, dataset=dataset) + entries = jsonshape.mapping_list(payload.get("splits")) + if not entries: + raise APIError(f"Hugging Face reports no splits for '{dataset}'.") + return entries + + +def pick_subset(entries: list[dict[str, object]], subset: str | None, dataset: str) -> str: + configs = list(dict.fromkeys(str(entry.get("config")) for entry in entries)) + if subset is not None: + if subset in configs: + return subset + raise UsageError(f"'{dataset}' has no subset '{subset}' (subsets: {', '.join(configs)}).") + if len(configs) == 1: + return configs[0] + if "default" in configs: + return "default" + raise UsageError( + f"'{dataset}' has multiple subsets: {', '.join(configs)}.", + suggestion="Pick one with --subset.", + ) + + +def pick_split( + entries: list[dict[str, object]], config: str, split: str | None, dataset: str +) -> str: + splits = [str(entry.get("split")) for entry in entries if str(entry.get("config")) == config] + if split is not None: + if split in splits: + return split + raise UsageError( + f"'{dataset}' has no '{split}' split in subset '{config}' " + f"(splits: {', '.join(splits)})." + ) + if "test" in splits: + return "test" + if len(splits) == 1: + return splits[0] + raise UsageError( + f"'{dataset}' has several splits in subset '{config}': {', '.join(splits)}.", + suggestion="Pick one with --split (eval sets usually score 'test').", + ) diff --git a/aai_cli/init/devserver.py b/aai_cli/init/devserver.py index 13a01546..ab0d2e8d 100644 --- a/aai_cli/init/devserver.py +++ b/aai_cli/init/devserver.py @@ -3,7 +3,7 @@ from pathlib import Path -from aai_cli import steps +from aai_cli import output, steps from aai_cli.init import runner @@ -21,6 +21,18 @@ def install_step(target: Path, *, no_install: bool, use_uv: bool) -> steps.Step: return {"name": "install", "status": "installed", "detail": "uv" if use_uv else "venv + pip"} +def notify_port_change(requested: int, chosen: int, *, json_mode: bool, quiet: bool) -> None: + """One stderr line when the requested port was busy and a neighbor was bound. + + `assembly dev`/`assembly share` silently substituting a free port would leave the + user pointing tools at a dead port. Port 0 means "any free port", so no notice + there, and ``--quiet`` suppresses it. + """ + if quiet or requested in (0, chosen): + return + output.emit_warning(f"Port {requested} is in use; using {chosen}.", json_mode=json_mode) + + # Local dev binds the loopback interface only. The template Procfile says # `--host 0.0.0.0` — correct for the deploy targets (Railway/Fly route traffic into # the container) but wrong for `assembly dev`/`assembly share`: the .env beside it holds a real diff --git a/aai_cli/init/keys.py b/aai_cli/init/keys.py index bc3df2bc..4fd58753 100644 --- a/aai_cli/init/keys.py +++ b/aai_cli/init/keys.py @@ -1,12 +1,23 @@ from __future__ import annotations +import os + from aai_cli import config -def resolve_optional_api_key(*, profile: str | None) -> str | None: - """The CLI's key chain (env -> keyring), but None instead of raising when absent. +def resolve_optional_api_key(*, profile: str | None) -> tuple[str | None, str | None]: + """The CLI's key chain (env -> keyring) plus which source supplied the key. - `assembly init` scaffolds even without a key (writing a placeholder), so it must not - fail the way run commands do. + Returns ``(key, source)`` with source ``"environment"`` or ``"keyring"``, or + ``(None, None)`` when absent. `assembly init` scaffolds even without a key + (writing a placeholder), so it must not fail the way run commands do; the + source feeds the report's ``key`` row. """ - return config.resolve_api_key_optional(profile=profile) + key = config.resolve_api_key_optional(profile=profile) + if key is None: + return None, None + # Mirror resolve_api_key's whitespace handling: a blank env var is "unset", + # so a key that actually came from the keyring must not report "environment". + env_value = os.environ.get(config.ENV_API_KEY, "").strip() + source = "environment" if env_value else "keyring" + return key, source diff --git a/aai_cli/init/runner.py b/aai_cli/init/runner.py index 274657ca..de03ec7c 100644 --- a/aai_cli/init/runner.py +++ b/aai_cli/init/runner.py @@ -10,6 +10,7 @@ import webbrowser from pathlib import Path +from aai_cli import output from aai_cli.errors import CLIError @@ -121,6 +122,18 @@ def run_setup(target: Path, *, use_uv: bool) -> subprocess.CompletedProcess[str] return last +def open_app_browser(port: int) -> None: + """Open the app URL, saying where to point a browser when none can launch. + + `webbrowser.open` returns False on headless boxes (no display/$BROWSER); a + silent False would leave the user staring at a running server with no URL. + The hint goes to stderr so stdout stays clean for pipelines. + """ + url = f"http://localhost:{port}" + if not webbrowser.open(url): + output.error_console.print(output.hint(f"Couldn't open a browser — visit {url}")) + + def run_server( target: Path, *, @@ -137,7 +150,7 @@ def run_server( proc = subprocess.Popen(command, cwd=target, env=env) try: if wait_for_port(port) and open_browser: - webbrowser.open(f"http://localhost:{port}") + open_app_browser(port) proc.wait() except KeyboardInterrupt: proc.terminate() diff --git a/aai_cli/init/scaffold.py b/aai_cli/init/scaffold.py index 3e28b0fa..8304203a 100644 --- a/aai_cli/init/scaffold.py +++ b/aai_cli/init/scaffold.py @@ -58,6 +58,25 @@ def target_conflict(target: Path) -> bool: return target.is_dir() and any(target.iterdir()) +def existing_env_key(target: Path) -> str | None: + """The real API key already configured in ``target/.env``, or None. + + Re-scaffolding (``assembly init --force``) rewrites ``.env``; when no key resolves + for the new write, blindly writing the placeholder would silently wipe a key the + user already configured. Returns None for a missing ``.env``, a blank value, or + the placeholder itself — only a configured real key is worth preserving. + """ + env_path = target / ".env" + if not env_path.is_file(): + return None + for line in env_path.read_text().splitlines(): + if line.startswith("ASSEMBLYAI_API_KEY="): + value = line.removeprefix("ASSEMBLYAI_API_KEY=").strip() + if value and value != PLACEHOLDER_KEY: + return value + return None + + def _copy_tree(node: Traversable, dest: Path) -> None: for child in node.iterdir(): if child.name in _SKIP_NAMES or child.name.endswith(".pyc"): diff --git a/aai_cli/llm.py b/aai_cli/llm.py index 40156b82..cb2e715b 100644 --- a/aai_cli/llm.py +++ b/aai_cli/llm.py @@ -73,6 +73,31 @@ def _client(api_key: str) -> OpenAI: return OpenAI(api_key=api_key, base_url=environments.active().llm_gateway_base) +# Lowercased substrings that mark a gateway 401/403 as a plan-entitlement block +# rather than a bad key or an intercepting proxy. "no access" is the gateway's own +# phrasing for accounts without the LLM Gateway entitlement. +_ENTITLEMENT_HINTS = ("entitle", "plan", "upgrade", "billing", "no access") + +_PAID_PLAN_SUGGESTION = ( + "The LLM Gateway requires a paid plan — check your plan at " + "https://www.assemblyai.com/dashboard." +) +_ACCESS_DENIED_SUGGESTION = ( + "Check your API key ('assembly login') and that your network/proxy allows the " + "LLM Gateway, then try again." +) + + +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): + return _PAID_PLAN_SUGGESTION + return _ACCESS_DENIED_SUGGESTION + + def complete( api_key: str, *, @@ -99,14 +124,13 @@ def complete( extra_body=extra_body, ) except (openai.AuthenticationError, openai.PermissionDeniedError) as exc: - # The gateway returns 401/403 for both an invalid key and a plan - # entitlement block ("no access to LLM Gateway"), so surface its actual - # message rather than a generic "run assembly login" that misleads unpaid - # accounts (the key is fine; the feature requires a paid plan). + # 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. raise APIError( f"LLM Gateway access denied: {exc}", - suggestion="The LLM Gateway requires a paid plan — check your plan at " - "https://www.assemblyai.com/dashboard.", + suggestion=_denial_suggestion(exc), ) from exc except openai.OpenAIError as exc: raise APIError( diff --git a/aai_cli/main.py b/aai_cli/main.py index a2821555..e3205f34 100644 --- a/aai_cli/main.py +++ b/aai_cli/main.py @@ -10,6 +10,7 @@ from rich.table import Table from typer import completion, rich_utils from typer._click.exceptions import ClickException, NoSuchOption +from typer._click.exceptions import UsageError as ClickUsageError from typer._click.utils import PacifyFlushWrapper from typer.core import TyperGroup @@ -42,7 +43,7 @@ transcripts, ) from aai_cli.context import AppState, env_override_warning, resolve_environment -from aai_cli.errors import CLIError, NotAuthenticated +from aai_cli.errors import CLIError, NotAuthenticated, UsageError from aai_cli.help_text import examples_epilog from aai_cli.onboard import wizard from aai_cli.onboard.sections import WizardContext @@ -154,14 +155,79 @@ def _patch_module(module: ModuleType, **attrs: object) -> None: _format_click_error = rich_utils.rich_format_error +# Flags users habitually pass at the wrong level: `--json` belongs on the subcommand +# (`assembly transcribe --json`), while these live on the root callback +# (`assembly --sandbox transcribe`). A bare "No such option" — or worse, a similarity +# guess like "(Possible options: --version)" — is unlearnable, so the Click error +# formatter appends the correct placement instead. +_JSON_FLAGS = ("--json", "-j") +_ROOT_ONLY_FLAGS = ("--quiet", "-q", "--sandbox", "--env", "--profile", "-p") + + +def _misplaced_flag_hint(err: NoSuchOption) -> str | None: + """A placement hint when a known flag landed at the wrong level, else None.""" + ctx = err.ctx + if ctx is None: + return None + if ctx.parent is None: + if err.option_name in _JSON_FLAGS: + return "Pass --json after the subcommand: assembly --json" + return None + if err.option_name in _ROOT_ONLY_FLAGS: + command = ctx.command_path.removeprefix("assembly ") + return ( + "This is a global flag; pass it before the subcommand: " + f"assembly {err.option_name} {command} …" + ) + return None + + +def _rewrite_version_command_error(err: ClickException) -> None: + # There is no `version` subcommand (the reflex is `assembly --version`), and the + # closest-match engine would suggest an unrelated command ("Did you mean + # 'sessions'?"). Point at the real spelling instead. + if err.message.startswith("No such command 'version'"): + err.message = "No such command 'version'. Did you mean 'assembly --version'?" + + +def _click_error_requests_json(err: ClickException) -> bool: + """Whether the invocation that failed to parse had opted into JSON output. + + A parse error fires before any command's own ``--json`` is read, so sniff the raw + token list the root group stashed on the context (see ``_OrderedGroup.parse_args``). + A ClickException raised without a context falls back to the process argv. + """ + ctx = err.ctx if isinstance(err, ClickUsageError) else None + if ctx is not None and _RAW_ARGS_META_KEY in ctx.meta: + raw_args: list[str] = ctx.meta[_RAW_ARGS_META_KEY] + else: + raw_args = sys.argv[1:] + return _command_line_requests_json(raw_args) + 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))})" + # into the message ourselves so the user sees "(Possible options: --json)" — or, + # for a known flag passed at the wrong level, the placement hint instead of a + # misleading similarity guess. + if isinstance(self, NoSuchOption): + hint = _misplaced_flag_hint(self) + if hint is not None: + self.message = f"{self.message}. {hint}" + elif self.possibilities: + self.message = ( + f"{self.message} (Possible options: {', '.join(sorted(self.possibilities))})" + ) self.possibilities = None + _rewrite_version_command_error(self) + if _click_error_requests_json(self): + # An invocation that opted into JSON gets the uniform {"error": …} envelope for + # parse errors too, mirroring the root-callback failure path; the exit code (2) + # is Click's and unchanged. NoArgsIsHelpError never reaches this branch: its + # message is the help screen and a bare invocation carries no JSON flag. + output.emit_error(UsageError(self.format_message()), json_mode=True) + return _format_click_error(self) @@ -190,8 +256,8 @@ def _format_click_error_fixed(self: ClickException) -> None: def _version_callback(value: bool) -> None: """Print the version and exit when `assembly --version`/`-V` is passed, before any command - runs. Mirrors the reflex (`tool --version`) every other CLI answers; the `version` - subcommand stays for parity.""" + runs. Mirrors the reflex (`tool --version`) every other CLI answers. There is + deliberately no `version` subcommand; the unknown-command error points here instead.""" if value: typer.echo(__version__) raise typer.Exit() @@ -297,16 +363,16 @@ def main( is_eager=True, # pragma: no mutate ), ) -> None: - conflict_warning = _sandbox_conflict_warning(sandbox, env) - if sandbox and env is None: - env = "sandbox000" - state = AppState(profile=profile, env=env, quiet=quiet) - ctx.obj = state # The command's own --json flag isn't parsed yet, so sniff the pending command line: # a root-callback failure (e.g. bad --env) still emits the JSON error shape when the # invocation opted into JSON, and renders human text on stderr otherwise. raw_args: list[str] = ctx.meta.get(_RAW_ARGS_META_KEY, []) json_mode = output.resolve_json(explicit=_command_line_requests_json(raw_args)) + conflict_warning = _sandbox_conflict_warning(sandbox, env) + if sandbox and env is None: + env = "sandbox000" + state = AppState(profile=profile, env=env, quiet=quiet) + ctx.obj = state try: environments.set_active(resolve_environment(state)) except CLIError as err: diff --git a/aai_cli/onboard/sections.py b/aai_cli/onboard/sections.py index 83bbe5af..63500a0e 100644 --- a/aai_cli/onboard/sections.py +++ b/aai_cli/onboard/sections.py @@ -6,8 +6,9 @@ import assemblyai as aai import typer +from rich.markup import escape -from aai_cli import config, environments, output, transcribe_exec, transcribe_render +from aai_cli import config, environments, output, theme, transcribe_exec, transcribe_render from aai_cli.commands import doctor as doctor_cmd from aai_cli.commands import init as init_cmd from aai_cli.commands import setup as setup_cmd @@ -86,7 +87,8 @@ def first_request(prompter: Prompter, ctx: WizardContext) -> SectionResult: except CLIError as exc: output.error_console.print(output.fail(f"Transcription failed: {exc.message}")) return SectionResult.FAILED - transcribe_render.render_transcript_result(transcript, output.console) + if not ctx.json_mode: # --json owns stdout (the final summary); skip the human render + transcribe_render.render_transcript_result(transcript, output.console) return SectionResult.DONE @@ -98,18 +100,59 @@ def first_request(prompter: Prompter, ctx: WizardContext) -> SectionResult: ] -def environment(prompter: Prompter, _ctx: WizardContext) -> SectionResult: +# Status -> (glyph, style) for the wizard's environment render (same look as doctor's). +_CHECK_SYMBOLS = { + "ok": (theme.SYMBOL_SUCCESS, "aai.success"), + "warn": (theme.SYMBOL_WARN, "aai.warn"), + "fail": (theme.SYMBOL_ERROR, "aai.error"), +} + + +def _environment_summary(checks: list[doctor_cmd.Check]) -> str: + """The closing line, computed from the actual statuses: doctor.render's + all-or-nothing `ok` flag can't say "warnings only", which previously put + "Everything looks good." right under a warning.""" + failed = sum(1 for c in checks if c["status"] == "fail") + warned = sum(1 for c in checks if c["status"] == "warn") + if failed: + noun = "problem" if failed == 1 else "problems" + return output.fail(f"{failed} {noun} found — see fixes above.") + if warned: + noun = "warning" if warned == 1 else "warnings" + return output.warn(f"Ready — {warned} {noun} (only affects streaming/agent).") + return output.success("Everything looks good.") + + +def _render_environment(checks: list[doctor_cmd.Check]) -> str: + """The wizard's render of the doctor checks: doctor-style per-check lines, with + the summary derived from what the checks actually reported.""" + lines = [output.heading("Environment check")] + for c in checks: + symbol, style = _CHECK_SYMBOLS[c["status"]] + lines.append( + f" [{style}]{escape(symbol)}[/{style}] {escape(c['name'])} — {escape(c['detail'])}" + ) + if c["fix"]: + lines.append(" " + output.hint(f"fix: {escape(c['fix'])}")) + lines.append(" " + _environment_summary(checks)) + return "\n".join(lines) + + +def environment(prompter: Prompter, ctx: WizardContext) -> SectionResult: checks = [ doctor_cmd.check_python(), doctor_cmd.check_ffmpeg(), doctor_cmd.check_audio(), ] - # `render` already prints its own "Environment check" heading, so we don't call - # prompter.section here (that would show the title twice); just space it from the - # previous section with a blank line. - output.console.print() - output.console.print(doctor_cmd.render({"ok": True, "checks": checks})) - prompter.note("Warnings here only affect live streaming and the voice agent.") + if not ctx.json_mode: # --json owns stdout (the final summary); skip the human render + # `_render_environment` prints its own "Environment check" heading, so we don't + # call prompter.section here (that would show the title twice); just space it + # from the previous section with a blank line. + output.console.print() + output.console.print(_render_environment(checks)) + prompter.note("Warnings here only affect live streaming and the voice agent.") + if any(c["status"] == "fail" for c in checks): + return SectionResult.FAILED return SectionResult.DONE @@ -157,11 +200,12 @@ def claude_code(prompter: Prompter, _ctx: WizardContext) -> SectionResult: return SectionResult.DONE -def next_steps(prompter: Prompter, _ctx: WizardContext) -> SectionResult: +def next_steps(prompter: Prompter, ctx: WizardContext) -> SectionResult: prompter.section("You're set up") - output.console.print(output.hint("Transcribe a file: assembly transcribe ")) - output.console.print(output.hint("Stream live audio: assembly stream")) - output.console.print(output.hint("Build an app: assembly init")) + if not ctx.json_mode: # --json owns stdout (the final summary); hints are human-only + output.console.print(output.hint("Transcribe a file: assembly transcribe ")) + output.console.print(output.hint("Stream live audio: assembly stream")) + output.console.print(output.hint("Build an app: assembly init")) return SectionResult.DONE diff --git a/aai_cli/onboard/wizard.py b/aai_cli/onboard/wizard.py index f4b57d86..12740f84 100644 --- a/aai_cli/onboard/wizard.py +++ b/aai_cli/onboard/wizard.py @@ -1,39 +1,86 @@ from __future__ import annotations +from collections.abc import Callable + from aai_cli import output from aai_cli.errors import NotAuthenticated from aai_cli.onboard import sections from aai_cli.onboard.prompter import Prompter, WizardCancelled from aai_cli.onboard.sections import SectionResult, WizardContext +_SectionFn = Callable[[Prompter, WizardContext], SectionResult] + def run_onboarding(prompter: Prompter, ctx: WizardContext) -> int: """Run the ordered sections; return a process exit code. - Auth is the one hard stop (no key → later sections can't run). Cancellation - (Ctrl-C / empty pick) exits cleanly. The terminal cursor is always restored. + Auth is the one hard stop (no key → later sections can't run); any other failed + section is recorded and surfaced in the closing line, with exit code 1, instead + of being declared a success. Cancellation (Ctrl-C / empty pick) exits cleanly. + The terminal cursor is always restored. """ + results: dict[str, str] = {} + + def _run(label: str, section: _SectionFn) -> SectionResult: + result = section(prompter, ctx) + results[label] = result.value + return result + try: - sections.welcome(prompter, ctx) - if sections.auth(prompter, ctx) is SectionResult.FAILED: + _run("welcome", sections.welcome) + if _run("sign-in", sections.auth) is SectionResult.FAILED: # The auth section already printed the specific next step (browser retry, # or — non-interactively — `assembly login`/ASSEMBLYAI_API_KEY), so keep this # terminal line neutral rather than implying a re-run always fixes it. - output.error_console.print(output.fail("Sign-in didn't complete.")) - return NotAuthenticated().exit_code - sections.first_request(prompter, ctx) - sections.environment(prompter, ctx) - sections.build_path(prompter, ctx) - sections.claude_code(prompter, ctx) - sections.next_steps(prompter, ctx) + if not ctx.json_mode: + output.error_console.print(output.fail("Sign-in didn't complete.")) + return _summarize(ctx, results, NotAuthenticated().exit_code) + _run("first transcription", sections.first_request) + _run("environment", sections.environment) + _run("build path", sections.build_path) + _run("coding agent", sections.claude_code) + _run("next steps", sections.next_steps) # Last on purpose: the dev server blocks until Ctrl-C. - sections.launch_app(prompter, ctx) + _run("launch app", sections.launch_app) except WizardCancelled: output.error_console.print( output.hint("Setup cancelled. Run `assembly onboard` to resume.") ) return 130 else: - return 0 + return _summarize(ctx, results, 0) finally: output.console.show_cursor(show=True) + + +def _final_code(results: dict[str, str], code: int) -> tuple[list[str], int]: + """The failed section names, and the exit code they imply. + + Each failed section turns a would-be-0 exit into 1; a harder failure (the auth + stop's 4) keeps its own code. + """ + failed = [name for name, value in results.items() if value == SectionResult.FAILED.value] + if failed and code == 0: + code = 1 + return failed, code + + +def _failure_line(failed: list[str]) -> str: + noun = "issue" if len(failed) == 1 else "issues" + return f"Set up with {len(failed)} {noun} ({', '.join(failed)} failed)." + + +def _summarize(ctx: WizardContext, results: dict[str, str], code: int) -> int: + """Fold the per-section results into the closing output and final exit code. + + Under --json the summary is the one stdout payload; human runs get a closing + stderr line naming what failed. + """ + failed, code = _final_code(results, code) + if ctx.json_mode: + output.emit_ndjson( + {"ok": code == 0, "exit_code": code, "sections": results, "failed": failed} + ) + elif failed: + output.error_console.print(output.fail(_failure_line(failed))) + return code diff --git a/aai_cli/streaming/diagnostics.py b/aai_cli/streaming/diagnostics.py new file mode 100644 index 00000000..bb5a427e --- /dev/null +++ b/aai_cli/streaming/diagnostics.py @@ -0,0 +1,82 @@ +"""Hygiene helpers shared by the realtime session paths (stream, agent, speak). + +Two concerns every WebSocket-backed command shares, kept in one place so the +three paths fail identically: silencing library loggers that would dirty stderr +next to the CLI's own normalized error, and turning a rejected WebSocket +handshake (HTTP 401/403) into a CLIError that carries an actionable suggestion +instead of a bare status line. +""" + +from __future__ import annotations + +import logging + +from aai_cli import ws as wsutil +from aai_cli.errors import APIError, CLIError, NotAuthenticated + +# The assemblyai SDK's streaming client logs its own connection failures at ERROR +# ("Connection failed: WebSocket handshake rejected (HTTP 403) (code=403)") through +# child loggers of this one. The CLI configures no logging, so Python's last-resort +# handler would print that line to stderr right before the CLI's normalized error — +# a duplicate in human mode and a non-JSON line polluting --json stderr. The +# CLIError already carries the message, so the logger is raised above ERROR. +SDK_STREAMING_LOGGER = "assemblyai.streaming" + +# Handshake statuses that mean the server refused the connection outright. +_HANDSHAKE_AUTH_STATUSES = (401, 403) +_UNAUTHORIZED = 401 + + +def silence_streaming_logging() -> None: + """Silence the library loggers that would dirty stderr during a realtime run. + + Extends the shared websockets silencing (``aai_cli.ws``) with the assemblyai + SDK's streaming logger, which only the `stream` path uses. Idempotent. + """ + wsutil.silence_websockets_logging() + logging.getLogger(SDK_STREAMING_LOGGER).setLevel(logging.CRITICAL) + + +def handshake_suggestion(host: str) -> str: + """The actionable next steps for a rejected streaming handshake.""" + target = host or "the streaming endpoint" + return ( + "Check 'assembly whoami', that your key matches this environment " + f"(--sandbox?), and network/proxy access to {target}." + ) + + +def _handshake_status(error: object) -> int | None: + """The HTTP status of a rejected WebSocket handshake (401/403), else None. + + Reads the two structured shapes only — the assemblyai SDK's StreamingError + carries the status on ``.code``; websockets' InvalidStatus carries it on + ``.response.status_code`` — never the message text. + """ + code = getattr(error, "code", None) + if code in _HANDSHAKE_AUTH_STATUSES: + return int(code) + status = getattr(getattr(error, "response", None), "status_code", None) + if status in _HANDSHAKE_AUTH_STATUSES: + return int(status) + return None + + +def handshake_error(error: object, message: str, *, host: str) -> CLIError | None: + """An auth-flavored CLIError for a handshake 401/403, else None. + + 401 means the credential itself was rejected -> NotAuthenticated (exit 4, + ``rejected_key=True`` so auto-login won't retry an env-provided key). 403 + stays an APIError — it also covers WAF/region/plan blocks, mirroring + ``aai_cli.ws.is_rejected_key`` — but now carries the same suggestion. + """ + status = _handshake_status(error) + if status is None: + return None + if status == _UNAUTHORIZED: + return NotAuthenticated( + f"{message}: {error}", + suggestion=handshake_suggestion(host), + rejected_key=True, + ) + return APIError(f"{message}: {error}", suggestion=handshake_suggestion(host)) diff --git a/aai_cli/telemetry.py b/aai_cli/telemetry.py index 85360175..f6ee7efb 100644 --- a/aai_cli/telemetry.py +++ b/aai_cli/telemetry.py @@ -74,11 +74,67 @@ def consent_granted() -> bool: return config.get_telemetry_enabled() is not False +def consent_source() -> str: + """Which layer decided :func:`consent_granted`, in the order that layer wins: + an env kill-switch (``env:AAI_TELEMETRY_DISABLED`` / ``env:DO_NOT_TRACK``), the + choice persisted by ``assembly telemetry enable/disable`` (``config``), or the + opt-out ``default``.""" + if os.environ.get(ENV_DISABLED): + return f"env:{ENV_DISABLED}" + if os.environ.get(ENV_DO_NOT_TRACK): + return f"env:{ENV_DO_NOT_TRACK}" + if config.get_telemetry_enabled() is not None: + return "config" + return "default" + + def is_enabled() -> bool: """Telemetry runs only with both a token to send with and consent to send.""" return bool(client_token()) and consent_granted() +FIRST_RUN_NOTICE = ( + "Anonymous usage data is collected to improve the CLI; opt out with " + "'assembly telemetry disable' (or DO_NOT_TRACK=1)." +) + + +def _notice_suppressed(raw_args: list[str]) -> bool: + """Whether the invocation asked for quiet or machine-readable output. + + The one-time disclosure is human-facing chrome: it must not decorate a + ``--quiet`` run nor pollute the machine-readable stderr a ``--json`` (or + ``-o json``) pipeline relies on. Mirrors ``main._command_line_requests_json`` + (telemetry can't import main without a cycle) plus the quiet flags. + """ + for index, token in enumerate(raw_args): + if token in ("--quiet", "-q", "--json", "-j", "--output=json", "-ojson"): + return True + if token in ("-o", "--output") and raw_args[index + 1 : index + 2] == ["json"]: + return True + return False + + +def _maybe_emit_first_run_notice() -> None: + """Disclose collection once, when the anonymous device id is first minted. + + Printed to stderr so stdout stays pipeline-clean. Minting the id here makes the + disclosure at-most-once-ever: every later run sees the persisted id and stays + silent (including when the first run suppressed the line via --quiet/--json). + Wrapped like every other telemetry side effect — a config failure must never + break the command being recorded. + """ + try: + if config.has_device_id(): + return + config.get_device_id() + if _notice_suppressed(sys.argv[1:]): + return + sys.stderr.write(FIRST_RUN_NOTICE + "\n") + except (OSError, CLIError): + return + + def build_event( command: str, *, @@ -205,6 +261,7 @@ def track(command: str) -> Generator[None]: if not is_enabled(): yield return + _maybe_emit_first_run_notice() started = time.monotonic() try: yield diff --git a/aai_cli/transcribe_exec.py b/aai_cli/transcribe_exec.py index 800b8998..c457b3fb 100644 --- a/aai_cli/transcribe_exec.py +++ b/aai_cli/transcribe_exec.py @@ -8,6 +8,7 @@ from __future__ import annotations import json +import os import tempfile from pathlib import Path from typing import Any, NamedTuple @@ -57,6 +58,56 @@ def validate_out_with_llm(out: Path | None, llm_prompts: list[str] | None) -> No ) +def validate_out_path(out: Path | None) -> None: + """Reject an unusable ``--out`` up front, before the (billed, possibly long) + transcription runs — not after it finishes.""" + if out is None: + return + if ".." in out.parts: # reject path-traversal segments in --out + raise UsageError(f"--out path can't contain '..': {out}") + parent = out.parent + if not parent.is_dir(): + raise UsageError( + f"--out directory doesn't exist: {parent}", + suggestion="Create it first, or point --out at an existing directory.", + ) + if not os.access(parent, os.W_OK): + raise UsageError(f"--out directory isn't writable: {parent}") + + +def validate_json_with_output( + output_field: choices.TranscriptOutput | None, *, json_mode: bool +) -> None: + """``--json`` promises the full JSON payload (same as ``-o json``); any other + ``-o`` field contradicts it rather than silently winning.""" + if json_mode and output_field is not None and output_field is not choices.TranscriptOutput.json: + raise UsageError( + f"--json conflicts with -o {output_field.value}.", + suggestion="Drop --json, or use -o json for the full JSON payload.", + ) + + +def warn_unrecognized_extension(source: str | None, *, json_mode: bool, quiet: bool) -> None: + """Warn when a single local source doesn't carry a known audio extension. + + Directory batch mode filters by ``AUDIO_EXTENSIONS``; single-file mode uploads + anything, so a likely-non-audio file (e.g. ``.txt``) gets a stderr heads-up — + never an error, since the server is the truth about what it can transcribe. + """ + from aai_cli.transcribe_batch import AUDIO_EXTENSIONS # avoid a module-load cycle + + if quiet or not source or source.startswith(("http://", "https://")): + return + suffix = Path(source).suffix.lower() + if not suffix or suffix in AUDIO_EXTENSIONS: + return + output.emit_warning( + f"'{source}' has extension '{suffix}', which doesn't look like audio; " + "the API decides what it can transcribe.", + json_mode=json_mode, + ) + + def render_transform_steps(d: dict[str, Any]) -> str: """Human view of chained LLM-Gateway steps: the lone output, or each step labeled.""" steps = d["transform"]["steps"] @@ -139,8 +190,7 @@ def deliver_result( transform chain, or the default JSON/human render — first match wins.""" if out is not None: # Write a clean file artifact and confirm on stderr; stdout stays empty. - if ".." in out.parts: # reject path-traversal segments in --out - raise UsageError(f"--out path can't contain '..': {out}") + # The path itself was validated up front by validate_out_path. out.write_text(out_payload(transcript, output_field, json_mode=json_mode) + "\n") if not quiet: output.error_console.print(output.success(f"Saved to {escape(str(out))}")) diff --git a/aai_cli/tts/session.py b/aai_cli/tts/session.py index 8a6e26ee..ebde6e43 100644 --- a/aai_cli/tts/session.py +++ b/aai_cli/tts/session.py @@ -12,6 +12,7 @@ from aai_cli import environments from aai_cli import ws as wsutil from aai_cli.errors import APIError, CLIError +from aai_cli.streaming import diagnostics from aai_cli.tts import audio @@ -109,7 +110,12 @@ def _default_connect( def _open_ws(connect: _Connect, api_key: str, url: str) -> _WebSocket: - """Open the TTS socket, mapping a connect failure to a clean CLIError.""" + """Open the TTS socket, mapping a connect failure to a clean CLIError. + + A rejected handshake (HTTP 401/403) gets the shared actionable suggestion + (whoami / environment / network); anything else keeps the wsutil mapping. + """ + message = "Could not connect to the TTS service" try: return connect( url, @@ -117,7 +123,12 @@ def _open_ws(connect: _Connect, api_key: str, url: str) -> _WebSocket: max_size=None, ) except Exception as exc: - raise wsutil.auth_or_api_error(exc, "Could not connect to the TTS service") from exc + rejected = diagnostics.handshake_error( + exc, message, host=environments.active().streaming_tts_host + ) + if rejected is not None: + raise rejected from exc + raise wsutil.auth_or_api_error(exc, message) from exc def _run_protocol( diff --git a/aai_cli/youtube.py b/aai_cli/youtube.py index 9b05e4dd..d91ecf89 100644 --- a/aai_cli/youtube.py +++ b/aai_cli/youtube.py @@ -29,6 +29,20 @@ )?""" ) +# yt-dlp appends report-a-bug boilerplate ("please report this issue on +# https://github.com/yt-dlp/yt-dlp/issues… Confirm you are on the latest version using +# yt-dlp -U") to most extractor errors; it isn't actionable for CLI users, so it is +# trimmed off before the message reaches our one clean error line. +_YTDLP_BUG_REPORT_RE = re.compile(r";?\s*please report this issue on .*", re.IGNORECASE | re.DOTALL) + + +def _ytdlp_error_message(exc: BaseException) -> str: + """The meaningful part of a yt-dlp failure: its message without the trailing + report-a-bug boilerplate or the redundant ``ERROR:`` prefix.""" + text = _YTDLP_BUG_REPORT_RE.sub("", str(exc)).strip() + return text.removeprefix("ERROR:").strip() or str(exc).strip() + + # yt-dlp's default logger prints its own "ERROR: …" line straight to stderr before the # CLI can raise its one clean error, duplicating the message. Route yt-dlp's output to # a swallow-everything logger (NullHandler, no propagation) instead. @@ -175,7 +189,7 @@ def download_audio(url: str, dest_dir: Path, *, download_sections: list[str] | N path = Path(ydl.prepare_filename(info)) except Exception as exc: # yt-dlp raises many types; surface one clean CLI error raise CLIError( - f"Could not download audio from {url}: {exc}", + f"Could not download audio from {url}: {_ytdlp_error_message(exc)}", error_type="youtube_error", exit_code=1, ) from exc diff --git a/pyproject.toml b/pyproject.toml index 58c0c840..a77e3013 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -105,6 +105,7 @@ dev = [ "import-linter>=2.3", "zizmor>=1.10", "coverage>=7.0", + "click>=8.1", ] [tool.uv] @@ -274,6 +275,7 @@ audioop-lts = "audioop" # The CLI templates carry their own requirements and are dependency-checked by # dedicated install tests, not by the root package metadata. DEP002 = ["fastapi", "python-dotenv", "python-multipart", "uvicorn"] -# coverage is read by scripts/mutation_gate.py (a dev-only gate run from check.sh, -# never shipped in the wheel), so deptry sees a dev dep imported from non-test code. -DEP004 = ["fastapi", "httpx", "hypothesis", "pytest", "coverage"] +# coverage is read by scripts/mutation_gate.py and click by +# scripts/generated_code_compile_gate.py (dev-only gates run from check.sh, never +# shipped in the wheel), so deptry sees dev deps imported from non-test code. +DEP004 = ["fastapi", "httpx", "hypothesis", "pytest", "coverage", "click"] diff --git a/scripts/generated_code_compile_gate.py b/scripts/generated_code_compile_gate.py index 7ffde0c5..a44e44ef 100644 --- a/scripts/generated_code_compile_gate.py +++ b/scripts/generated_code_compile_gate.py @@ -4,13 +4,19 @@ import sys from pathlib import Path -from typer.testing import CliRunner +import typer.main +from click.testing import CliRunner from aai_cli.main import app _ARG_COUNT = 2 _USAGE_EXIT = 2 +# Compile exactly what `assembly … --show-code > script.py` would capture: stdout +# only (stderr carries human chrome like warnings), with telemetry disabled so a +# gate run never mints a device id or spawns a flusher on the host. +_ENV = {"AAI_TELEMETRY_DISABLED": "1"} + def _write_fixture( runner: CliRunner, @@ -18,9 +24,10 @@ def _write_fixture( name: str, args: tuple[str, ...], ) -> None: - result = runner.invoke(app, list(args)) + command = typer.main.get_command(app) + result = runner.invoke(command, list(args), env=_ENV) if result.exit_code != 0: - detail = result.output.strip() or str(result.exception) + detail = result.stderr.strip() or result.output.strip() or str(result.exception) raise RuntimeError(f"{name}: {' '.join(args)} failed: {detail}") code = result.output if not code.strip(): @@ -101,7 +108,7 @@ def main() -> int: ), ) - runner = CliRunner() + runner = CliRunner(mix_stderr=False) for name, args in cases: _write_fixture(runner, out_dir, name, args) diff --git a/tests/__snapshots__/test_cli_output_snapshots.ambr b/tests/__snapshots__/test_cli_output_snapshots.ambr index f1499568..fcc007a1 100644 --- a/tests/__snapshots__/test_cli_output_snapshots.ambr +++ b/tests/__snapshots__/test_cli_output_snapshots.ambr @@ -25,15 +25,13 @@ Have a live two-way voice conversation with an AssemblyAI voice agent. - Use headphones: the mic stays open while the agent speaks, so on speakers it - would - hear itself and loop. Pass an audio file/URL (or --sample) to speak a recorded - clip to - the agent instead of the microphone; the session then ends after the agent's - reply. + Use headphones: the mic stays open while the agent speaks, so on + speakers it would hear itself and loop. Pass an audio file/URL (or + --sample) to speak a recorded clip to the agent instead of the + microphone; the session then ends after the agent's reply. - This only runs a conversation in the terminal — it writes no code. To build - a voice agent app, run 'assembly init voice-agent' instead. + This only runs a conversation in the terminal — it writes no code. To + build a voice agent app, run 'assembly init voice-agent' instead. ╭─ Arguments ──────────────────────────────────────────────────────────────────╮ │ source [SOURCE] Audio file path or URL to speak to the agent. Omit │ @@ -96,12 +94,14 @@ List recent audit-log entries for your account. ╭─ Options ────────────────────────────────────────────────────────────────────╮ - │ --limit INTEGER How many entries to show. [default: 20] │ - │ --action TEXT Filter by raw action name. │ - │ --resource TEXT Filter by raw resource type. │ - │ --include-logins Show successful login events. │ - │ --json -j Output raw JSON. │ - │ --help Show this message and exit. │ + │ --limit INTEGER RANGE [x>=1] How many entries to show. │ + │ [default: 20] │ + │ --action TEXT Filter by raw action name. │ + │ --resource TEXT Filter by raw resource type. │ + │ --include-logins Show successful login │ + │ events. │ + │ --json -j Output raw JSON. │ + │ --help Show this message and exit. │ ╰──────────────────────────────────────────────────────────────────────────────╯ Examples @@ -159,6 +159,7 @@ │ --railway Deploy to Railway. │ │ --fly Deploy to Fly.io. │ │ --yes -y Skip the confirmation prompt. │ + │ --json -j Output raw JSON. │ │ --help Show this message and exit. │ ╰──────────────────────────────────────────────────────────────────────────────╯ @@ -290,8 +291,9 @@ │ columns, in seconds). │ │ --collar FLOAT RANGE [x>=0.0] DER forgiveness (seconds) │ │ around each reference │ - │ turn boundary. │ - │ [default: 1.0] │ + │ turn boundary (default: │ + │ 1.0; needs │ + │ --speaker-labels). │ │ --concurrency INTEGER RANGE [x>=1] How many items to │ │ transcribe at once │ │ (sequential by default). │ @@ -342,7 +344,9 @@ ╭─ Options ────────────────────────────────────────────────────────────────────╮ │ --no-install Scaffold only; don't install or launch. │ │ --no-open Install + launch, but don't open the browser. │ - │ --force Overwrite a non-empty target directory. │ + │ --force Overwrite a non-empty target directory │ + │ (overlays the template; files not in the │ + │ template are kept). │ │ --here Scaffold into the current directory. │ │ --port INTEGER Local server port. [default: 3000] │ │ --json -j Output raw JSON. │ @@ -479,26 +483,30 @@ │ prompt [PROMPT] The prompt to send to the model. │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Options ────────────────────────────────────────────────────────────────────╮ - │ --model TEXT LLM Gateway model. │ - │ [default: claude-haiku-4-5-20251001] │ - │ --transcript-id TEXT Inject this transcript's text into the │ - │ prompt. │ - │ --system TEXT Optional system prompt. │ - │ --follow -f Re-run the prompt over a growing │ - │ transcript piped on stdin, refreshing │ - │ the answer in place on every finalized │ - │ turn (e.g. assembly stream -o text | │ - │ assembly llm -f "summarize action │ - │ items as I talk"). Ctrl-C to stop. │ - │ --output -o [text|json] Print one field of the result: text │ - │ (just the answer, pipe-friendly) or │ - │ json. │ - │ --max-tokens INTEGER Max tokens to generate. │ - │ [default: 1000] │ - │ --list-models Print known models and exit. │ - │ --json -j Output raw JSON (one object per turn │ - │ in --follow mode). │ - │ --help Show this message and exit. │ + │ --model TEXT LLM Gateway model. │ + │ [default: │ + │ claude-haiku-4-5-20251001] │ + │ --transcript-id TEXT Inject this transcript's text │ + │ into the prompt. │ + │ --system TEXT Optional system prompt. │ + │ --follow -f Re-run the prompt over a │ + │ growing transcript piped on │ + │ stdin, refreshing the answer │ + │ in place on every finalized │ + │ turn (e.g. assembly stream -o │ + │ text | assembly llm -f │ + │ "summarize action items as I │ + │ talk"). Ctrl-C to stop. │ + │ --output -o [text|json] Print one field of the │ + │ result: text (just the │ + │ answer, pipe-friendly) or │ + │ json. │ + │ --max-tokens INTEGER RANGE [x>=1] Max tokens to generate. │ + │ [default: 1000] │ + │ --list-models Print known models and exit. │ + │ --json -j Output raw JSON (one object │ + │ per turn in --follow mode). │ + │ --help Show this message and exit. │ ╰──────────────────────────────────────────────────────────────────────────────╯ Examples @@ -617,10 +625,12 @@ List recent streaming sessions. ╭─ Options ────────────────────────────────────────────────────────────────────╮ - │ --limit INTEGER How many sessions to show. [default: 10] │ - │ --status TEXT Filter: created, completed, or error. │ - │ --json -j Output raw JSON. │ - │ --help Show this message and exit. │ + │ --limit INTEGER RANGE [x>=1] How many sessions to show. │ + │ [default: 10] │ + │ --status [created|completed|error] Only show sessions with this │ + │ status. │ + │ --json -j Output raw JSON. │ + │ --help Show this message and exit. │ ╰──────────────────────────────────────────────────────────────────────────────╯ Examples @@ -762,27 +772,32 @@ Synthesize speech from text with AssemblyAI streaming TTS (sandbox only). - Plays the audio through your speakers by default, or writes a WAV with --out. - Speaker-labeled input (from 'assembly transcribe --speaker-labels') is - detected - automatically: the labels are stripped and each speaker gets a different - voice. This feature only exists in the sandbox today — run it as - 'assembly --sandbox speak' (--sandbox goes before the subcommand). + Plays the audio through your speakers by default, or writes a WAV with + --out. Speaker-labeled input (from 'assembly transcribe + --speaker-labels') is detected automatically: the labels are stripped + and each speaker gets a different voice. This feature only exists in + the sandbox today — run it as 'assembly --sandbox speak' (--sandbox + goes before the subcommand). ╭─ Arguments ──────────────────────────────────────────────────────────────────╮ │ text [TEXT] Text to speak. Omit to read from stdin. │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Options ────────────────────────────────────────────────────────────────────╮ - │ --voice TEXT Voice id, or SPEAKER=VOICE for diarized │ - │ input (repeatable, e.g. --voice A=jane). │ - │ --language TEXT Language of the text. [default: English] │ - │ --sample-rate INTEGER Output sample rate in Hz. Server default if │ - │ omitted. │ - │ --out PATH Write a WAV file instead of playing through │ - │ the speakers. │ - │ --json -j Emit JSON metadata about the synthesized │ - │ audio. │ - │ --help Show this message and exit. │ + │ --voice TEXT Voice id (e.g. jane, michael, │ + │ mary, paul, eve, george), or │ + │ SPEAKER=VOICE for diarized │ + │ input (repeatable, e.g. --voice │ + │ A=jane). │ + │ --language TEXT Language of the text. │ + │ [default: English] │ + │ --sample-rate INTEGER RANGE [x>=1] Output sample rate in Hz │ + │ (positive). Server default if │ + │ omitted. │ + │ --out PATH Write a WAV file instead of │ + │ playing through the speakers. │ + │ --json -j Emit JSON metadata about the │ + │ synthesized audio. │ + │ --help Show this message and exit. │ ╰──────────────────────────────────────────────────────────────────────────────╯ Examples @@ -830,13 +845,20 @@ │ --help Show this message and exit. │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Audio Capture ──────────────────────────────────────────────────────────────╮ - │ --sample-rate INTEGER Force a microphone capture rate in Hz │ - │ (default: device native). │ - │ --device INTEGER Microphone device index. │ - │ --system-audio macOS only: stream system/app audio and │ - │ microphone as separate sessions. │ - │ --system-audio-only macOS only: stream system/app audio │ - │ without the microphone. │ + │ --sample-rate INTEGER RANGE [x>=1] Audio rate in Hz │ + │ (positive): capture rate │ + │ for the mic, or the │ + │ declared rate of raw PCM on │ + │ stdin (default: device │ + │ native / 16000). │ + │ --device INTEGER Microphone device index. │ + │ --system-audio macOS only: stream │ + │ system/app audio and │ + │ microphone as separate │ + │ sessions. │ + │ --system-audio-only macOS only: stream │ + │ system/app audio without │ + │ the microphone. │ ╰──────────────────────────────────────────────────────────────────────────────╯ ╭─ Model & Language ───────────────────────────────────────────────────────────╮ │ --speech-model [universal-streaming-m Streaming speech model. │ @@ -1052,8 +1074,8 @@ with the full result, and a re-run skips sources already transcribed. Curated flags cover common features; --config KEY=VALUE and --config-file - reach - every other field. Analysis (summary, chapters, ...) renders in human mode. + reach every other field. Analysis (summary, chapters, ...) renders in human + mode. ╭─ Arguments ──────────────────────────────────────────────────────────────────╮ │ source [SOURCE] Audio file, URL, YouTube/podcast URL, or a │ @@ -1100,8 +1122,9 @@ │ language. │ │ --keyterms-prompt TEXT Boost a key term │ │ (repeatable). │ - │ --temperature FLOAT Speech model │ - │ temperature. │ + │ --temperature FLOAT RANGE Speech model temperature │ + │ [0.0<=x<=1.0] (0 most deterministic, 1 │ + │ least). │ │ --prompt TEXT Prompt to bias the │ │ speech model (supported │ │ models only). │ @@ -1157,7 +1180,7 @@ │ --custom-spelling-file FILE JSON map of custom │ │ spellings. │ │ --audio-start INTEGER RANGE [x>=0] Start offset in ms. │ - │ --audio-end INTEGER End offset in ms. │ + │ --audio-end INTEGER RANGE [x>=0] End offset in ms. │ │ --download-sections TEXT For a YouTube/podcast │ │ URL, download only part │ │ of the source (yt-dlp │ @@ -1286,7 +1309,7 @@ │ --start TEXT Start date (YYYY-MM-DD). Default: 30d │ │ ago. │ │ --end TEXT End date (YYYY-MM-DD). Default: today. │ - │ --window TEXT Window size, e.g. 'day' or 'month'. │ + │ --window TEXT Window size: 'day', 'week', or 'month'. │ │ --include-zero,--all Include zero-usage windows (matches │ │ --include-logins on `assembly audit`). │ │ --json -j Output raw JSON. │ diff --git a/tests/test_account_command.py b/tests/test_account_command.py index a2d1d5ee..20642a8c 100644 --- a/tests/test_account_command.py +++ b/tests/test_account_command.py @@ -1,5 +1,6 @@ import json +import pytest from typer.testing import CliRunner from aai_cli import config @@ -14,7 +15,7 @@ def _auth(): config.set_session("default", session_jwt="jwt", session_token="tok", account_id=42) -def _login_result(): +def _login_result(*, json_mode=False): return LoginResult( api_key="sk_from_oauth", session_jwt="jwt", session_token="tok", account_id=42 ) @@ -328,7 +329,7 @@ def test_usage_rejects_invalid_date(mocker): def test_usage_invalid_date_fails_before_session_resolution(monkeypatch, mocker): # Not logged in + a bad --start/--end: date validation must run before # resolve_session, so the user gets a fast exit-2 usage error, not a login flow. - def _no_login(): + def _no_login(**_kwargs): raise AssertionError("login flow must not start for an invalid date") monkeypatch.setattr("aai_cli.context._interactive_session", lambda: True) @@ -385,3 +386,56 @@ def test_format_usage_number_fractional_trims_trailing_zeros(): assert account._format_usage_number(1234.5) == "1,234.5" assert account._format_usage_number(0.000001) == "0.000001" assert account._format_usage_number(2.5000004) == "2.5" + + +def test_usage_rejects_end_before_start(monkeypatch, mocker): + # A reversed range is a fast exit-2 usage error before session resolution or + # any AMS call — even when not logged in. + monkeypatch.setattr("aai_cli.context._interactive_session", lambda: True) + monkeypatch.setattr( + "aai_cli.context.run_login_flow", + lambda **_: (_ for _ in ()).throw(AssertionError("login must not start")), + ) + get_usage = mocker.patch("aai_cli.commands.account.ams.get_usage", autospec=True) + result = runner.invoke(app, ["usage", "--start", "2026-06-01", "--end", "2026-01-01"]) + assert result.exit_code == 2 + assert "is before" in result.output + get_usage.assert_not_called() + + +def test_usage_allows_equal_start_and_end(mocker): + # A single-day range (end == start) is valid: pins the strict `<` in the check. + _auth() + get_usage = mocker.patch( + "aai_cli.commands.account.ams.get_usage", autospec=True, return_value={"usage_items": []} + ) + result = runner.invoke(app, ["usage", "--start", "2026-01-01", "--end", "2026-01-01", "--json"]) + assert result.exit_code == 0 + get_usage.assert_called_once() + + +def test_usage_rejects_unknown_window(monkeypatch, mocker): + # --window was free text silently misinterpreted server-side; now it's validated + # client-side, before session resolution or any AMS call. + monkeypatch.setattr("aai_cli.context._interactive_session", lambda: True) + monkeypatch.setattr( + "aai_cli.context.run_login_flow", + lambda **_: (_ for _ in ()).throw(AssertionError("login must not start")), + ) + get_usage = mocker.patch("aai_cli.commands.account.ams.get_usage", autospec=True) + result = runner.invoke(app, ["usage", "--window", "fortnight"]) + assert result.exit_code == 2 + assert "Invalid --window" in result.output + assert "day, week, month" in result.output + get_usage.assert_not_called() + + +@pytest.mark.parametrize("window", ["day", "week", "month"]) +def test_usage_accepts_each_known_window(mocker, window): + _auth() + get_usage = mocker.patch( + "aai_cli.commands.account.ams.get_usage", autospec=True, return_value={"usage_items": []} + ) + result = runner.invoke(app, ["usage", "--window", window, "--json"]) + assert result.exit_code == 0 + assert get_usage.call_args[0][3] == window # passed through to AMS unchanged diff --git a/tests/test_agent_command.py b/tests/test_agent_command.py index cc4a920f..de0ea243 100644 --- a/tests/test_agent_command.py +++ b/tests/test_agent_command.py @@ -15,7 +15,7 @@ def _invoke_split(args): return runner.invoke(app, args) -def _login_result(): +def _login_result(*, json_mode=False): return LoginResult( api_key="sk_from_oauth", session_jwt="jwt", session_token="tok", account_id=7 ) @@ -50,8 +50,10 @@ def test_list_voices_json_emits_machine_readable_array(monkeypatch): result = runner.invoke(app, ["agent", "--list-voices", "--json"]) assert result.exit_code == 0 voices = json.loads(result.output) - assert voices == VOICES # the whole list, as a machine-readable array - assert "ivy" in voices + # The whole catalog, each entry carrying its name and language group. + assert voices == [{"name": v.name, "language": v.language} for v in VOICES] + assert {"name": "ivy", "language": "English"} in voices + assert {"name": "arjun", "language": "Multilingual"} in voices def test_agent_unauthenticated_runs_login(monkeypatch): diff --git a/tests/test_agent_session_run.py b/tests/test_agent_session_run.py index 7118a533..2aebf5bd 100644 --- a/tests/test_agent_session_run.py +++ b/tests/test_agent_session_run.py @@ -246,7 +246,12 @@ def reject(url, **kwargs): _run_with_connect(reject) assert exc.value.error_type == "api_error" assert exc.value.exit_code == 1 + assert "Could not connect to the voice agent" in exc.value.message assert "HTTP 403" in exc.value.message + # The rejected handshake carries the actionable next steps, env host included. + assert exc.value.suggestion is not None + assert "assembly whoami" in exc.value.suggestion + assert "agents.assemblyai.com" in exc.value.suggestion def test_run_session_handshake_401_is_still_auth_failure(): @@ -257,6 +262,9 @@ def reject(url, **kwargs): with pytest.raises(NotAuthenticated) as exc: _run_with_connect(reject) assert exc.value.exit_code == 4 + assert exc.value.rejected_key is True + assert exc.value.suggestion is not None + assert "assembly whoami" in exc.value.suggestion def test_run_session_auth_worded_failure_is_still_auth_failure(): diff --git a/tests/test_agent_voices.py b/tests/test_agent_voices.py index 42b51a2c..7626fa02 100644 --- a/tests/test_agent_voices.py +++ b/tests/test_agent_voices.py @@ -1,26 +1,81 @@ +import dataclasses + +import pytest + from aai_cli.agent import voices +# The catalog pinned exactly, split by language group: --list-voices renders these +# groups and the names back the --voice typo check, so a drifted entry is a bug. +_ENGLISH_NAMES = [ + "ivy", + "james", + "tyler", + "winter", + "sam", + "mia", + "bella", + "david", + "jack", + "kyle", + "helen", + "martha", + "river", + "emma", + "victor", + "eleanor", + "sophie", + "oliver", +] +_MULTILINGUAL_NAMES = [ + "arjun", + "ethan", + "dmitri", + "lukas", + "lena", + "pierre", + "mina", + "ren", + "mei", + "joon", + "giulia", + "luca", + "lucia", + "hana", + "mateo", + "diego", +] + -def test_voices_includes_default(): - assert "ivy" in voices.VOICES +def test_voice_catalog_matches_known_groups(): + assert [v.name for v in voices.VOICES if v.language == voices.ENGLISH] == _ENGLISH_NAMES + assert [v.name for v in voices.VOICES if v.language == voices.MULTILINGUAL] == ( + _MULTILINGUAL_NAMES + ) + # Every voice belongs to exactly one of the two known groups. + assert {v.language for v in voices.VOICES} == {"English", "Multilingual"} -def test_voices_are_unique_and_nonempty(): - assert voices.VOICES - assert len(voices.VOICES) == len(set(voices.VOICES)) +def test_voice_entries_are_immutable(): + # frozen=True: the catalog is shared module state, so entries must not be mutable. + field = "name" # via setattr: a literal `voice.name = …` is rejected by type checkers + with pytest.raises(dataclasses.FrozenInstanceError): + setattr(voices.VOICES[0], field, "hacked") -def test_format_voice_list_mentions_voices(): - out = voices.format_voice_list() - assert "ivy" in out - assert "james" in out +def test_voice_names_are_unique_and_in_catalog_order(): + assert voices.VOICE_NAMES == _ENGLISH_NAMES + _MULTILINGUAL_NAMES + assert len(voices.VOICE_NAMES) == len(set(voices.VOICE_NAMES)) def test_default_voice_is_in_voices(): - assert voices.DEFAULT_VOICE in voices.VOICES + assert voices.DEFAULT_VOICE == "ivy" + assert voices.DEFAULT_VOICE in voices.VOICE_NAMES -def test_format_voice_list_contains_all_voices(): - out = voices.format_voice_list() - for v in voices.VOICES: - assert v in out +def test_format_voice_list_groups_by_language(): + blocks = voices.format_voice_list().split("\n\n") + assert [block.splitlines()[0] for block in blocks] == ["English:", "Multilingual:"] + english, multilingual = blocks + # Names are indented under their group header, one per line, in catalog order. + assert english.splitlines()[1:] == [f" {name}" for name in _ENGLISH_NAMES] + assert multilingual.splitlines()[1:] == [f" {name}" for name in _MULTILINGUAL_NAMES] diff --git a/tests/test_audit_command.py b/tests/test_audit_command.py index 0ca2befb..33b7711b 100644 --- a/tests/test_audit_command.py +++ b/tests/test_audit_command.py @@ -1,4 +1,5 @@ import json +import re from typer.testing import CliRunner @@ -14,7 +15,7 @@ def _auth(): config.set_session("default", session_jwt="jwt", session_token="tok", account_id=42) -def _login_result(): +def _login_result(*, json_mode=False): return LoginResult( api_key="sk_from_oauth", session_jwt="jwt", session_token="tok", account_id=42 ) @@ -184,3 +185,38 @@ def test_audit_without_session_runs_login(monkeypatch, mocker): assert config.get_session("default") == {"jwt": "jwt", "token": "tok"} logs.assert_not_called() assert "Run the same command again" in result.output + + +def test_audit_rejects_nonpositive_limit(mocker): + # --limit is min=1: zero/negative values are a fast click-level usage error. + _auth() + list_logs = mocker.patch("aai_cli.commands.audit.ams.list_audit_logs", autospec=True) + for bad in ("0", "-5"): + result = runner.invoke(app, ["audit", "--limit", bad]) + assert result.exit_code == 2 + # CI forces color on (Rich under GITHUB_ACTIONS), interleaving style codes + # mid-message, so assert on the color-free render (see test_help_rendering.py). + plain = re.sub(r"\x1b\[[0-9;]*m", "", result.output) + assert "--limit" in plain + list_logs.assert_not_called() + + +def test_audit_accepts_limit_one(mocker): + # The boundary value is allowed (pins min=1, not min=2). + _auth() + list_logs = mocker.patch( + "aai_cli.commands.audit.ams.list_audit_logs", autospec=True, return_value={"data": []} + ) + result = runner.invoke(app, ["audit", "--limit", "1", "--json"]) + assert result.exit_code == 0 + list_logs.assert_called_once_with("jwt", limit=1, action_taken=None, resource_type=None) + + +def test_audit_default_limit_is_20(mocker): + _auth() + list_logs = mocker.patch( + "aai_cli.commands.audit.ams.list_audit_logs", autospec=True, return_value={"data": []} + ) + result = runner.invoke(app, ["audit", "--json"]) + assert result.exit_code == 0 + list_logs.assert_called_once_with("jwt", limit=20, action_taken=None, resource_type=None) diff --git a/tests/test_auth_flow.py b/tests/test_auth_flow.py index 19ddfaf4..ebc176cd 100644 --- a/tests/test_auth_flow.py +++ b/tests/test_auth_flow.py @@ -99,7 +99,7 @@ def test_run_login_flow_opens_the_discovery_start_url(monkeypatch): # The browser is opened with exactly the URL build_start_url() produces. seen = {} monkeypatch.setattr(flow.discovery, "build_start_url", lambda: "start-url") - monkeypatch.setattr(flow, "_open_browser", lambda url: seen.setdefault("url", url)) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: seen.setdefault("url", url)) _fake_start_capture( monkeypatch, loopback.CallbackResult(token="tok", token_type="discovery_oauth") ) @@ -123,7 +123,7 @@ def test_run_login_flow_opens_the_discovery_start_url(monkeypatch): def test_run_login_flow_rejects_wrong_token_type(monkeypatch): - monkeypatch.setattr(flow, "_open_browser", lambda url: None) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: None) _fake_start_capture( monkeypatch, loopback.CallbackResult(token="tok", token_type="something_else") ) @@ -134,7 +134,7 @@ def test_run_login_flow_rejects_wrong_token_type(monkeypatch): def test_run_login_flow_happy_path(monkeypatch): opened = {} - monkeypatch.setattr(flow, "_open_browser", lambda url: opened.setdefault("url", url)) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: opened.setdefault("url", url)) _fake_start_capture( monkeypatch, loopback.CallbackResult(token="tok", token_type="discovery_oauth") ) @@ -159,7 +159,7 @@ def test_run_login_flow_happy_path(monkeypatch): def test_run_login_flow_timeout_raises_auth_typed_error(monkeypatch): - monkeypatch.setattr(flow, "_open_browser", lambda url: None) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: None) _fake_start_capture(monkeypatch, loopback.CallbackResult(error="timeout")) with pytest.raises(NotAuthenticated) as exc: flow.run_login_flow() @@ -203,7 +203,7 @@ def test_find_or_create_creates_when_existing_token_has_no_api_key(monkeypatch): def test_run_login_flow_uses_exchange_account(monkeypatch): # The signed-in account comes from exchange()'s response; the flow must not make a # second round-trip to fetch it. - monkeypatch.setattr(flow, "_open_browser", lambda url: None) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: None) _fake_start_capture( monkeypatch, loopback.CallbackResult(token="tok", token_type="discovery_oauth") ) @@ -232,7 +232,7 @@ def fake_find(acct, jwt): def test_run_login_flow_multi_org_notes_selection(monkeypatch, capsys): - monkeypatch.setattr(flow, "_open_browser", lambda url: None) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: None) _fake_start_capture( monkeypatch, loopback.CallbackResult(token="tok", token_type="discovery_oauth") ) @@ -262,7 +262,7 @@ def test_run_login_flow_multi_org_notes_selection(monkeypatch, capsys): def test_open_browser_prints_fallback_to_stderr(monkeypatch, capsys): monkeypatch.setattr(flow.webbrowser, "open", lambda _url: (_ for _ in ()).throw(OSError())) - flow._open_browser("https://login.example") + flow._open_browser("https://login.example", json_mode=False) err = capsys.readouterr().err assert "https://login.example" in err @@ -270,7 +270,7 @@ def test_open_browser_prints_fallback_to_stderr(monkeypatch, capsys): def test_run_login_flow_missing_session_token_raises_api_error(monkeypatch): - monkeypatch.setattr(flow, "_open_browser", lambda url: None) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: None) _fake_start_capture( monkeypatch, loopback.CallbackResult(token="tok", token_type="discovery_oauth") ) @@ -284,7 +284,7 @@ def test_run_login_flow_missing_session_token_raises_api_error(monkeypatch): def test_run_login_flow_org_missing_id_raises_api_error(monkeypatch): - monkeypatch.setattr(flow, "_open_browser", lambda url: None) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: None) _fake_start_capture( monkeypatch, loopback.CallbackResult(token="tok", token_type="discovery_oauth") ) @@ -301,7 +301,7 @@ def test_run_login_flow_org_missing_id_raises_api_error(monkeypatch): def test_run_login_flow_zero_orgs_raises(monkeypatch): - monkeypatch.setattr(flow, "_open_browser", lambda url: None) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: None) _fake_start_capture( monkeypatch, loopback.CallbackResult(token="tok", token_type="discovery_oauth") ) @@ -319,7 +319,7 @@ def test_run_login_flow_zero_orgs_raises(monkeypatch): def test_run_login_flow_returns_session_material(monkeypatch): - monkeypatch.setattr(flow, "_open_browser", lambda url: None) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: None) _fake_start_capture( monkeypatch, loopback.CallbackResult(token="tok", token_type="discovery_oauth", error=None), @@ -379,7 +379,7 @@ def fake_start(): ) monkeypatch.setattr(flow, "_start_capture", fake_start) - monkeypatch.setattr(flow, "_open_browser", lambda url: order.append("browser")) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: order.append("browser")) _stub_ams_happy_path(monkeypatch) assert flow.run_login_flow().api_key == "sk_final" @@ -392,7 +392,7 @@ def fail_start(): monkeypatch.setattr(flow, "_start_capture", fail_start) opened = [] - monkeypatch.setattr(flow, "_open_browser", lambda url: opened.append(url)) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: opened.append(url)) with pytest.raises(APIError, match="callback server"): flow.run_login_flow() @@ -402,7 +402,7 @@ def fail_start(): def test_run_login_flow_prints_waiting_hint(monkeypatch, capsys): # Headless/slow logins must not sit in 120s of silence: the flow says it is # waiting and names the non-browser alternative. - monkeypatch.setattr(flow, "_open_browser", lambda url: None) + monkeypatch.setattr(flow, "_open_browser", lambda url, **_: None) _fake_start_capture( monkeypatch, loopback.CallbackResult(token="tok", token_type="discovery_oauth") ) @@ -412,3 +412,83 @@ def test_run_login_flow_prints_waiting_hint(monkeypatch, capsys): err = capsys.readouterr().err assert "Waiting up to 2 minutes" in err assert "assembly login --api-key" in err + assert '"hint"' not in err # json_mode defaults to False: prose, not JSON objects + + +def test_open_browser_warns_when_open_returns_false(monkeypatch, capsys): + # webbrowser.open returns False — without raising — on headless boxes; the user + # must still get the manual-URL fallback rather than 120s of silence. + monkeypatch.setattr(flow.webbrowser, "open", lambda _url: False) + flow._open_browser("https://login.example", json_mode=False) + err = capsys.readouterr().err + assert "Could not open a browser" in err + assert "https://login.example" in err + + +def test_open_browser_no_fallback_when_open_succeeds(monkeypatch, capsys): + monkeypatch.setattr(flow.webbrowser, "open", lambda _url: True) + flow._open_browser("https://login.example", json_mode=False) + err = capsys.readouterr().err + assert "Opening your browser" in err + assert "Could not open a browser" not in err + + +def test_open_browser_json_mode_emits_structured_hints(monkeypatch, capsys): + import json + + monkeypatch.setattr(flow.webbrowser, "open", lambda _url: False) + flow._open_browser("https://login.example", json_mode=True) + lines = [json.loads(line) for line in capsys.readouterr().err.strip().splitlines()] + assert len(lines) == 2 # every stderr line is machine-readable + assert "browser" in lines[0]["hint"] + assert lines[0]["url"] == "https://login.example" + assert "Could not open a browser" in lines[1]["hint"] + assert lines[1]["url"] == "https://login.example" + + +def test_run_login_flow_json_mode_keeps_stderr_machine_readable(monkeypatch, capsys): + import json + + monkeypatch.setattr(flow.webbrowser, "open", lambda _url: True) + _fake_start_capture( + monkeypatch, loopback.CallbackResult(token="tok", token_type="discovery_oauth") + ) + _stub_ams_happy_path(monkeypatch) + + assert flow.run_login_flow(json_mode=True).api_key == "sk_final" + lines = [json.loads(line) for line in capsys.readouterr().err.strip().splitlines()] + waiting = next(obj for obj in lines if "Waiting up to 2 minutes" in obj["hint"]) + assert "assembly login --api-key" in waiting["hint"] + assert "url" not in waiting # the url field only ships on the browser-open notes + opening = next(obj for obj in lines if "Opening your browser" in obj["hint"]) + assert opening["url"].startswith("https://") + + +def test_run_login_flow_multi_org_json_note(monkeypatch, capsys): + import json + + monkeypatch.setattr(flow.webbrowser, "open", lambda _url: True) + _fake_start_capture( + monkeypatch, loopback.CallbackResult(token="tok", token_type="discovery_oauth") + ) + monkeypatch.setattr( + flow.ams, + "discover", + lambda token: { + "organizations": [ + {"organization_id": "org_1", "organization_name": "Acme"}, + {"organization_id": "org_2", "organization_name": "Beta"}, + ], + "intermediate_session_token": "ist", + }, + ) + monkeypatch.setattr( + flow.ams, + "exchange", + lambda ist, org: {"account": {"id": 9}, "session_jwt": "jwt", "session_token": "t"}, + ) + monkeypatch.setattr(flow, "find_or_create_cli_key", lambda acct, jwt: "sk_final") + + assert flow.run_login_flow(json_mode=True).api_key == "sk_final" + lines = [json.loads(line) for line in capsys.readouterr().err.strip().splitlines()] + assert any("Acme" in obj["hint"] for obj in lines) # the chosen org is still named diff --git a/tests/test_client.py b/tests/test_client.py index 18f7b3e7..aaf4527b 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -121,6 +121,43 @@ def test_list_transcripts_supports_pydantic_v1_items(mocker): assert rows == [{"id": "t2", "status": "queued"}] +def test_list_transcripts_error_is_clean_without_request_repr_or_doubled_prefix(mocker): + # The generic _sdk_errors wrap must compact the httpx repr ("Request: <…>") + # and drop the SDK's own "failed to retrieve transcripts:" preamble, which + # doubled up with the wrapper's "Could not list transcripts:" prefix. + raw = ( + "failed to retrieve transcripts: \n" + "Reason: Host not in allowlist\n" + "Request: " + ) + T = mocker.patch.object(client.aai, "Transcriber", autospec=True) + T.return_value.list_transcripts.side_effect = aai.types.AssemblyAIError(raw) + with pytest.raises(APIError) as exc: + client.list_transcripts("sk") + assert exc.value.message == "Could not list transcripts: Reason: Host not in allowlist" + assert exc.value.suggestion == "Check your network and try again." + + +def test_get_transcript_error_strips_id_bearing_sdk_preamble(mocker): + mocker.patch.object( + client.aai.Transcript, + "get_by_id", + side_effect=RuntimeError("failed to retrieve transcript t_x: server exploded"), + ) + with pytest.raises(APIError) as exc: + client.get_transcript("sk", "t_x") + assert exc.value.message == "Could not fetch transcript t_x: server exploded" + + +def test_sdk_error_without_preamble_keeps_reason_verbatim(mocker): + # Reasons that don't carry the SDK's preamble pass through unchanged. + T = mocker.patch.object(client.aai, "Transcriber", autospec=True) + T.return_value.list_transcripts.side_effect = ValueError("connection reset by peer") + with pytest.raises(APIError) as exc: + client.list_transcripts("sk") + assert exc.value.message == "Could not list transcripts: connection reset by peer" + + def test_list_transcripts_auth_error_becomes_apierror(mocker): T = mocker.patch.object(client.aai, "Transcriber", autospec=True) T.return_value.list_transcripts.side_effect = aai.types.AssemblyAIError("nope") diff --git a/tests/test_client_streaming.py b/tests/test_client_streaming.py index fbee8635..3261e856 100644 --- a/tests/test_client_streaming.py +++ b/tests/test_client_streaming.py @@ -160,6 +160,72 @@ def stream(self, source): client.stream_audio("sk_bad", [b"\x00"], params=_stream_params()) +def test_stream_audio_handshake_403_event_carries_suggestion(monkeypatch): + # The SDK reports a rejected handshake as an Error event (StreamingError with the + # HTTP status on .code); the CLI must add the actionable next steps, not stop at + # "Streaming error: WebSocket handshake rejected (HTTP 403)". + from assemblyai.streaming.v3 import StreamingError + + class Handshake403Client(_FakeStreamingClient): + def stream(self, source): + from assemblyai.streaming.v3 import StreamingEvents + + self.handlers[StreamingEvents.Error]( + self, StreamingError("WebSocket handshake rejected (HTTP 403)", code=403) + ) + + monkeypatch.setattr(client, "StreamingClient", Handshake403Client) + with pytest.raises(APIError) as exc: + client.stream_audio("sk", [b"\x00"], params=_stream_params()) + assert exc.value.message == "Streaming error: WebSocket handshake rejected (HTTP 403)" + assert exc.value.suggestion is not None + assert "assembly whoami" in exc.value.suggestion + assert "--sandbox" in exc.value.suggestion + # The suggestion names the active environment's streaming host (production here). + assert "streaming.assemblyai.com" in exc.value.suggestion + + +def test_stream_audio_handshake_401_event_is_not_authenticated_with_suggestion(monkeypatch): + from assemblyai.streaming.v3 import StreamingError + + from aai_cli.errors import NotAuthenticated + + class Handshake401Client(_FakeStreamingClient): + def stream(self, source): + from assemblyai.streaming.v3 import StreamingEvents + + self.handlers[StreamingEvents.Error]( + self, StreamingError("WebSocket handshake rejected (HTTP 401)", code=401) + ) + + monkeypatch.setattr(client, "StreamingClient", Handshake401Client) + with pytest.raises(NotAuthenticated) as exc: + client.stream_audio("sk_bad", [b"\x00"], params=_stream_params()) + assert exc.value.exit_code == 4 + assert exc.value.rejected_key is True + assert exc.value.suggestion is not None + assert "assembly whoami" in exc.value.suggestion + + +def test_stream_audio_silences_sdk_and_websockets_loggers(monkeypatch): + # The streaming setup must raise the library loggers above ERROR so a reader-thread + # failure can't dump a duplicate log line or raw traceback next to the CLIError. + import logging + + names = ("assemblyai.streaming", "websockets", "websockets.client") + previous = {name: logging.getLogger(name).level for name in names} + for name in names: + logging.getLogger(name).setLevel(logging.NOTSET) + try: + monkeypatch.setattr(client, "StreamingClient", _FakeStreamingClient) + client.stream_audio("sk", [b"\x00"], params=_stream_params(), on_turn=lambda e: None) + for name in names: + assert logging.getLogger(name).level == logging.CRITICAL, name + finally: + for name, level in previous.items(): + logging.getLogger(name).setLevel(level) + + def test_stream_audio_mid_stream_error_becomes_apierror(monkeypatch): class StreamFails(_FakeStreamingClient): def stream(self, source): diff --git a/tests/test_completion.py b/tests/test_completion.py index 8c1f61b2..2b77199a 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -2,7 +2,7 @@ import typer -from aai_cli.agent.voices import VOICES, complete_voice +from aai_cli.agent.voices import VOICE_NAMES, complete_voice from aai_cli.llm import KNOWN_MODELS, complete_model from aai_cli.main import app @@ -43,11 +43,11 @@ def test_complete_model_unknown_prefix_returns_nothing(): def test_complete_voice_filters_by_prefix(): - prefix = VOICES[0][:2] + prefix = VOICE_NAMES[0][:2] suggestions = complete_voice(prefix) assert suggestions assert all(v.startswith(prefix) for v in suggestions) def test_complete_voice_empty_prefix_returns_all(): - assert complete_voice("") == VOICES + assert complete_voice("") == VOICE_NAMES diff --git a/tests/test_config.py b/tests/test_config.py index d8a4e5cc..b93a179d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -405,3 +405,48 @@ def rejected(service, username, secret): assert "set ASSEMBLYAI_API_KEY instead" in exc.value.suggestion # The macOS path stays for keychain users. assert "security delete-generic-password -s assemblyai-cli" in exc.value.suggestion + + +def test_resolve_treats_whitespace_only_env_key_as_missing(monkeypatch): + # `export ASSEMBLYAI_API_KEY=' '` must read as "no key" (the clean exit-4 + # not-signed-in path), never reach httpx as an illegal header value. + monkeypatch.setenv("ASSEMBLYAI_API_KEY", " ") + with pytest.raises(NotAuthenticated): + config.resolve_api_key() + + +def test_resolve_strips_padding_from_env_key(monkeypatch): + monkeypatch.setenv("ASSEMBLYAI_API_KEY", " sk_padded \n") + assert config.resolve_api_key() == "sk_padded" + + +def test_resolve_treats_whitespace_only_keyring_key_as_missing(): + config.set_api_key("default", " ") + with pytest.raises(NotAuthenticated): + config.resolve_api_key() + + +def test_resolve_strips_padding_from_keyring_key(): + config.set_api_key("default", " sk_stored\n") + assert config.resolve_api_key() == "sk_stored" + + +def test_resolve_rejects_whitespace_only_flag(): + with pytest.raises(CLIError) as exc: + config.resolve_api_key(api_key_flag=" ") + assert exc.value.error_type == "invalid_key" + assert exc.value.exit_code == 2 + + +def test_resolve_strips_padding_from_flag(): + assert config.resolve_api_key(api_key_flag=" sk_flag ") == "sk_flag" + + +def test_validate_profile_public_wrapper(): + # Public so resolution-time callers (context.AppState) can fail fast on a typo'd + # --profile before any network work. + config.validate_profile("ok-name_1") # valid: no exception + with pytest.raises(CLIError) as exc: + config.validate_profile("bad name!") + assert exc.value.exit_code == 2 + assert exc.value.message.startswith("Invalid profile name") diff --git a/tests/test_context.py b/tests/test_context.py index 39460381..c9e3e5db 100644 --- a/tests/test_context.py +++ b/tests/test_context.py @@ -64,7 +64,7 @@ def test_run_command_skips_auto_login_when_session_not_interactive(monkeypatch): # and the ORIGINAL NotAuthenticated must surface with its actionable suggestion. monkeypatch.setattr( "aai_cli.context.run_login_flow", - lambda: (_ for _ in ()).throw(AssertionError("non-interactive must not auto-login")), + lambda **_: (_ for _ in ()).throw(AssertionError("non-interactive must not auto-login")), ) def body(state, json_mode): @@ -81,7 +81,7 @@ def body(state, json_mode): def test_run_command_not_interactive_json_keeps_clean_error_shape(monkeypatch): monkeypatch.setattr( "aai_cli.context.run_login_flow", - lambda: (_ for _ in ()).throw(AssertionError("non-interactive must not auto-login")), + lambda **_: (_ for _ in ()).throw(AssertionError("non-interactive must not auto-login")), ) def body(state, json_mode): @@ -102,7 +102,9 @@ def test_run_command_auto_login_notice_suppressed_in_json_mode(monkeypatch): _force_interactive(monkeypatch) monkeypatch.setattr( "aai_cli.context.run_login_flow", - lambda: LoginResult(api_key="sk_auto", session_jwt="j", session_token="t", account_id=1), + lambda **_: LoginResult( + api_key="sk_auto", session_jwt="j", session_token="t", account_id=1 + ), ) def body(state, json_mode): @@ -127,7 +129,7 @@ def test_run_command_auto_logs_in_and_asks_for_rerun(monkeypatch): _force_interactive(monkeypatch) monkeypatch.setattr( "aai_cli.context.run_login_flow", - lambda: LoginResult( + lambda **_: LoginResult( api_key="sk_auto", session_jwt="jwt_auto", session_token="tok_auto", @@ -155,7 +157,7 @@ def test_run_command_auto_login_persistence_failure_is_clean(monkeypatch): _force_interactive(monkeypatch) monkeypatch.setattr( "aai_cli.context.run_login_flow", - lambda: LoginResult( + lambda **_: LoginResult( api_key="sk_auto", session_jwt="jwt_auto", session_token="tok_auto", @@ -179,7 +181,7 @@ def body(state, json_mode): def test_run_command_auto_login_failure_is_clean(monkeypatch): _force_interactive(monkeypatch) - def fail_login(): + def fail_login(**_kwargs): raise APIError("Login failed: the server returned an unexpected response.") monkeypatch.setattr("aai_cli.context.run_login_flow", fail_login) @@ -197,7 +199,7 @@ def test_run_command_auto_login_timeout_maps_to_auth_error(monkeypatch): # generic api_error. _force_interactive(monkeypatch) - def fail_login(): + def fail_login(**_kwargs): raise NotAuthenticated("Login timed out waiting for the browser.") monkeypatch.setattr("aai_cli.context.run_login_flow", fail_login) @@ -217,7 +219,7 @@ def test_run_command_skips_auto_login_for_rejected_env_key(monkeypatch): monkeypatch.setenv(config.ENV_API_KEY, "sk_bad") monkeypatch.setattr( "aai_cli.context.run_login_flow", - lambda: (_ for _ in ()).throw(AssertionError("env key retry cannot be fixed")), + lambda **_: (_ for _ in ()).throw(AssertionError("env key retry cannot be fixed")), ) def body(state, json_mode): @@ -341,7 +343,7 @@ def test_run_command_auto_logs_in_when_env_key_set_but_error_is_not_a_rejection( monkeypatch.setenv(config.ENV_API_KEY, "sk_env") ran = {"login": 0} - def fake_login(): + def fake_login(**_kwargs): ran["login"] += 1 return LoginResult(api_key="sk_auto", session_jwt="j", session_token="t", account_id=7) @@ -411,3 +413,49 @@ def body(state, json_mode): result = runner.invoke(_make_app(body), ["go"], standalone_mode=False) assert isinstance(result.exception, BrokenPipeError) + + +def test_resolve_session_suggestion_never_offers_api_key_env_var(): + # The inherited NotAuthenticated suggestion offers ASSEMBLYAI_API_KEY, which can + # never satisfy AMS session commands (they authenticate with the browser-session + # JWT, not the API key). The session-specific suggestion must say what actually + # works and drop the dead-end env-var advice. + from aai_cli.context import resolve_session + + with pytest.raises(NotAuthenticated) as exc: + resolve_session(AppState()) + assert exc.value.suggestion is not None + assert "browser" in exc.value.suggestion + assert "API key alone" in exc.value.suggestion + 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.errors import CLIError + + with pytest.raises(CLIError) as exc: + AppState(profile="bad name!").resolve_profile() + assert exc.value.exit_code == 2 + assert exc.value.message.startswith("Invalid profile name") + + +def test_resolve_profile_returns_valid_explicit_profile(): + assert AppState(profile="staging").resolve_profile() == "staging" + + +def test_persist_browser_login_passes_json_mode_to_flow(monkeypatch): + from aai_cli.context import persist_browser_login + + seen = {} + + def fake(*, json_mode): + seen["json_mode"] = json_mode + return LoginResult(api_key="sk", session_jwt="j", session_token="t", account_id=1) + + monkeypatch.setattr("aai_cli.context.run_login_flow", fake) + persist_browser_login("default", "production") + assert seen["json_mode"] is False # human prose stays the default + persist_browser_login("default", "production", json_mode=True) + assert seen["json_mode"] is True # --json reaches the flow's stderr notes diff --git a/tests/test_deploy.py b/tests/test_deploy.py index 31f89858..70ea0d4e 100644 --- a/tests/test_deploy.py +++ b/tests/test_deploy.py @@ -188,9 +188,42 @@ def test_deploy_confirm_no_aborts(monkeypatch: pytest.MonkeyPatch) -> None: result = runner.invoke(app, ["deploy"]) assert result.exit_code == 0, result.output assert "Aborted" in result.output + # Human mode prints plain text, not the JSON shape. + assert '"status"' not in result.output assert _cmds(calls) == [] +def test_deploy_json_flag_is_accepted(monkeypatch: pytest.MonkeyPatch) -> None: + # deploy now has the standard --json flag like its init/dev/share siblings. + calls = _stub(monkeypatch, available=("vercel",)) + result = runner.invoke(app, ["deploy", "--yes", "--json"]) + assert result.exit_code == 0, result.output + assert "No such option" not in result.output + assert _cmds(calls) == [["vercel", "deploy"]] + + +def test_deploy_json_abort_is_machine_readable(monkeypatch: pytest.MonkeyPatch) -> None: + import json + + calls = _stub(monkeypatch, available=("vercel",), confirm=False) + result = runner.invoke(app, ["deploy", "--json"]) + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == {"status": "aborted", "target": "Vercel"} + assert "Aborted." not in result.stdout + assert _cmds(calls) == [] + + +def test_deploy_json_error_is_enveloped(monkeypatch: pytest.MonkeyPatch) -> None: + import json + + _stub(monkeypatch, available=()) + result = runner.invoke(app, ["deploy", "--yes", "--json"]) + assert result.exit_code == 1 + err = json.loads(result.stderr) + assert err["error"]["type"] == "missing_dependency" + assert "Vercel CLI" in err["error"]["message"] + + def test_deploy_yes_skips_prompt(monkeypatch: pytest.MonkeyPatch) -> None: calls = _stub(monkeypatch, available=("vercel",), confirm=False) result = runner.invoke(app, ["deploy", "--yes"]) diff --git a/tests/test_dev.py b/tests/test_dev.py index 80e888ba..9b51c555 100644 --- a/tests/test_dev.py +++ b/tests/test_dev.py @@ -171,6 +171,51 @@ def test_dev_server_nonzero_exit_propagates(tmp_path, monkeypatch): assert result.exit_code == 3 +def test_dev_busy_port_notice_on_stderr(tmp_path, monkeypatch): + # A busy --port silently substituting a neighbor would leave the user pointing + # tools at a dead port; the substitution is announced on stderr. + monkeypatch.chdir(tmp_path) + _make_project(tmp_path) + _stub_runner(monkeypatch) + monkeypatch.setattr("aai_cli.init.runner.find_free_port", lambda port, **k: port + 1) + result = runner.invoke(app, ["dev", "--no-open", "--port", "5000"]) + assert result.exit_code == 0, result.output + assert "Port 5000 is in use; using 5001." in result.stderr + assert "is in use" not in result.stdout # stderr-only: stdout stays pipeline-clean + + +def test_dev_no_port_notice_when_requested_port_is_free(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + _make_project(tmp_path) + _stub_runner(monkeypatch) # find_free_port returns the requested port + result = runner.invoke(app, ["dev", "--no-open", "--port", "5000"]) + assert result.exit_code == 0, result.output + assert "is in use" not in result.output + + +def test_dev_busy_port_notice_suppressed_by_quiet(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + _make_project(tmp_path) + _stub_runner(monkeypatch) + monkeypatch.setattr("aai_cli.init.runner.find_free_port", lambda port, **k: port + 1) + result = runner.invoke(app, ["--quiet", "dev", "--no-open", "--port", "5000"]) + assert result.exit_code == 0, result.output + assert "is in use" not in result.output + + +def test_dev_busy_port_notice_structured_in_json(tmp_path, monkeypatch): + import json + + monkeypatch.chdir(tmp_path) + _make_project(tmp_path) + _stub_runner(monkeypatch) + monkeypatch.setattr("aai_cli.init.runner.find_free_port", lambda port, **k: port + 1) + result = runner.invoke(app, ["dev", "--no-open", "--port", "5000", "--json"]) + assert result.exit_code == 0, result.output + warning = json.loads(result.stderr.strip().splitlines()[0]) + assert warning["warning"] == "Port 5000 is in use; using 5001." + + def test_dev_json_emits_install_step(tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) _make_project(tmp_path) diff --git a/tests/test_devserver.py b/tests/test_devserver.py index b832b5ac..92b6c757 100644 --- a/tests/test_devserver.py +++ b/tests/test_devserver.py @@ -153,3 +153,39 @@ def test_override_host_appends_when_absent(): def test_local_host_constant_is_loopback(): assert devserver.LOCAL_HOST == "127.0.0.1" + + +def test_notify_port_change_emits_warning_with_both_ports(monkeypatch): + calls = [] + monkeypatch.setattr( + "aai_cli.output.emit_warning", + lambda msg, *, json_mode: calls.append((msg, json_mode)), + ) + devserver.notify_port_change(5000, 5001, json_mode=True, quiet=False) + # json_mode passes through so --json runs get the structured {"warning": ...} line. + assert calls == [("Port 5000 is in use; using 5001.", True)] + + +def test_notify_port_change_silent_cases(monkeypatch): + calls = [] + monkeypatch.setattr( + "aai_cli.output.emit_warning", + lambda msg, *, json_mode: calls.append(msg), + ) + # Same port bound: nothing to announce. + devserver.notify_port_change(5000, 5000, json_mode=False, quiet=False) + # Port 0 means "any free port": the substitution is the requested behavior. + devserver.notify_port_change(0, 4242, json_mode=False, quiet=False) + # --quiet suppresses the notice. + devserver.notify_port_change(5000, 5001, json_mode=False, quiet=True) + assert calls == [] + + +def test_notify_port_change_human_mode_passthrough(monkeypatch): + calls = [] + monkeypatch.setattr( + "aai_cli.output.emit_warning", + lambda msg, *, json_mode: calls.append(json_mode), + ) + devserver.notify_port_change(5000, 5001, json_mode=False, quiet=False) + assert calls == [False] diff --git a/tests/test_doctor.py b/tests/test_doctor.py index 064cc659..a04fba6e 100644 --- a/tests/test_doctor.py +++ b/tests/test_doctor.py @@ -73,9 +73,11 @@ def test_doctor_rejected_key_fails(healthy, monkeypatch): assert result.exit_code == 1 api = _checks(result)["api-key"] assert api["status"] == "fail" - # 401 and 403 both mean "rejected" (proxies often answer 403), so the - # message must not claim a specific 401. - assert "rejected (HTTP 401/403)" in api["detail"] + # validate_key collapses every auth-shaped failure (401, 403, proxy "forbidden") + # to False, so the detail must not claim a status code that was never observed. + assert api["detail"] == "API key was rejected by the server." + assert "401" not in api["detail"] + assert "assembly login" in api["fix"] def test_doctor_network_error_is_a_failure(healthy, monkeypatch): diff --git a/tests/test_eval_command.py b/tests/test_eval_command.py index f56d4456..93310fd5 100644 --- a/tests/test_eval_command.py +++ b/tests/test_eval_command.py @@ -8,16 +8,12 @@ import dataclasses import json import re -import threading -from pathlib import Path from types import SimpleNamespace import pytest from typer.testing import CliRunner from aai_cli import config, eval_data -from aai_cli.commands import evaluate -from aai_cli.errors import APIError from aai_cli.main import app runner = CliRunner() @@ -124,6 +120,7 @@ def test_json_payload_shape(tmp_path, mocker): assert "der" not in payload assert payload["rows"][0] == {"item": "a.wav", "words": 2, "errors": 0, "wer": 0.0} assert payload["rows"][1] == {"item": "b.wav", "words": 2, "errors": 1, "wer": 0.5} + assert "failed" not in payload # only present when a row failed @pytest.mark.parametrize("model", ["universal-3-pro", "universal-2"]) @@ -351,81 +348,14 @@ def fake_status(message, *, json_mode, quiet): assert seen == ["[1/2] Transcribing a.wav…", "[2/2] Transcribing b.wav…"] -def test_concurrency_runs_items_at_once_and_keeps_dataset_order(tmp_path, mocker): - # With concurrency 2 and two items, both transcriptions must be in flight at - # once (the barrier times out otherwise), and the rows stay in dataset order - # no matter which finishes first. - _auth() - _write_wer_manifest(tmp_path) - barrier = threading.Barrier(2, timeout=10) - texts = {"a.wav": "hello there", "b.wav": "goodbye cow"} - - def fake_transcribe(api_key, audio, *, config): - barrier.wait() - return _transcript(texts[Path(audio).name]) - - mocker.patch( - "aai_cli.commands.evaluate.client.transcribe", autospec=True, side_effect=fake_transcribe - ) - result = runner.invoke(app, ["eval", "manifest.csv", "--concurrency", "2", "--json"]) - assert result.exit_code == 0 - payload = _payload_of(result) - assert [row["item"] for row in payload["rows"]] == ["a.wav", "b.wav"] - assert payload["rows"][0]["wer"] == 0.0 - assert payload["rows"][1]["wer"] == 0.5 - - -def test_concurrency_shows_one_pooled_status(tmp_path, mocker, monkeypatch): - _auth() - _write_wer_manifest(tmp_path) - _mock_transcribe(mocker, [_transcript("hello there"), _transcript("goodbye now")]) - seen = [] - - @contextlib.contextmanager - def fake_status(message, *, json_mode, quiet): - seen.append(message) - yield - - monkeypatch.setattr("aai_cli.commands.evaluate.output.status", fake_status) - assert runner.invoke(app, ["eval", "manifest.csv", "--concurrency", "2"]).exit_code == 0 - assert seen == ["Transcribing 2 items (concurrency 2)…"] - - -def test_concurrent_failure_drops_queued_items_and_fails_cleanly(tmp_path, mocker, monkeypatch): - # The abort path must shut the pool down with cancel_futures=True — that's what - # keeps one failure from burning an API call per queued item. Asserted on the - # shutdown call because which queued futures actually get dropped is a race - # (the test_transcribe_batch.py pattern). - _auth() - _write_wer_manifest(tmp_path) - seen = {} - real = evaluate.ThreadPoolExecutor - - class Capture(real): - def shutdown(self, wait=True, *, cancel_futures=False): - seen.setdefault("cancel_futures", cancel_futures) # first call wins - super().shutdown(wait=wait, cancel_futures=cancel_futures) - - monkeypatch.setattr(evaluate, "ThreadPoolExecutor", Capture) - _mock_transcribe(mocker, [APIError("rate limited"), APIError("rate limited")]) - result = runner.invoke(app, ["eval", "manifest.csv", "--concurrency", "2"]) - assert result.exit_code == 1 - assert "rate limited" in result.output - assert seen["cancel_futures"] is True - - -def test_concurrency_below_one_is_a_usage_error(tmp_path): - result = runner.invoke(app, ["eval", "org/ds", "--concurrency", "0"]) +def test_collar_without_speaker_labels_is_a_usage_error(mocker): + # Mirrors transcribe's --speakers-expected guard: a silently inert flag is a bug. + tx = mocker.patch("aai_cli.commands.evaluate.client.transcribe", autospec=True) + result = runner.invoke(app, ["eval", "org/ds", "--collar", "0.5"]) assert result.exit_code == 2 - - -def test_api_error_mid_run_fails_cleanly(tmp_path, mocker): - _auth() - _write_wer_manifest(tmp_path) - _mock_transcribe(mocker, [_transcript("hello there"), APIError("rate limited")]) - result = runner.invoke(app, ["eval", "manifest.csv"]) - assert result.exit_code == 1 - assert "rate limited" in result.output + assert "--collar only applies" in result.output + assert "Add --speaker-labels." in result.output + tx.assert_not_called() def test_missing_manifest_is_a_usage_failure(tmp_path): diff --git a/tests/test_eval_data_hf.py b/tests/test_eval_data_hf.py index f0c9b50c..ea38a59c 100644 --- a/tests/test_eval_data_hf.py +++ b/tests/test_eval_data_hf.py @@ -10,7 +10,7 @@ import httpx2 as httpx import pytest -from aai_cli import der, eval_data +from aai_cli import der, eval_data, eval_hf_api from aai_cli.errors import APIError, UsageError # ------------------------------------------------------- Hugging Face datasets @@ -23,7 +23,7 @@ def fake_client(*args, **kwargs): kwargs["transport"] = httpx.MockTransport(handler) return real_client(*args, **kwargs) - monkeypatch.setattr(eval_data.httpx, "Client", fake_client) + monkeypatch.setattr(eval_hf_api.httpx, "Client", fake_client) def _audio_cell(url="https://hf.example/audio/0.wav"): @@ -212,6 +212,40 @@ def test_hf_auth_failure_suggests_hf_token(monkeypatch, status): with pytest.raises(APIError) as exc: eval_data.load("org/gated", limit=1) assert str(status) in exc.value.message + assert "denied access" in exc.value.message + assert "gated" in exc.value.message # the response body is surfaced, not discarded + assert exc.value.suggestion is not None and "HF_TOKEN" in exc.value.suggestion + + +def test_hf_proxy_403_surfaces_detail_without_hf_token_hint(monkeypatch): + # A sandbox proxy block ("Host not in allowlist") is not a gated dataset; the + # body must be shown and the misleading HF_TOKEN hint withheld. + _patch_transport(monkeypatch, lambda request: httpx.Response(403, text="Host not in allowlist")) + with pytest.raises(APIError) as exc: + eval_data.load("org/ds", limit=1) + assert "denied access" in exc.value.message + assert "Host not in allowlist" in exc.value.message + assert exc.value.suggestion is None + + +def test_hf_401_with_empty_body_keeps_hf_token_hint(monkeypatch): + # No body to judge by -> fall back to the gated/private guess (and no ": " tail). + _patch_transport(monkeypatch, lambda request: httpx.Response(401)) + with pytest.raises(APIError) as exc: + eval_data.load("org/ds", limit=1) + assert exc.value.message.endswith("(HTTP 401)") + assert exc.value.suggestion is not None and "HF_TOKEN" in exc.value.suggestion + + +@pytest.mark.parametrize( + "detail", + ["This dataset is gated", "private dataset", "Authentication required", "Invalid token"], +) +def test_hf_auth_sounding_details_keep_hf_token_hint(monkeypatch, detail): + _patch_transport(monkeypatch, lambda request: httpx.Response(403, json={"error": detail})) + with pytest.raises(APIError) as exc: + eval_data.load("org/ds", limit=1) + assert detail in exc.value.message assert exc.value.suggestion is not None and "HF_TOKEN" in exc.value.suggestion diff --git a/tests/test_eval_data_manifest.py b/tests/test_eval_data_manifest.py index a83260cb..9709e57d 100644 --- a/tests/test_eval_data_manifest.py +++ b/tests/test_eval_data_manifest.py @@ -101,6 +101,28 @@ def test_manifest_audio_file_missing_names_resolved_path(tmp_path): assert exc.value.suggestion is not None and str(tmp_path) in exc.value.suggestion +def test_manifest_with_unsupported_suffix_rejected(tmp_path): + # A .parquet (or any non-.csv/.jsonl file) must name the real constraint, not + # fail as "line 1 is not valid JSON" from the JSONL fallback parser. + manifest = tmp_path / "data.parquet" + manifest.write_bytes(b"PAR1\x00not-json") + with pytest.raises(UsageError) as exc: + eval_data.load(str(manifest), limit=10) + assert "Manifests must be .csv or .jsonl" in exc.value.message + assert "data.parquet" in exc.value.message + assert "not valid JSON" not in exc.value.message + + +def test_manifest_with_no_recognized_audio_column_uses_an_article(tmp_path): + # Grammar: "an audio column", not "a audio column". + manifest = tmp_path / "m.csv" + manifest.write_text("wav,text\na.wav,hello\n", encoding="utf-8") + with pytest.raises(UsageError) as exc: + eval_data.load(str(manifest), limit=10) + assert "Could not find an audio column" in exc.value.message + assert exc.value.suggestion is not None and "--audio-column" in exc.value.suggestion + + def test_manifest_row_without_audio_value_reports_row_number(tmp_path): _write_audio(tmp_path, "a.wav") manifest = tmp_path / "m.csv" diff --git a/tests/test_eval_failures.py b/tests/test_eval_failures.py new file mode 100644 index 00000000..6797fd3a --- /dev/null +++ b/tests/test_eval_failures.py @@ -0,0 +1,217 @@ +"""`assembly eval` concurrency and failure-resilience behavior. + +Split out of test_eval_command.py to keep modules under the 500-line gate. +A bad row must not discard the other (paid) rows; only a rejected key or a +non-CLIError bug aborts the run (cancelling queued items in concurrent mode). +""" + +import contextlib +import json +import threading +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from aai_cli.commands import evaluate +from aai_cli.errors import APIError, auth_failure +from aai_cli.main import app +from tests.test_eval_command import ( + _auth, + _mock_transcribe, + _payload_of, + _transcript, + _write_wer_manifest, +) + +runner = CliRunner() + + +@pytest.fixture(autouse=True) +def workdir(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + + +def test_concurrency_runs_items_at_once_and_keeps_dataset_order(tmp_path, mocker): + # With concurrency 2 and two items, both transcriptions must be in flight at + # once (the barrier times out otherwise), and the rows stay in dataset order + # no matter which finishes first. + _auth() + _write_wer_manifest(tmp_path) + barrier = threading.Barrier(2, timeout=10) + texts = {"a.wav": "hello there", "b.wav": "goodbye cow"} + + def fake_transcribe(api_key, audio, *, config): + barrier.wait() + return _transcript(texts[Path(audio).name]) + + mocker.patch( + "aai_cli.commands.evaluate.client.transcribe", autospec=True, side_effect=fake_transcribe + ) + result = runner.invoke(app, ["eval", "manifest.csv", "--concurrency", "2", "--json"]) + assert result.exit_code == 0 + payload = _payload_of(result) + assert [row["item"] for row in payload["rows"]] == ["a.wav", "b.wav"] + assert payload["rows"][0]["wer"] == 0.0 + assert payload["rows"][1]["wer"] == 0.5 + + +def test_concurrency_shows_one_pooled_status(tmp_path, mocker, monkeypatch): + _auth() + _write_wer_manifest(tmp_path) + _mock_transcribe(mocker, [_transcript("hello there"), _transcript("goodbye now")]) + seen = [] + + @contextlib.contextmanager + def fake_status(message, *, json_mode, quiet): + seen.append(message) + yield + + monkeypatch.setattr("aai_cli.commands.evaluate.output.status", fake_status) + assert runner.invoke(app, ["eval", "manifest.csv", "--concurrency", "2"]).exit_code == 0 + assert seen == ["Transcribing 2 items (concurrency 2)…"] + + +class _CapturingPool: + """Wrap ThreadPoolExecutor so a test can see how shutdown was called.""" + + def __init__(self, monkeypatch): + self.seen = {} + real = evaluate.ThreadPoolExecutor + capture = self + + class Capture(real): + def shutdown(self, wait=True, *, cancel_futures=False): + capture.seen.setdefault("cancel_futures", cancel_futures) # first call wins + super().shutdown(wait=wait, cancel_futures=cancel_futures) + + monkeypatch.setattr(evaluate, "ThreadPoolExecutor", Capture) + + +def test_concurrent_row_failure_keeps_other_rows_and_exits_nonzero(tmp_path, mocker, monkeypatch): + # A per-row APIError must not cancel the pool: the other (paid) rows finish + # and the run exits 1 with the failure tally, same as sequential mode. + _auth() + _write_wer_manifest(tmp_path) + pool = _CapturingPool(monkeypatch) + _mock_transcribe(mocker, [_transcript("hello there"), APIError("rate limited")]) + result = runner.invoke(app, ["eval", "manifest.csv", "--concurrency", "2", "--json"]) + assert result.exit_code == 1 + assert "1 of 2 items failed" in result.output + assert pool.seen["cancel_futures"] is False + payload = _payload_of(result) + assert payload["failed"] == 1 + errored = [row for row in payload["rows"] if "error" in row] + assert len(errored) == 1 and "rate limited" in errored[0]["error"] + assert any("wer" in row for row in payload["rows"]) # the good row was still scored + + +def test_concurrent_rejected_key_drops_queued_items(tmp_path, mocker, monkeypatch): + # One rejected key fails every row identically, so the abort path must shut + # the pool down with cancel_futures=True — that's what keeps it from burning + # an API call per queued item. Asserted on the shutdown call because which + # queued futures actually get dropped is a race (the test_transcribe_batch.py + # pattern). + _auth() + _write_wer_manifest(tmp_path) + pool = _CapturingPool(monkeypatch) + _mock_transcribe(mocker, [auth_failure(), auth_failure()]) + result = runner.invoke(app, ["eval", "manifest.csv", "--concurrency", "2"]) + assert result.exit_code == 4 + assert pool.seen["cancel_futures"] is True + + +def test_concurrent_internal_bug_drops_queued_items(tmp_path, mocker, monkeypatch): + # A non-CLIError is a bug, not a row outcome: re-raised (internal error), + # with queued items cancelled. + _auth() + _write_wer_manifest(tmp_path) + pool = _CapturingPool(monkeypatch) + _mock_transcribe(mocker, [RuntimeError("boom"), RuntimeError("boom")]) + result = runner.invoke(app, ["eval", "manifest.csv", "--concurrency", "2"]) + assert result.exit_code == 1 + assert "boom" in result.output + assert pool.seen["cancel_futures"] is True + + +def test_concurrency_below_one_is_a_usage_error(tmp_path): + result = runner.invoke(app, ["eval", "org/ds", "--concurrency", "0"]) + assert result.exit_code == 2 + + +def test_api_error_mid_run_fails_cleanly(tmp_path, mocker): + _auth() + _write_wer_manifest(tmp_path) + _mock_transcribe(mocker, [_transcript("hello there"), APIError("rate limited")]) + result = runner.invoke(app, ["eval", "manifest.csv"]) + assert result.exit_code == 1 + assert "rate limited" in result.output + + +def _write_three_row_manifest(tmp_path): + for name in ("a.wav", "b.wav", "c.wav"): + (tmp_path / name).write_bytes(b"fake-audio") + (tmp_path / "manifest.csv").write_text( + "audio,text\na.wav,hello there\nb.wav,goodbye now\nc.wav,see you\n", encoding="utf-8" + ) + + +def test_failed_row_keeps_completed_rows_and_summary_pools_scored_only(tmp_path, mocker): + # One bad row must not discard the completed (paid) rows: the run keeps going, + # the summary pools only the scored rows, and the exit code is nonzero. + _auth() + _write_three_row_manifest(tmp_path) + _mock_transcribe( + mocker, + [_transcript("hello there"), APIError("rate limited"), _transcript("see you")], + ) + result = runner.invoke(app, ["eval", "manifest.csv", "--json"]) + assert result.exit_code == 1 + payload = _payload_of(result) + assert payload["items"] == 3 + assert payload["failed"] == 1 + assert payload["rows"][0] == {"item": "a.wav", "words": 2, "errors": 0, "wer": 0.0} + assert payload["rows"][1] == {"item": "b.wav", "error": "rate limited"} + assert payload["rows"][2] == {"item": "c.wav", "words": 2, "errors": 0, "wer": 0.0} + # Pooled over the two scored rows only — the failed row contributes no words. + assert payload["words"] == 4 + assert payload["errors"] == 0 + assert payload["wer"] == 0.0 + err = next( + json.loads(line) for line in result.output.splitlines() if line.startswith('{"error"') + ) + assert err["error"]["type"] == "eval_failed" + assert "1 of 3 items failed" in err["error"]["message"] + + +def test_failed_row_renders_error_column_in_human_table(tmp_path, mocker): + _auth() + _write_wer_manifest(tmp_path) + _mock_transcribe(mocker, [_transcript("hello there"), APIError("rate limited")]) + result = runner.invoke(app, ["eval", "manifest.csv"]) + assert result.exit_code == 1 + assert "ERROR" in result.output # the failure column appears + assert "rate limited" in result.output # with the row's error + assert "0.00%" in result.output # and the scored row still renders + assert "1 of 2 items failed" in result.output + + +def test_rejected_key_aborts_eval_with_auth_exit_code(tmp_path, mocker): + # A rejected key fails every row identically — abort instead of billing through + # the whole dataset, and keep the auth exit code (4), not the eval-failed 1. + _auth() + _write_wer_manifest(tmp_path) + tx = _mock_transcribe(mocker, [auth_failure()]) + result = runner.invoke(app, ["eval", "manifest.csv"]) + assert result.exit_code == 4 + assert tx.call_count == 1 # aborted on the first row, no further billing + assert "rejected" in result.output + + +def test_unauthenticated_fails_before_dataset_download(mocker): + # Credentials resolve before the dataset loads: a signed-out user must not + # pull the whole dataset first. + load = mocker.patch("aai_cli.commands.evaluate.eval_data.load", autospec=True) + result = runner.invoke(app, ["eval", "org/ds"]) + assert result.exit_code == 4 + load.assert_not_called() diff --git a/tests/test_help_rendering.py b/tests/test_help_rendering.py index c467d816..431dc071 100644 --- a/tests/test_help_rendering.py +++ b/tests/test_help_rendering.py @@ -5,7 +5,9 @@ 80-column terminal, and unknown-flag suggestions must not leak a tuple repr. """ +import json import re +import sys import pytest from typer.testing import CliRunner @@ -53,6 +55,119 @@ def test_unknown_flag_without_suggestion_renders_plain(): plain = _plain(result.output) assert "No such option: --zzqq" in plain assert "Possible options" not in plain + # An unknown flag that isn't a misplaced global gets no placement hint either. + assert "global flag" not in plain + + +def _json_error(result): + """Parse the single {"error": …} envelope a JSON-mode failure emits on stderr.""" + line = next(line for line in result.output.splitlines() if line.startswith("{")) + return json.loads(line)["error"] + + +@pytest.mark.parametrize( + ("argv", "fragment"), + [ + (["transcripts", "get", "--json"], "Missing argument 'TRANSCRIPT_ID'"), + (["llm", "hi", "--max-tokens", "abc", "--json"], "not a valid integer"), + ], + ids=["missing-argument", "bad-option-value"], +) +def test_parse_error_with_json_emits_error_envelope(argv, fragment): + # Click-level parse errors must honor --json like root-callback failures do: + # a machine-readable {"error": …} envelope on stderr, exit code still 2. + result = runner.invoke(app, argv) + assert result.exit_code == 2 + error = _json_error(result) + assert error["type"] == "usage_error" + assert fragment in error["message"] + # The human Usage/panel chrome is replaced by the envelope, not added to it. + assert "Usage:" not in result.output + + +def test_parse_error_without_json_keeps_human_panel(): + result = runner.invoke(app, ["transcripts", "get"]) + assert result.exit_code == 2 + plain = _plain(result.output) + assert "Usage: assembly transcripts get" in plain + assert "Missing argument 'TRANSCRIPT_ID'" in plain + assert '{"error"' not in plain + + +def test_root_level_unknown_flag_gets_no_placement_hint(): + # Only the known misplaced flags earn a hint; a root-level typo stays a plain error. + result = runner.invoke(app, ["--zzqq", "transcribe", "x.wav"]) + assert result.exit_code == 2 + plain = _plain(result.output) + assert "No such option: --zzqq" in plain + assert "Pass --json after the subcommand" not in plain + + +def test_root_level_json_flag_points_at_subcommand_placement(): + # `--json` is a per-command flag; at root level the old similarity guess + # ("Possible options: --version") was actively misleading. And since the user + # asked for JSON, the error itself arrives as the JSON envelope. + result = runner.invoke(app, ["--json", "transcribe", "x.wav"]) + assert result.exit_code == 2 + error = _json_error(result) + assert error["type"] == "usage_error" + assert "No such option: --json" in error["message"] + assert "Pass --json after the subcommand: assembly --json" in error["message"] + assert "Possible options" not in error["message"] + + +@pytest.mark.parametrize( + ("argv", "expected"), + [ + (["doctor", "-q"], "assembly -q doctor"), + (["speak", "hi", "--sandbox"], "assembly --sandbox speak"), + ], + ids=["-q", "--sandbox"], +) +def test_global_flag_on_subcommand_points_at_root_placement(argv, expected): + result = runner.invoke(app, argv, env={"COLUMNS": "300"}) + assert result.exit_code == 2 + plain = _plain(result.output) + assert f"No such option: {argv[-1]}" in plain + assert "This is a global flag; pass it before the subcommand" in plain + assert expected in plain + + +def test_version_command_suggests_version_flag(): + # No `version` subcommand exists; the closest-match engine used to suggest the + # unrelated 'sessions'. Point at the real spelling instead. + result = runner.invoke(app, ["version"], env={"COLUMNS": "300"}) + assert result.exit_code == 2 + plain = _plain(result.output) + assert "Did you mean 'assembly --version'?" in plain + assert "sessions" not in plain + + +def test_misplaced_flag_hint_without_context_is_none(): + from typer._click.exceptions import NoSuchOption + + from aai_cli.main import _misplaced_flag_hint + + assert _misplaced_flag_hint(NoSuchOption("--json")) is None + + +def test_click_error_without_context_falls_back_to_argv(monkeypatch, capsys): + # A ClickException raised without a context has no stashed token list; the + # formatter then sniffs the real process argv for the JSON opt-in. + from typer._click.exceptions import ClickException + + from aai_cli.main import _format_click_error_fixed + + monkeypatch.setattr(sys, "argv", ["assembly", "--json"]) + _format_click_error_fixed(ClickException("boom")) + captured = capsys.readouterr().err + assert json.loads(captured) == {"error": {"type": "usage_error", "message": "boom"}} + + monkeypatch.setattr(sys, "argv", ["assembly"]) + _format_click_error_fixed(ClickException("boom")) + captured = capsys.readouterr().err + assert "boom" in captured + assert '{"error"' not in captured def test_noclip_table_pins_leading_columns_and_passes_row_args_through(): diff --git a/tests/test_init_command.py b/tests/test_init_command.py index 9f2a6bbb..6b1838ef 100644 --- a/tests/test_init_command.py +++ b/tests/test_init_command.py @@ -119,13 +119,6 @@ def test_init_refuses_nonempty_dir_without_force(tmp_path, monkeypatch): assert result.exit_code == 1 -def test_init_force_overwrites(tmp_path, monkeypatch): - monkeypatch.chdir(tmp_path) - assert runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install"]).exit_code == 0 - result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install", "--force"]) - assert result.exit_code == 0 - - def test_init_no_template_non_interactive_errors(tmp_path, monkeypatch): # CliRunner has no TTY, so the picker can't run; bare `assembly init` must error helpfully. monkeypatch.chdir(tmp_path) @@ -157,17 +150,27 @@ def test_init_prints_cli_banner_in_human_mode(tmp_path, monkeypatch): assert "AssemblyAI CLI" not in result.stdout -def test_init_banner_stays_off_stdout_on_error_paths(tmp_path, monkeypatch): - # The banner prints before template validation; an error run must still leave - # stdout empty (errors + banner are both stderr-only in human mode). +def test_init_banner_skipped_on_error_only_runs(tmp_path, monkeypatch): + # The banner prints only after validation passes: a pure error run (unknown + # template) stays undecorated like the sibling commands, and stdout stays empty. monkeypatch.chdir(tmp_path) monkeypatch.setattr("aai_cli.output.resolve_json", lambda *, explicit: False) result = runner.invoke(app, ["init", "nope", "x", "--no-install"]) assert result.exit_code == 1 - assert "AssemblyAI CLI" in result.stderr + assert "AssemblyAI CLI" not in result.stderr assert result.stdout == "" +def test_init_banner_skipped_on_target_conflict_error(tmp_path, monkeypatch): + # Target validation failures are error-only runs too: no banner. + monkeypatch.chdir(tmp_path) + monkeypatch.setattr("aai_cli.output.resolve_json", lambda *, explicit: False) + assert runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install"]).exit_code == 0 + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install"]) + assert result.exit_code == 1 + assert "AssemblyAI CLI" not in result.stderr + + def test_init_help_enumerates_template_names(): import re diff --git a/tests/test_init_force_and_report.py b/tests/test_init_force_and_report.py new file mode 100644 index 00000000..c3d0c5b6 --- /dev/null +++ b/tests/test_init_force_and_report.py @@ -0,0 +1,203 @@ +"""Tests for `assembly init` --force semantics, the key report row, and port hints. + +Split out of test_init_command.py to keep modules under the 500-line gate. +""" + +import subprocess + +from typer.testing import CliRunner + +from aai_cli.main import app + +runner = CliRunner() +TEMPLATE = "audio-transcription" + + +def test_init_force_overwrites(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + assert runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install"]).exit_code == 0 + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install", "--force"]) + assert result.exit_code == 0 + + +def test_init_target_is_existing_file_usage_error(tmp_path, monkeypatch): + # A target that exists but is a FILE is a clean usage error (exit 2), not the + # "Unexpected error: [Errno 17] File exists" internal-bug path mkdir would hit. + monkeypatch.chdir(tmp_path) + (tmp_path / "myapp").write_text("I am a file") + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install"]) + assert result.exit_code == 2 + assert "exists and is not a directory" in result.output + assert "Unexpected error" not in result.output + assert (tmp_path / "myapp").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. + monkeypatch.chdir(tmp_path) + monkeypatch.setattr("aai_cli.output.resolve_json", lambda *, explicit: False) + assert runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install"]).exit_code == 0 + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install", "--force"]) + assert result.exit_code == 0 + flat = " ".join(result.stderr.split()) + assert "overwriting existing files" in flat + assert "files not in the template are kept" in flat + assert "overwriting existing files" not in result.stdout + + +def test_init_force_no_overwrite_notice_for_fresh_target(tmp_path, monkeypatch): + # --force against a missing/empty target overwrites nothing, so no notice. + monkeypatch.chdir(tmp_path) + result = runner.invoke(app, ["init", TEMPLATE, "fresh", "--no-install", "--force"]) + assert result.exit_code == 0, result.output + assert "overwriting existing files" not in result.output + + +def test_init_force_overwrite_notice_is_structured_in_json(tmp_path, monkeypatch): + import json + + monkeypatch.chdir(tmp_path) + assert runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install"]).exit_code == 0 + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install", "--force", "--json"]) + assert result.exit_code == 0, result.output + warning = json.loads(result.stderr.strip().splitlines()[0]) + assert "overwriting existing files" in warning["warning"] + + +def test_init_force_preserves_configured_env_key(tmp_path, monkeypatch): + # A real key the user configured in .env must survive a keyless --force re-run + # (previously it was silently reset to the placeholder). + import json + + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("ASSEMBLYAI_API_KEY", "sk-configured") + assert runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install"]).exit_code == 0 + monkeypatch.delenv("ASSEMBLYAI_API_KEY") + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install", "--force", "--json"]) + assert result.exit_code == 0, result.output + assert "ASSEMBLYAI_API_KEY=sk-configured" in (tmp_path / "myapp" / ".env").read_text() + payload = json.loads(result.stdout) + key_row = next(s for s in payload if s["name"] == "key") + assert key_row["status"] == "kept" + assert "preserved" in key_row["detail"] + + +def test_init_force_over_placeholder_still_writes_placeholder(tmp_path, monkeypatch): + # Nothing worth preserving: a placeholder .env re-scaffolds to a placeholder + # with the usual skipped-key row. + import json + + monkeypatch.chdir(tmp_path) + assert runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install"]).exit_code == 0 + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install", "--force", "--json"]) + assert result.exit_code == 0, result.output + assert "your_assemblyai_api_key_here" in (tmp_path / "myapp" / ".env").read_text() + payload = json.loads(result.stdout) + key_row = next(s for s in payload if s["name"] == "key") + assert key_row["status"] == "skipped" + assert "no API key found" in key_row["detail"] + + +def test_init_force_with_preserved_key_still_launches(tmp_path, monkeypatch): + # The preserved .env key counts as having a key: deps install and the server + # launches, with no bogus "no API key" launch-skipped row. + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("ASSEMBLYAI_API_KEY", "sk-configured") + assert runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install"]).exit_code == 0 + monkeypatch.delenv("ASSEMBLYAI_API_KEY") + monkeypatch.setattr( + "aai_cli.init.runner.run_setup", + lambda *a, **k: subprocess.CompletedProcess([], 0, "", ""), + ) + monkeypatch.setattr("aai_cli.init.runner.find_free_port", lambda preferred: 4321) + launched = {"v": False} + monkeypatch.setattr( + "aai_cli.init.runner.launch_and_open", + lambda *a, **k: launched.__setitem__("v", True) or 0, + ) + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--force"]) + assert result.exit_code == 0, result.output + assert launched["v"] is True + assert "no API key" not in result.output + + +def test_init_reports_key_written_from_environment(tmp_path, monkeypatch): + import json + + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("ASSEMBLYAI_API_KEY", "sk-env") + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install", "--json"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + key_row = next(s for s in payload if s["name"] == "key") + assert key_row["status"] == "written" + assert key_row["detail"] == "from environment" + + +def test_init_reports_key_written_from_keyring(tmp_path, monkeypatch): + import json + + from aai_cli import config + + monkeypatch.chdir(tmp_path) + config.set_api_key("default", "sk-stored") + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install", "--json"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + key_row = next(s for s in payload if s["name"] == "key") + assert key_row["status"] == "written" + assert key_row["detail"] == "from keyring" + + +def test_init_blank_env_var_reports_keyring_source(tmp_path, monkeypatch): + # A whitespace-only ASSEMBLYAI_API_KEY is "unset" to the key chain, so a key + # that actually resolved from the keyring must not be attributed to the env. + import json + + from aai_cli import config + + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("ASSEMBLYAI_API_KEY", " ") + config.set_api_key("default", "sk-stored") + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install", "--json"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + key_row = next(s for s in payload if s["name"] == "key") + assert key_row["status"] == "written" + assert key_row["detail"] == "from keyring" + + +def test_init_no_install_hint_carries_custom_port(tmp_path, monkeypatch): + # `--no-install --port N` signs off with `assembly dev --port N`, not a bare + # `assembly dev` that would boot the default port instead. + monkeypatch.chdir(tmp_path) + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install", "--port", "5005"]) + assert result.exit_code == 0, result.output + packed = "".join(result.output.split()) # the hint line wraps on long tmp paths + assert "assemblydev--port5005" in packed + + +def test_init_no_install_hint_default_port_needs_no_flag(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + result = runner.invoke(app, ["init", TEMPLATE, "myapp", "--no-install"]) + assert result.exit_code == 0, result.output + packed = "".join(result.output.split()) + assert "assemblydev`" in packed + assert "--port" not in result.output + + +def test_init_launch_skipped_detail_carries_custom_port(tmp_path, monkeypatch): + # Logged out + install: the launch-skipped row's run command keeps the chosen port. + import json + + monkeypatch.chdir(tmp_path) + monkeypatch.setattr( + "aai_cli.init.runner.run_setup", + lambda *a, **k: subprocess.CompletedProcess([], 0, "", ""), + ) + result = runner.invoke(app, ["init", TEMPLATE, "app", "--port", "5005", "--json"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + launch_row = next(s for s in payload if s["name"] == "launch") + assert "assembly dev --port 5005" in launch_row["detail"] diff --git a/tests/test_init_keys.py b/tests/test_init_keys.py index e471a502..a1a3b060 100644 --- a/tests/test_init_keys.py +++ b/tests/test_init_keys.py @@ -4,14 +4,22 @@ def test_resolves_from_env(monkeypatch): monkeypatch.setenv("ASSEMBLYAI_API_KEY", "env-key-123") - assert keys.resolve_optional_api_key(profile=None) == "env-key-123" + assert keys.resolve_optional_api_key(profile=None) == ("env-key-123", "environment") def test_resolves_from_keyring(memory_keyring): config.set_api_key("default", "stored-key-456") - assert keys.resolve_optional_api_key(profile=None) == "stored-key-456" + assert keys.resolve_optional_api_key(profile=None) == ("stored-key-456", "keyring") + + +def test_env_wins_over_keyring_and_names_environment(monkeypatch, memory_keyring): + # Both sources set: the env var wins (mirrors resolve_api_key's chain), and the + # source says so — a "keyring" label here would point the user at the wrong place. + config.set_api_key("default", "stored-key-456") + monkeypatch.setenv("ASSEMBLYAI_API_KEY", "env-key-123") + assert keys.resolve_optional_api_key(profile=None) == ("env-key-123", "environment") def test_returns_none_when_absent(): # isolate_env strips the env var and memory_keyring starts empty. - assert keys.resolve_optional_api_key(profile=None) is None + assert keys.resolve_optional_api_key(profile=None) == (None, None) diff --git a/tests/test_init_runner.py b/tests/test_init_runner.py index 5eee5b8d..7b9cc8e9 100644 --- a/tests/test_init_runner.py +++ b/tests/test_init_runner.py @@ -275,3 +275,45 @@ def fake_popen(cmd, **kwargs): assert captured["cmd"] == ["uvicorn", "x"] assert captured["env"] == {"PORT": "3000"} assert captured["cwd"] == Path("/proj") + + +def _capture_stderr_prints(monkeypatch): + printed = [] + monkeypatch.setattr( + runner.output.error_console, + "print", + lambda *a, **k: printed.append(str(a[0]) if a else ""), + ) + return printed + + +def test_open_app_browser_hints_when_no_browser_can_launch(monkeypatch): + # webbrowser.open returns False on headless boxes; the user must be told where + # to point a browser instead of getting silence. + monkeypatch.setattr(runner.webbrowser, "open", lambda url: False) + printed = _capture_stderr_prints(monkeypatch) + runner.open_app_browser(4321) + assert any( + "Couldn't open a browser" in line and "http://localhost:4321" in line for line in printed + ) + + +def test_open_app_browser_silent_when_browser_opens(monkeypatch): + monkeypatch.setattr(runner.webbrowser, "open", lambda url: True) + printed = _capture_stderr_prints(monkeypatch) + runner.open_app_browser(4321) + assert printed == [] + + +def test_run_server_headless_browser_failure_prints_hint(monkeypatch): + # End to end through run_server: a False from webbrowser.open surfaces the hint. + proc = _FakeProc(returncode=0) + monkeypatch.setattr(runner.subprocess, "Popen", lambda *a, **k: proc) + monkeypatch.setattr(runner, "wait_for_port", lambda port: True) + monkeypatch.setattr(runner.webbrowser, "open", lambda url: False) + printed = _capture_stderr_prints(monkeypatch) + rc = runner.run_server( + Path("/proj"), command=["uvicorn", "x"], port=3000, env=None, open_browser=True + ) + assert rc == 0 + assert any("Couldn't open a browser" in line for line in printed) diff --git a/tests/test_init_scaffold.py b/tests/test_init_scaffold.py index 978ab950..18e33730 100644 --- a/tests/test_init_scaffold.py +++ b/tests/test_init_scaffold.py @@ -131,6 +131,33 @@ def test_scaffold_is_idempotent_over_existing_tree(tmp_path): assert "ASSEMBLYAI_API_KEY=k2" in (target / ".env").read_text() +def test_existing_env_key_none_when_env_missing(tmp_path): + assert scaffold.existing_env_key(tmp_path) is None + + +def test_existing_env_key_none_for_placeholder(tmp_path): + (tmp_path / ".env").write_text(f"ASSEMBLYAI_API_KEY={scaffold.PLACEHOLDER_KEY}\n") + assert scaffold.existing_env_key(tmp_path) is None + + +def test_existing_env_key_none_for_blank_value(tmp_path): + (tmp_path / ".env").write_text("ASSEMBLYAI_API_KEY=\n") + assert scaffold.existing_env_key(tmp_path) is None + + +def test_existing_env_key_none_when_key_line_absent(tmp_path): + (tmp_path / ".env").write_text("ASSEMBLYAI_BASE_URL=https://api.example\n") + assert scaffold.existing_env_key(tmp_path) is None + + +def test_existing_env_key_returns_configured_key(tmp_path): + # Other lines before/after the key line are skipped; trailing whitespace is trimmed. + (tmp_path / ".env").write_text( + "OTHER=1\nASSEMBLYAI_API_KEY=sk-configured \nASSEMBLYAI_BASE_URL=https://api.example\n" + ) + assert scaffold.existing_env_key(tmp_path) == "sk-configured" + + def test_target_conflict_detects_nonempty_dir(tmp_path): empty = tmp_path / "empty" empty.mkdir() diff --git a/tests/test_keys.py b/tests/test_keys.py index 006d74b7..53ff2bb6 100644 --- a/tests/test_keys.py +++ b/tests/test_keys.py @@ -14,7 +14,7 @@ def _auth(): config.set_session("default", session_jwt="jwt", session_token="tok", account_id=42) -def _login_result(): +def _login_result(*, json_mode=False): return LoginResult( api_key="sk_from_oauth", session_jwt="jwt", session_token="tok", account_id=42 ) @@ -207,3 +207,37 @@ def test_keys_create_with_explicit_project_skips_lookup(mocker): assert result.exit_code == 0 list_projects.assert_not_called() create.assert_called_once_with(42, 9, "ci", "jwt") + + +def test_keys_create_rejects_empty_name(monkeypatch, mocker): + # Local validation fires before session resolution or any AMS call — even when + # not logged in (no login flow may start for a request that can never be valid). + monkeypatch.setattr("aai_cli.context._interactive_session", lambda: True) + monkeypatch.setattr( + "aai_cli.context.run_login_flow", + lambda **_: (_ for _ in ()).throw(AssertionError("login must not start")), + ) + create = mocker.patch("aai_cli.commands.keys.ams.create_token", autospec=True) + result = runner.invoke(app, ["keys", "create", "--name", ""]) + assert result.exit_code == 2 + assert "must not be empty" in result.output + create.assert_not_called() + + +def test_keys_create_rejects_whitespace_only_name(mocker): + _auth() + create = mocker.patch("aai_cli.commands.keys.ams.create_token", autospec=True) + result = runner.invoke(app, ["keys", "create", "--name", " ", "--json"]) + assert result.exit_code == 2 + assert json.loads(result.output)["error"]["type"] == "usage_error" + create.assert_not_called() + + +def test_bare_keys_command_shows_subcommand_help(): + # `assembly keys` with no subcommand renders the sub-app help (no_args_is_help) + # instead of a bare "Missing command." error. + result = runner.invoke(app, ["keys"]) + assert "list" in result.output + assert "create" in result.output + assert "rename" in result.output + assert "Missing command" not in result.output diff --git a/tests/test_llm.py b/tests/test_llm.py index 41056119..ed410bda 100644 --- a/tests/test_llm.py +++ b/tests/test_llm.py @@ -72,17 +72,56 @@ def test_complete_auth_error_surfaces_gateway_message(monkeypatch): _fake_client(monkeypatch, error=err) with pytest.raises(APIError, match="access denied") as exc: llm.complete("sk", model="m", messages=[]) - # Access-denied is usually a plan entitlement block, so the hint names the cause. + # 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 + + +def test_complete_entitlement_denial_suggests_paid_plan(monkeypatch): + # The gateway's own entitlement block ("no access to LLM Gateway") is the one + # 401/403 where pointing at billing is right. + err = openai.PermissionDeniedError( + "Your account has no access to the LLM Gateway.", + response=httpx.Response(403, request=_REQUEST), + body=None, + ) + _fake_client(monkeypatch, error=err) + with pytest.raises(APIError, match="access denied") as exc: + llm.complete("sk", model="m", messages=[]) assert exc.value.suggestion is not None and "paid plan" in exc.value.suggestion -def test_complete_permission_error_surfaces_gateway_message(monkeypatch): +def test_complete_proxy_denial_does_not_suggest_paid_plan(monkeypatch): + # A corporate-proxy 403 says nothing about plans; sending the user to billing + # would mislead — they need to look at their key/network instead. err = openai.PermissionDeniedError( - "forbidden", response=httpx.Response(403, request=_REQUEST), body=None + "Host not in allowlist", response=httpx.Response(403, request=_REQUEST), body=None ) _fake_client(monkeypatch, error=err) - with pytest.raises(APIError, match="access denied"): + with pytest.raises(APIError, match="access denied") as exc: llm.complete("sk", model="m", messages=[]) + assert exc.value.suggestion == llm._ACCESS_DENIED_SUGGESTION + + +@pytest.mark.parametrize("hint", ["entitlement", "plan", "upgrade", "billing", "no access"]) +def test_denial_suggestion_matches_each_entitlement_hint(hint): + assert ( + llm._denial_suggestion(Exception(f"denied: {hint} required")) == llm._PAID_PLAN_SUGGESTION + ) + + +class _DenialWithBody(Exception): + def __init__(self, message: str, body: object) -> None: + super().__init__(message) + self.body = body + + +def test_denial_suggestion_reads_the_response_body_too(): + # The entitlement marker can live in the structured body rather than str(exc). + exc = _DenialWithBody("403 Forbidden", body={"error": "upgrade your plan"}) + assert llm._denial_suggestion(exc) == llm._PAID_PLAN_SUGGESTION def test_complete_bad_request_maps_to_api_error(monkeypatch): diff --git a/tests/test_llm_command.py b/tests/test_llm_command.py index b332009a..e89cb0c1 100644 --- a/tests/test_llm_command.py +++ b/tests/test_llm_command.py @@ -1,4 +1,5 @@ import json +import re import types from typer.testing import CliRunner @@ -15,7 +16,7 @@ def _auth(): config.set_api_key("default", "sk_live") -def _login_result(): +def _login_result(*, json_mode=False): return LoginResult( api_key="sk_from_oauth", session_jwt="jwt", session_token="tok", account_id=7 ) @@ -146,6 +147,90 @@ def fake_complete(api_key, *, model, messages, max_tokens, transcript_id=None): assert "{{ transcript }}" in seen["content"] +def test_llm_invalid_transcript_id_exits_2_without_network(monkeypatch): + # Same cheap local validation as `transcripts get`: a malformed id never + # reaches the gateway. + _auth() + monkeypatch.setattr( + "aai_cli.commands.llm.gateway.complete", + lambda *a, **k: (_ for _ in ()).throw(AssertionError("must not call the gateway")), + ) + result = runner.invoke(app, ["llm", "summarize", "--transcript-id", "not-a-real-id!!"]) + assert result.exit_code == 2 + assert "doesn't look like a transcript id" in result.output + + +def test_llm_max_tokens_must_be_at_least_one(monkeypatch): + # min=1 on --max-tokens: 0 and negatives are rejected client-side. + _auth() + monkeypatch.setattr( + "aai_cli.commands.llm.gateway.complete", + lambda *a, **k: (_ for _ in ()).throw(AssertionError("must not call the gateway")), + ) + for bad in ("0", "-5"): + result = runner.invoke(app, ["llm", "hi", "--max-tokens", bad]) + assert result.exit_code == 2 + # CI forces color on (Rich under GITHUB_ACTIONS), interleaving style codes + # mid-message, so assert on the color-free render (see test_help_rendering.py). + plain = re.sub(r"\x1b\[[0-9;]*m", "", result.output) + assert "max-tokens" in plain.lower() + + +def test_llm_transcript_id_warns_about_ignored_stdin(monkeypatch): + _auth() + monkeypatch.setattr("aai_cli.commands.llm.gateway.complete", lambda *a, **k: _payload("s")) + result = runner.invoke( + app, ["llm", "summarize", "--transcript-id", "t_9"], input="ignored stdin" + ) + assert result.exit_code == 0 + assert "Ignoring piped stdin; --transcript-id takes priority." in result.output + + +def test_llm_transcript_id_stdin_warning_is_machine_readable_in_json_mode(monkeypatch): + # In --json mode the warning must ship as its own {"warning": …} line (like the + # env-mismatch warning), keeping stderr machine-readable. + _auth() + monkeypatch.setattr("aai_cli.commands.llm.gateway.complete", lambda *a, **k: _payload("s")) + result = runner.invoke(app, ["llm", "summarize", "--transcript-id", "t_9", "--json"], input="x") + assert result.exit_code == 0 + objs = [json.loads(line) for line in result.output.splitlines() if line.strip()] + warning = next(o for o in objs if "warning" in o) + assert warning == {"warning": "Ignoring piped stdin; --transcript-id takes priority."} + payload = next(o for o in objs if "output" in o) + assert payload["output"] == "s" + + +def test_llm_transcript_id_stdin_warning_suppressed_by_quiet(monkeypatch): + _auth() + monkeypatch.setattr("aai_cli.commands.llm.gateway.complete", lambda *a, **k: _payload("s")) + result = runner.invoke( + app, ["--quiet", "llm", "summarize", "--transcript-id", "t_9"], input="x" + ) + assert result.exit_code == 0 + assert "Ignoring piped stdin" not in result.output + + +def test_llm_transcript_id_no_warning_when_stdin_is_a_terminal(monkeypatch): + _auth() + monkeypatch.setattr("aai_cli.commands.llm.stdio.stdin_is_piped", lambda: False) + monkeypatch.setattr("aai_cli.commands.llm.gateway.complete", lambda *a, **k: _payload("s")) + result = runner.invoke(app, ["llm", "summarize", "--transcript-id", "t_9"]) + assert result.exit_code == 0 + assert "Ignoring piped stdin" not in result.output + + +def test_llm_list_models_rejects_output_flag(monkeypatch): + # -o selects a field of a one-shot result; in --list-models mode it would be + # silently ignored, so reject it (mirrors how --follow rejects -o). + monkeypatch.setattr( + "aai_cli.commands.llm.gateway.complete", + lambda *a, **k: (_ for _ in ()).throw(AssertionError("must not call the gateway")), + ) + result = runner.invoke(app, ["llm", "--list-models", "-o", "json"]) + assert result.exit_code == 2 + assert "one-shot" in result.output + + def test_llm_missing_prompt_exits_2(monkeypatch): _auth() monkeypatch.setattr("aai_cli.commands.llm.gateway.complete", lambda *a, **k: _payload()) diff --git a/tests/test_login.py b/tests/test_login.py index c7756c15..b5e2b759 100644 --- a/tests/test_login.py +++ b/tests/test_login.py @@ -9,7 +9,7 @@ runner = CliRunner() -def _fake_login_result(key="sk_from_oauth"): +def _fake_login_result(key="sk_from_oauth", **_kwargs): return LoginResult(api_key=key, session_jwt="jwt_x", session_token="tok_x", account_id=7) @@ -148,7 +148,7 @@ def test_logout_clears_session(): def test_login_oauth_flow_failure_exits_nonzero(monkeypatch): from aai_cli.errors import APIError - def boom(): + def boom(**_kwargs): raise APIError("Login failed: the server returned an unexpected response.") monkeypatch.setattr("aai_cli.context.run_login_flow", boom) @@ -161,7 +161,7 @@ def test_login_timeout_is_auth_typed_with_exit_4(monkeypatch): # The loopback timeout surfaces as not_authenticated/exit 4, not api_error/1. from aai_cli.errors import NotAuthenticated - def timed_out(): + def timed_out(**_kwargs): raise NotAuthenticated("Login timed out waiting for the browser.") monkeypatch.setattr("aai_cli.context.run_login_flow", timed_out) @@ -177,7 +177,9 @@ def test_login_empty_api_key_flag_is_usage_error(monkeypatch): # into the browser flow. monkeypatch.setattr( "aai_cli.context.run_login_flow", - lambda: (_ for _ in ()).throw(AssertionError("empty --api-key must not start a browser")), + lambda **_: (_ for _ in ()).throw( + AssertionError("empty --api-key must not start a browser") + ), ) result = runner.invoke(app, ["login", "--api-key", ""]) assert result.exit_code == 2 @@ -188,7 +190,9 @@ def test_login_empty_api_key_flag_is_usage_error(monkeypatch): def test_login_whitespace_api_key_flag_is_usage_error(monkeypatch): monkeypatch.setattr( "aai_cli.context.run_login_flow", - lambda: (_ for _ in ()).throw(AssertionError("blank --api-key must not start a browser")), + lambda **_: (_ for _ in ()).throw( + AssertionError("blank --api-key must not start a browser") + ), ) result = runner.invoke(app, ["login", "--api-key", " "]) assert result.exit_code == 2 @@ -206,7 +210,7 @@ def test_login_empty_api_key_flag_json_error_shape(): def test_login_api_key_flag_still_bypasses_oauth(monkeypatch, mocker): monkeypatch.setattr( "aai_cli.context.run_login_flow", - lambda: (_ for _ in ()).throw(AssertionError("OAuth must not run with --api-key")), + lambda **_: (_ for _ in ()).throw(AssertionError("OAuth must not run with --api-key")), ) mocker.patch("aai_cli.commands.login.client.validate_key", autospec=True, return_value=True) result = runner.invoke(app, ["login", "--api-key", "sk_flag2"]) @@ -223,7 +227,7 @@ def test_login_binds_env_to_profile(monkeypatch): def test_sandbox_flag_is_shortcut_for_env(monkeypatch): - monkeypatch.setattr("aai_cli.context.run_login_flow", lambda: _fake_login_result("sk_x")) + monkeypatch.setattr("aai_cli.context.run_login_flow", lambda **_: _fake_login_result("sk_x")) result = runner.invoke(app, ["--sandbox", "login"]) assert result.exit_code == 0 assert config.get_profile_env("default") == "sandbox000" @@ -397,7 +401,7 @@ def test_login_failure_never_auto_logs_in_again(monkeypatch): calls = {"n": 0} - def timed_out(): + def timed_out(**_kwargs): calls["n"] += 1 raise NotAuthenticated("Login timed out waiting for the browser.") @@ -416,7 +420,7 @@ def test_logout_never_auto_logs_in(monkeypatch): monkeypatch.setattr("aai_cli.context._interactive_session", lambda: True) monkeypatch.setattr( "aai_cli.context.run_login_flow", - lambda: (_ for _ in ()).throw(AssertionError("logout must never start a login")), + lambda **_: (_ for _ in ()).throw(AssertionError("logout must never start a login")), ) monkeypatch.setattr( "aai_cli.commands.login.config.clear_api_key", @@ -424,3 +428,14 @@ def test_logout_never_auto_logs_in(monkeypatch): ) result = runner.invoke(app, ["logout"]) assert result.exit_code == 4 + + +def test_invalid_profile_name_fails_before_any_network(mocker): + # `assembly --profile 'bad name!' login --api-key X` used to validate the key over + # the network first and only reject the profile at keyring-write time; the root + # callback now rejects it at resolution time. + validate = mocker.patch("aai_cli.commands.login.client.validate_key", autospec=True) + result = runner.invoke(app, ["--profile", "bad name!", "login", "--api-key", "sk_x"]) + assert result.exit_code == 2 + assert "Invalid profile name" in result.output + validate.assert_not_called() diff --git a/tests/test_login_guards.py b/tests/test_login_guards.py new file mode 100644 index 00000000..28bc0378 --- /dev/null +++ b/tests/test_login_guards.py @@ -0,0 +1,151 @@ +"""Tests for login keyring preflight, truthful logout, and offline whoami. + +Split out of test_login.py to keep modules under the 500-line gate. +""" + +import json + +from typer.testing import CliRunner + +from aai_cli import config +from aai_cli.auth.flow import LoginResult +from aai_cli.main import app + +runner = CliRunner() + + +def _fake_login_result(key="sk_from_oauth", **_kwargs): + return LoginResult(api_key=key, session_jwt="jwt_x", session_token="tok_x", account_id=7) + + +def test_login_browser_flow_fails_fast_when_keyring_unusable(monkeypatch): + # A user must not complete the whole browser OAuth dance only to fail on the + # final keyring write: probe the keyring first and point at what works. + monkeypatch.setattr("aai_cli.commands.login.config.keyring_usable", lambda: False) + monkeypatch.setattr( + "aai_cli.context.run_login_flow", + lambda **_: (_ for _ in ()).throw(AssertionError("browser flow must not start")), + ) + result = runner.invoke(app, ["login"]) + assert result.exit_code == 2 + assert "keyring" in result.output + assert "ASSEMBLYAI_API_KEY" in result.output + + +def test_login_api_key_flow_fails_fast_when_keyring_unusable(monkeypatch, mocker): + # --api-key also stores to the keyring, so it gets the same preflight — before + # the key-validation network call. + monkeypatch.setattr("aai_cli.commands.login.config.keyring_usable", lambda: False) + validate = mocker.patch("aai_cli.commands.login.client.validate_key", autospec=True) + result = runner.invoke(app, ["login", "--api-key", "sk_x"]) + assert result.exit_code == 2 + validate.assert_not_called() + assert config.get_api_key("default") is None + + +def test_login_keyring_preflight_json_error_shape(monkeypatch): + monkeypatch.setattr("aai_cli.commands.login.config.keyring_usable", lambda: False) + result = runner.invoke(app, ["login", "--json"]) + assert result.exit_code == 2 + payload = json.loads(result.output) + assert payload["error"]["type"] == "keyring_unusable" + assert "ASSEMBLYAI_API_KEY" in payload["error"]["suggestion"] + + +def test_login_empty_api_key_error_wins_over_keyring_preflight(monkeypatch): + # An explicit empty --api-key is a usage error in its own right; it must be + # reported as such even when the keyring is also unusable. + monkeypatch.setattr("aai_cli.commands.login.config.keyring_usable", lambda: False) + result = runner.invoke(app, ["login", "--api-key", "", "--json"]) + assert result.exit_code == 2 + assert json.loads(result.output)["error"]["type"] == "usage_error" + + +def test_login_passes_json_mode_to_browser_flow(monkeypatch): + # --json must reach the flow so its stderr progress notes ship as {"hint": ...} + # objects (machine-readable stderr), not human prose. + seen = {} + + def fake(*, json_mode): + seen["json_mode"] = json_mode + return _fake_login_result() + + monkeypatch.setattr("aai_cli.context.run_login_flow", fake) + assert runner.invoke(app, ["login", "--json"]).exit_code == 0 + assert seen["json_mode"] is True + assert runner.invoke(app, ["login"]).exit_code == 0 + assert seen["json_mode"] is False + + +def test_logout_fresh_machine_reports_nothing_to_clear(): + # Idempotent (exit 0), but truthful: nothing was stored, so don't claim a + # sign-out happened. + result = runner.invoke(app, ["logout"]) + assert result.exit_code == 0 + assert "No stored credentials" in result.output + assert "nothing to clear" in result.output + assert "Signed out" not in result.output + + +def test_logout_json_reports_cleared_false_when_nothing_stored(): + result = runner.invoke(app, ["logout", "--json"]) + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["cleared"] is False + assert payload["logged_out"] is True + + +def test_logout_key_only_reports_cleared_true(): + config.set_api_key("default", "sk_1234567890") + result = runner.invoke(app, ["logout", "--json"]) + assert result.exit_code == 0 + assert json.loads(result.output)["cleared"] is True + + +def test_logout_session_only_reports_cleared_true(): + # A browser session with no API key still counts as stored credentials (pins + # `had_key or had_session`: an `and` would claim nothing was cleared). + config.set_session("default", session_jwt="j", session_token="t", account_id=7) + result = runner.invoke(app, ["logout"]) + assert result.exit_code == 0 + assert "Signed out of default" in result.output + + +def test_whoami_network_failure_still_renders_table(mocker): + # A network failure must not suppress the local identity table; the status is + # "unreachable (network error)" — distinct from "key rejected" — and the exit + # code is 1 (api_error), keeping 4 reserved for a key the server refused. + from aai_cli.errors import APIError + + config.set_api_key("default", "sk_1234567890") + config.set_session("default", session_jwt="j", session_token="t", account_id=77) + mocker.patch("aai_cli.output.resolve_json", autospec=True, return_value=False) + mocker.patch( + "aai_cli.commands.login.client.validate_key", + autospec=True, + side_effect=APIError("Network error contacting AssemblyAI: connection refused"), + ) + result = runner.invoke(app, ["whoami"]) + assert result.exit_code == 1 + assert "Profile" in result.output and "default" in result.output + assert "unreachable (network error)" in result.output + assert "key rejected" not in result.output + assert "not rejected" in result.output # the suggestion distinguishes it from a 401 + + +def test_whoami_network_failure_json_reachable_is_null(mocker): + from aai_cli.errors import APIError + + config.set_api_key("default", "sk_1234567890") + mocker.patch( + "aai_cli.commands.login.client.validate_key", + autospec=True, + side_effect=APIError("Network error contacting AssemblyAI: connection refused"), + ) + result = runner.invoke(app, ["whoami", "--json"]) + assert result.exit_code == 1 + objs = [json.loads(line) for line in result.output.strip().splitlines()] + payload = next(o for o in objs if "profile" in o and "error" not in o) + assert payload["reachable"] is None # "couldn't check", not a rejection + err = next(o for o in objs if "error" in o) + assert err["error"]["type"] == "api_error" diff --git a/tests/test_onboard_command.py b/tests/test_onboard_command.py index d6594112..f83fe904 100644 --- a/tests/test_onboard_command.py +++ b/tests/test_onboard_command.py @@ -31,6 +31,49 @@ def test_onboard_propagates_exit_code_one(monkeypatch: pytest.MonkeyPatch) -> No monkeypatch.setattr("aai_cli.commands.onboard.wizard.run_onboarding", lambda p, c: 1) result = CliRunner().invoke(app, ["onboard"]) assert result.exit_code == 1 + # Human mode exits plainly — no error envelope text. + assert "did not complete" not in result.output + + +def test_onboard_json_failure_emits_error_envelope(monkeypatch: pytest.MonkeyPatch) -> None: + import json + + monkeypatch.setattr("aai_cli.commands.onboard.wizard.run_onboarding", lambda p, c: 4) + result = CliRunner().invoke(app, ["onboard", "--json"]) + assert result.exit_code == 4 # the wizard's own code, not a generic 1 + err = json.loads(result.stderr.strip().splitlines()[-1]) + assert err["error"]["type"] == "onboarding_incomplete" + assert "did not complete" in err["error"]["message"] + + +def test_onboard_json_success_has_no_error_envelope(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("aai_cli.commands.onboard.wizard.run_onboarding", lambda p, c: 0) + result = CliRunner().invoke(app, ["onboard", "--json"]) + assert result.exit_code == 0, result.output + assert "onboarding_incomplete" not in result.output + + +def test_onboard_json_emits_machine_readable_summary(monkeypatch: pytest.MonkeyPatch) -> None: + # End to end: `assembly onboard --json` puts exactly one JSON document on stdout + # (the section summary) — previously it produced zero machine-readable output. + import json + + class _FakeTranscript: + id = "t_1" + status = "completed" + text = "hello" + utterances = None + + monkeypatch.setenv("ASSEMBLYAI_API_KEY", "sk_test") + monkeypatch.setattr( + "aai_cli.transcribe_exec.run_transcription", lambda *a, **k: _FakeTranscript() + ) + result = CliRunner().invoke(app, ["onboard", "--json"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) # parses only if stdout is a single JSON doc + assert payload["ok"] is True + assert payload["sections"]["sign-in"] == "skipped" # key already present + assert payload["failed"] == [] def test_onboard_does_not_auto_login_on_auth_error(monkeypatch: pytest.MonkeyPatch) -> None: @@ -122,6 +165,16 @@ def test_onboard_stays_interactive_without_flag_or_agent( assert captured["forced"] is False +def test_onboard_json_forces_noninteractive(monkeypatch: pytest.MonkeyPatch) -> None: + # --json forces non-interactive even with no agent detected: a machine-output run + # can't block on prompts (and the interactive prompter writes prose to stdout). + monkeypatch.setattr("aai_cli.output.is_agentic", lambda: False) + captured = _spy_forced(monkeypatch) + result = CliRunner().invoke(app, ["onboard", "--json"]) + assert result.exit_code == 0, result.output + assert captured["forced"] is True + + def test_onboard_sorts_first_in_quick_start() -> None: result = CliRunner().invoke(app, ["--help"]) assert result.output.index("onboard") < result.output.index("init") diff --git a/tests/test_onboard_environment.py b/tests/test_onboard_environment.py new file mode 100644 index 00000000..6f735cc5 --- /dev/null +++ b/tests/test_onboard_environment.py @@ -0,0 +1,121 @@ +"""Tests for the onboarding wizard's environment section summary. + +Split out of test_onboard_sections.py to keep modules under the 500-line gate. +""" + +from __future__ import annotations + +import pytest + +from aai_cli.context import AppState +from aai_cli.onboard import sections +from aai_cli.onboard.prompter import NonInteractivePrompter +from aai_cli.onboard.sections import SectionResult, WizardContext +from tests.test_onboard_sections import _capture_console, _ScriptedPrompter + + +@pytest.fixture +def ctx() -> WizardContext: + return WizardContext(state=AppState(), profile="default", json_mode=False) + + +def _patch_checks( + monkeypatch: pytest.MonkeyPatch, + *, + python: str = "ok", + ffmpeg: str = "ok", + audio: str = "ok", +) -> None: + """Pin the three doctor checks the wizard runs to the given statuses.""" + + def _mk(name: str, status: str): + fix = None if status == "ok" else f"fix the {name}" + check: dict[str, object] = { + "name": name, + "status": status, + "affects": [], + "detail": f"{name} detail", + "fix": fix, + } + return lambda: check + + monkeypatch.setattr("aai_cli.commands.doctor.check_python", _mk("python", python)) + monkeypatch.setattr("aai_cli.commands.doctor.check_ffmpeg", _mk("ffmpeg", ffmpeg)) + monkeypatch.setattr("aai_cli.commands.doctor.check_audio", _mk("audio", audio)) + + +def test_environment_all_ok_says_everything_looks_good( + ctx: WizardContext, monkeypatch: pytest.MonkeyPatch +) -> None: + _patch_checks(monkeypatch) + printed = _capture_console(monkeypatch) + assert sections.environment(NonInteractivePrompter(), ctx) is SectionResult.DONE + flat = "\n".join(printed) + assert "Everything looks good." in flat + assert "Ready —" not in flat + assert "found — see fixes above" not in flat + + +def test_environment_warnings_only_uses_soft_summary( + ctx: WizardContext, monkeypatch: pytest.MonkeyPatch +) -> None: + # A warning must not sit under a green "Everything looks good." — the summary is + # computed from the actual check statuses. + _patch_checks(monkeypatch, audio="warn") + printed = _capture_console(monkeypatch) + assert sections.environment(NonInteractivePrompter(), ctx) is SectionResult.DONE + flat = "\n".join(printed) + assert "Ready — 1 warning (only affects streaming/agent)." in flat + assert "Everything looks good" not in flat + assert "fix: fix the audio" in flat # per-check fix hints still render + + +def test_environment_two_warnings_pluralize( + ctx: WizardContext, monkeypatch: pytest.MonkeyPatch +) -> None: + _patch_checks(monkeypatch, ffmpeg="warn", audio="warn") + printed = _capture_console(monkeypatch) + assert sections.environment(NonInteractivePrompter(), ctx) is SectionResult.DONE + assert "Ready — 2 warnings (only affects streaming/agent)." in "\n".join(printed) + + +def test_environment_failed_check_fails_section( + ctx: WizardContext, monkeypatch: pytest.MonkeyPatch +) -> None: + _patch_checks(monkeypatch, python="fail", audio="warn") + printed = _capture_console(monkeypatch) + assert sections.environment(NonInteractivePrompter(), ctx) is SectionResult.FAILED + flat = "\n".join(printed) + assert "1 problem found — see fixes above." in flat + assert "Everything looks good" not in flat + + +def test_environment_two_failures_pluralize( + ctx: WizardContext, monkeypatch: pytest.MonkeyPatch +) -> None: + _patch_checks(monkeypatch, python="fail", ffmpeg="fail") + printed = _capture_console(monkeypatch) + assert sections.environment(NonInteractivePrompter(), ctx) is SectionResult.FAILED + assert "2 problems found — see fixes above." in "\n".join(printed) + + +def test_environment_human_mode_notes_streaming_caveat( + ctx: WizardContext, monkeypatch: pytest.MonkeyPatch +) -> None: + _patch_checks(monkeypatch) + _capture_console(monkeypatch) + prompter = _ScriptedPrompter() + assert sections.environment(prompter, ctx) is SectionResult.DONE + assert any("only affect live streaming" in note for note in prompter.notes) + + +def test_environment_json_mode_keeps_stdout_clean(monkeypatch: pytest.MonkeyPatch) -> None: + # Under --json the final summary owns stdout: no human render, no note — but the + # section result is still computed from the checks. + json_ctx = WizardContext(state=AppState(), profile="default", json_mode=True) + _patch_checks(monkeypatch, python="fail") + printed = _capture_console(monkeypatch) + prompter = _ScriptedPrompter() + assert sections.environment(prompter, json_ctx) is SectionResult.FAILED + assert printed == [] + assert prompter.notes == [] diff --git a/tests/test_onboard_sections.py b/tests/test_onboard_sections.py index 8d12025c..98198729 100644 --- a/tests/test_onboard_sections.py +++ b/tests/test_onboard_sections.py @@ -143,18 +143,12 @@ def _boom(*a: object, **k: object) -> _FakeTranscript: assert sections.first_request(_ScriptedPrompter(text="bad.mp3"), ctx) is SectionResult.FAILED -def test_environment_is_non_blocking(ctx: WizardContext, monkeypatch: pytest.MonkeyPatch) -> None: - # Even if checks warn/fail, the section never blocks the wizard. - seen: dict[str, object] = {} - - def _capture_render(payload: dict[str, object]) -> str: - seen.update(payload) - return "" - - monkeypatch.setattr("aai_cli.commands.doctor.render", _capture_render) - assert sections.environment(NonInteractivePrompter(), ctx) is SectionResult.DONE - # The environment section always renders as a non-fatal report (ok=True). - assert seen["ok"] is True +def _capture_console(monkeypatch: pytest.MonkeyPatch) -> list[str]: + printed: list[str] = [] + monkeypatch.setattr( + output.console, "print", lambda *a, **k: printed.append(str(a[0]) if a else "") + ) + return printed def test_build_path_skip_choice_does_nothing( @@ -173,8 +167,54 @@ def _fake_run_init(*a: object, **k: object) -> Path: assert called is False -def test_next_steps(ctx: WizardContext) -> None: +def test_next_steps(ctx: WizardContext, monkeypatch: pytest.MonkeyPatch) -> None: + printed = _capture_console(monkeypatch) assert sections.next_steps(NonInteractivePrompter(), ctx) is SectionResult.DONE + flat = "\n".join(printed) + # Human mode prints the three next-step hints. + assert "assembly transcribe" in flat + assert "assembly stream" in flat + assert "assembly init" in flat + + +def test_next_steps_json_mode_keeps_stdout_clean(monkeypatch: pytest.MonkeyPatch) -> None: + json_ctx = WizardContext(state=AppState(), profile="default", json_mode=True) + printed = _capture_console(monkeypatch) + assert sections.next_steps(NonInteractivePrompter(), json_ctx) is SectionResult.DONE + assert printed == [] + + +def test_first_request_json_mode_skips_human_transcript_render( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Under --json the wizard's summary owns stdout; the Rich transcript render + # would corrupt it. + json_ctx = WizardContext(state=AppState(), profile="default", json_mode=True) + monkeypatch.setenv("ASSEMBLYAI_API_KEY", "sk_test") + monkeypatch.setattr(transcribe_exec, "run_transcription", lambda *a, **k: _FakeTranscript()) + rendered = {"n": 0} + monkeypatch.setattr( + transcribe_render, + "render_transcript_result", + lambda *a, **k: rendered.__setitem__("n", rendered["n"] + 1), + ) + assert sections.first_request(NonInteractivePrompter(), json_ctx) is SectionResult.DONE + assert rendered["n"] == 0 + + +def test_first_request_human_mode_renders_transcript( + ctx: WizardContext, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("ASSEMBLYAI_API_KEY", "sk_test") + monkeypatch.setattr(transcribe_exec, "run_transcription", lambda *a, **k: _FakeTranscript()) + rendered = {"n": 0} + monkeypatch.setattr( + transcribe_render, + "render_transcript_result", + lambda *a, **k: rendered.__setitem__("n", rendered["n"] + 1), + ) + assert sections.first_request(NonInteractivePrompter(), ctx) is SectionResult.DONE + assert rendered["n"] == 1 def test_welcome_cold_start(ctx: WizardContext) -> None: diff --git a/tests/test_onboard_wizard.py b/tests/test_onboard_wizard.py index 101e77c8..ee360012 100644 --- a/tests/test_onboard_wizard.py +++ b/tests/test_onboard_wizard.py @@ -1,5 +1,7 @@ from __future__ import annotations +import json + import pytest from aai_cli import output @@ -8,12 +10,46 @@ from aai_cli.onboard.prompter import NonInteractivePrompter, WizardCancelled from aai_cli.onboard.sections import SectionResult, WizardContext +ALL_SECTIONS = ( + "welcome", + "auth", + "first_request", + "environment", + "build_path", + "claude_code", + "next_steps", + "launch_app", +) + @pytest.fixture def ctx() -> WizardContext: return WizardContext(state=AppState(), profile="default", json_mode=False) +@pytest.fixture +def json_ctx() -> WizardContext: + return WizardContext(state=AppState(), profile="default", json_mode=True) + + +def _patch_sections(monkeypatch: pytest.MonkeyPatch, **overrides: SectionResult) -> None: + """Stub every section to DONE, with per-section result overrides.""" + + def _const(result: SectionResult): + return lambda p, c: result + + for name in ALL_SECTIONS: + monkeypatch.setattr(sections, name, _const(overrides.get(name, SectionResult.DONE))) + + +def _capture_stderr(monkeypatch: pytest.MonkeyPatch) -> list[str]: + printed: list[str] = [] + monkeypatch.setattr( + output.error_console, "print", lambda *a, **k: printed.append(str(a[0]) if a else "") + ) + return printed + + def test_auth_failure_stops_the_wizard(ctx: WizardContext, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(sections, "welcome", lambda p, c: SectionResult.DONE) monkeypatch.setattr(sections, "auth", lambda p, c: SectionResult.FAILED) @@ -71,6 +107,105 @@ def _section(p: object, c: object) -> SectionResult: ] +def test_failed_section_exits_one_with_closing_line( + ctx: WizardContext, monkeypatch: pytest.MonkeyPatch +) -> None: + # A failed non-auth section must not end in a cheery exit 0: the run exits 1 and + # the closing line names what failed. + _patch_sections(monkeypatch, first_request=SectionResult.FAILED) + printed = _capture_stderr(monkeypatch) + assert wizard.run_onboarding(NonInteractivePrompter(), ctx) == 1 + assert any("Set up with 1 issue (first transcription failed)." in line for line in printed) + + +def test_two_failed_sections_pluralize_and_name_both( + ctx: WizardContext, monkeypatch: pytest.MonkeyPatch +) -> None: + _patch_sections( + monkeypatch, first_request=SectionResult.FAILED, claude_code=SectionResult.FAILED + ) + printed = _capture_stderr(monkeypatch) + assert wizard.run_onboarding(NonInteractivePrompter(), ctx) == 1 + assert any( + "Set up with 2 issues (first transcription, coding agent failed)." in line + for line in printed + ) + + +def test_clean_run_prints_no_closing_failure_line( + ctx: WizardContext, monkeypatch: pytest.MonkeyPatch +) -> None: + _patch_sections(monkeypatch) + printed = _capture_stderr(monkeypatch) + assert wizard.run_onboarding(NonInteractivePrompter(), ctx) == 0 + assert not any("Set up with" in line for line in printed) + + +def test_json_summary_on_success( + json_ctx: WizardContext, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + # --json emits one machine-readable summary on stdout; the exact section map pins + # every label and the SectionResult value strings (auth here exercises "skipped"). + _patch_sections(monkeypatch, auth=SectionResult.SKIPPED) + assert wizard.run_onboarding(NonInteractivePrompter(), json_ctx) == 0 + payload = json.loads(capsys.readouterr().out.strip()) + assert payload == { + "ok": True, + "exit_code": 0, + "sections": { + "welcome": "done", + "sign-in": "skipped", + "first transcription": "done", + "environment": "done", + "build path": "done", + "coding agent": "done", + "next steps": "done", + "launch app": "done", + }, + "failed": [], + } + + +def test_json_summary_on_failure( + json_ctx: WizardContext, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + _patch_sections(monkeypatch, first_request=SectionResult.FAILED) + printed = _capture_stderr(monkeypatch) + assert wizard.run_onboarding(NonInteractivePrompter(), json_ctx) == 1 + payload = json.loads(capsys.readouterr().out.strip()) + assert payload["ok"] is False + assert payload["exit_code"] == 1 + assert payload["failed"] == ["first transcription"] + assert payload["sections"]["first transcription"] == "failed" + # JSON mode keeps the human closing line off stderr (the error envelope is the + # machine-readable failure signal). + assert not any("Set up with" in line for line in printed) + + +def test_json_summary_on_auth_stop( + json_ctx: WizardContext, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + # The auth hard stop keeps its own exit code (4, not the generic 1) and still + # emits the summary; the human "Sign-in didn't complete." line stays off JSON runs. + _patch_sections(monkeypatch, auth=SectionResult.FAILED) + printed = _capture_stderr(monkeypatch) + assert wizard.run_onboarding(NonInteractivePrompter(), json_ctx) == 4 + payload = json.loads(capsys.readouterr().out.strip()) + assert payload["ok"] is False + assert payload["exit_code"] == 4 + assert payload["sections"] == {"welcome": "done", "sign-in": "failed"} + assert payload["failed"] == ["sign-in"] + assert not any("Sign-in didn't complete" in line for line in printed) + + +def test_human_mode_emits_no_json_summary( + ctx: WizardContext, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + _patch_sections(monkeypatch) + assert wizard.run_onboarding(NonInteractivePrompter(), ctx) == 0 + assert capsys.readouterr().out.strip() == "" + + def test_cancel_returns_130(ctx: WizardContext, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(sections, "welcome", lambda p, c: SectionResult.DONE) diff --git a/tests/test_sessions_command.py b/tests/test_sessions_command.py index c19ff8be..16380d73 100644 --- a/tests/test_sessions_command.py +++ b/tests/test_sessions_command.py @@ -1,5 +1,6 @@ import json +import pytest from typer.testing import CliRunner from aai_cli import config @@ -14,7 +15,7 @@ def _auth(): config.set_session("default", session_jwt="jwt", session_token="tok", account_id=42) -def _login_result(): +def _login_result(*, json_mode=False): return LoginResult( api_key="sk_from_oauth", session_jwt="jwt", session_token="tok", account_id=42 ) @@ -24,6 +25,16 @@ def _human(monkeypatch): monkeypatch.setattr("aai_cli.output.resolve_json", lambda *, explicit: explicit) +def test_sessions_help_lists_list_before_get(): + # Pins the list-then-get subcommand order `transcripts --help` mirrors. + result = runner.invoke(app, ["sessions", "--help"]) + assert result.exit_code == 0 + lines = result.output.splitlines() + list_idx = next(i for i, line in enumerate(lines) if "List recent streaming sessions" in line) + get_idx = next(i for i, line in enumerate(lines) if "Show details for one" in line) + assert list_idx < get_idx + + def test_sessions_list_renders_rows(mocker): _auth() payload = { @@ -101,6 +112,50 @@ def test_sessions_list_passes_status_filter(mocker): list_streaming.assert_called_once_with("jwt", limit=5, status="error") +def test_sessions_list_limit_must_be_at_least_one(mocker): + # min=1 on --limit: 0 and negatives are rejected client-side, before any + # request (parity with `transcripts list`). + _auth() + list_streaming = mocker.patch("aai_cli.commands.sessions.ams.list_streaming", autospec=True) + for bad in ("0", "-3"): + result = runner.invoke(app, ["sessions", "list", "--limit", bad]) + assert result.exit_code == 2 + assert "limit" in result.output.lower() + list_streaming.assert_not_called() + + +def test_sessions_list_rejects_unknown_status(mocker): + # --status is a closed choice set; a typo fails instantly instead of silently + # filtering nothing server-side. + _auth() + list_streaming = mocker.patch("aai_cli.commands.sessions.ams.list_streaming", autospec=True) + result = runner.invoke(app, ["sessions", "list", "--status", "comlpeted"]) + assert result.exit_code == 2 + assert "status" in result.output.lower() + list_streaming.assert_not_called() + + +@pytest.mark.parametrize("status", ["created", "completed", "error"]) +def test_sessions_list_passes_each_status_value(mocker, status): + _auth() + list_streaming = mocker.patch( + "aai_cli.commands.sessions.ams.list_streaming", autospec=True, return_value={"data": []} + ) + result = runner.invoke(app, ["sessions", "list", "--status", status]) + assert result.exit_code == 0 + list_streaming.assert_called_once_with("jwt", limit=10, status=status) + + +def test_sessions_list_without_status_passes_none(mocker): + _auth() + list_streaming = mocker.patch( + "aai_cli.commands.sessions.ams.list_streaming", autospec=True, return_value={"data": []} + ) + result = runner.invoke(app, ["sessions", "list"]) + assert result.exit_code == 0 + list_streaming.assert_called_once_with("jwt", limit=10, status=None) + + def test_sessions_get_renders_detail(monkeypatch, mocker): _auth() _human(monkeypatch) diff --git a/tests/test_share.py b/tests/test_share.py index d0533a39..5ce6c210 100644 --- a/tests/test_share.py +++ b/tests/test_share.py @@ -172,6 +172,69 @@ def test_share_no_tunnel_url(tmp_path, monkeypatch): assert proxy.terminated is True +def _capture_tunnel_log(monkeypatch, server, proxy): + """Re-patch runner.spawn to record the cloudflared log path (after _stub).""" + seq = iter([server, proxy]) + logs = [] + + def spawn(command, **kwargs): + if kwargs.get("log_path") is not None: + logs.append(kwargs["log_path"]) + return next(seq) + + monkeypatch.setattr("aai_cli.init.runner.spawn", spawn) + return logs + + +def test_share_tunnel_timeout_keeps_log_and_points_at_it(tmp_path, monkeypatch): + # On "didn't report a tunnel URL in time", cloudflared's captured output is the + # only evidence — the error must name the log file, and the file must survive. + monkeypatch.chdir(tmp_path) + _make_project(tmp_path) + server, proxy = _stub( + monkeypatch, url=None, server=_FakeProc(poll_rc=None), proxy=_FakeProc(poll_rc=None) + ) + logs = _capture_tunnel_log(monkeypatch, server, proxy) + result = runner.invoke(app, ["share"]) + assert result.exit_code == 1 + [log] = logs + try: + assert log.exists() # the evidence is kept on the failure path + packed = "".join(result.output.split()) # the suggestion line may soft-wrap + assert str(log) in packed + assert "checkitforerrors" in packed + finally: + log.unlink(missing_ok=True) + + +def test_share_deletes_tunnel_log_on_clean_exit(tmp_path, monkeypatch): + # A successful share must not leave aai-tunnel-*.log litter in /tmp. + monkeypatch.chdir(tmp_path) + _make_project(tmp_path) + server, proxy = _stub(monkeypatch) + logs = _capture_tunnel_log(monkeypatch, server, proxy) + result = runner.invoke(app, ["share"]) + assert result.exit_code == 0, result.output + [log] = logs + assert not log.exists() + assert str(log) not in result.output # nothing points the user at a deleted file + + +def test_share_log_cleanup_tolerates_already_missing_file(tmp_path, monkeypatch): + # If the log vanished before cleanup, the unlink must not blow up the command. + monkeypatch.chdir(tmp_path) + _make_project(tmp_path) + _stub(monkeypatch) + + def await_and_remove(log_path, **kwargs): + log_path.unlink() + return "https://happy-slug.trycloudflare.com" + + monkeypatch.setattr("aai_cli.init.tunnel.await_url", await_and_remove) + result = runner.invoke(app, ["share"]) + assert result.exit_code == 0, result.output + + def test_share_keyboard_interrupt_is_clean(tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) _make_project(tmp_path) @@ -186,6 +249,26 @@ def test_share_keyboard_interrupt_is_clean(tmp_path, monkeypatch): assert proxy.terminated is True +def test_share_busy_port_notice_on_stderr(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + _make_project(tmp_path) + _stub(monkeypatch) + monkeypatch.setattr("aai_cli.init.runner.find_free_port", lambda p, **k: p + 1) + result = runner.invoke(app, ["share", "--port", "5000"]) + assert result.exit_code == 0, result.output + assert "Port 5000 is in use; using 5001." in result.stderr + + +def test_share_busy_port_notice_suppressed_by_quiet(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + _make_project(tmp_path) + _stub(monkeypatch) + monkeypatch.setattr("aai_cli.init.runner.find_free_port", lambda p, **k: p + 1) + result = runner.invoke(app, ["--quiet", "share", "--port", "5000"]) + assert result.exit_code == 0, result.output + assert "is in use" not in result.output + + def test_share_json_emits_url(tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) _make_project(tmp_path) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index edaf54d4..a495f581 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -55,6 +55,22 @@ def test_env_override_warning_is_structured_in_json_mode(monkeypatch, mocker): assert "may be rejected" in json.loads(warning_line)["warning"] +def test_sandbox_alone_targets_sandbox(): + from aai_cli import environments + + result = runner.invoke(app, ["--sandbox"]) + assert result.exit_code == 0 + assert environments.active().name == "sandbox000" + + +def test_sandbox_with_agreeing_env_is_fine(): + from aai_cli import environments + + result = runner.invoke(app, ["--sandbox", "--env", "sandbox000"]) + assert result.exit_code == 0 + assert environments.active().name == "sandbox000" + + def test_sandbox_flag_conflicting_env_warns(): # Credentials are environment-bound, so `--sandbox --env production` must not pick # one silently: --env wins, with a warning. Agreeing flags and --quiet stay silent. diff --git a/tests/test_speak.py b/tests/test_speak.py index 7aaeb9e1..5f510c6e 100644 --- a/tests/test_speak.py +++ b/tests/test_speak.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import re import pytest from typer.testing import CliRunner @@ -36,9 +37,11 @@ def test_production_env_is_rejected_with_sandbox_hint(): result = runner.invoke(app, ["speak", "Hello"]) # default = production assert result.exit_code == 2 assert "only available in the sandbox" in result.output - # The full command, not just the flag: --sandbox is root-only, so a naive - # `assembly speak … --sandbox` retry would fail with "No such option". - assert "assembly --sandbox speak" in result.output + # The suggestion spells out the exact corrected invocation: --sandbox is a root + # flag, so it must go before the command, not after it. + assert "Re-run as: assembly --sandbox speak" in result.output + # Rich wraps the suggestion at 80 columns, so compare whitespace-normalized. + assert "before the command" in " ".join(result.output.split()) def test_plays_audio_by_default(monkeypatch, fake_synthesize): @@ -225,3 +228,46 @@ def test_unlabeled_text_still_uses_single_voice_path(fake_synthesize, monkeypatc assert result.exit_code == 0 assert fake_synthesize["cfg"].voice == "mary" assert fake_synthesize["cfg"].text == "Just prose." + # No SPEAKER=VOICE mappings were given, so no "Ignoring" warning fires. + assert "Ignoring" not in result.stderr + + +def test_speaker_mappings_on_unlabeled_input_warn_not_silently_drop(fake_synthesize, monkeypatch): + # The mirror of the bare-voice-in-dialogue note: SPEAKER=VOICE mappings can't + # apply to plain prose, and the user is told instead of the flag vanishing. + monkeypatch.setattr("aai_cli.commands.speak.audio.play_pcm", lambda *a, **k: None) + result = runner.invoke(app, ["--sandbox", "speak", "Just prose.", "--voice", "A=vera"]) + assert result.exit_code == 0 + assert "Ignoring --voice SPEAKER=VOICE mappings" in result.stderr + assert "no speaker labels" in result.stderr + # Synthesis still ran with the default voice (the mapping never applies). + assert fake_synthesize["cfg"].voice == "jane" + + +def test_speaker_mappings_warning_is_structured_in_json_mode(fake_synthesize, monkeypatch): + monkeypatch.setattr("aai_cli.commands.speak.audio.play_pcm", lambda *a, **k: None) + result = runner.invoke( + app, ["--sandbox", "speak", "Just prose.", "--voice", "A=vera", "--json"] + ) + assert result.exit_code == 0 + # In --json mode the warning is its own {"warning": …} object on stderr, never + # a bare human line that would corrupt a machine-readable stream. + warning = next(json.loads(line) for line in result.stderr.splitlines() if line.startswith("{")) + assert "no speaker labels" in warning["warning"] + + +def test_sample_rate_must_be_positive(): + result = runner.invoke(app, ["--sandbox", "speak", "Hi", "--sample-rate", "0"]) + assert result.exit_code == 2 + # CI forces color on (Rich under GITHUB_ACTIONS), interleaving style codes + # mid-message, so assert on the color-free render (see test_help_rendering.py). + plain = re.sub(r"\x1b\[[0-9;]*m", "", result.output) + assert "--sample-rate" in plain + + +def test_sample_rate_floor_accepts_one(fake_synthesize, monkeypatch): + # min=1 exactly: 1 Hz is degenerate but valid (the server enforces its own floor). + monkeypatch.setattr("aai_cli.commands.speak.audio.play_pcm", lambda *a, **k: None) + result = runner.invoke(app, ["--sandbox", "speak", "Hi", "--sample-rate", "1"]) + assert result.exit_code == 0 + assert fake_synthesize["cfg"].sample_rate == 1 diff --git a/tests/test_stream_command.py b/tests/test_stream_command.py index 1c003e08..ab7bca21 100644 --- a/tests/test_stream_command.py +++ b/tests/test_stream_command.py @@ -6,9 +6,11 @@ """ import json +import re import time import types +import pytest from typer.testing import CliRunner from aai_cli import config @@ -29,7 +31,7 @@ def _drive_turns( on_turn(types.SimpleNamespace(transcript="hello world", end_of_turn=True)) -def _login_result(): +def _login_result(*, json_mode=False): return LoginResult( api_key="sk_from_oauth", session_jwt="jwt", session_token="tok", account_id=7 ) @@ -309,6 +311,52 @@ def fake_stream(api_key, source, *, params, **kwargs): assert seen["src"] == str(fake) +def test_stream_downloadable_url_resolves_credentials_before_downloading(monkeypatch): + # Regression guard for ordering: with no usable credential the command must fail + # authentication *before* yt-dlp runs, so a signed-out user never downloads a + # whole video only to be told to log in (mirrors transcribe's source -> auth -> + # work ordering). + monkeypatch.setattr("aai_cli.context._interactive_session", lambda: False) + downloads = [] + monkeypatch.setattr( + "aai_cli.commands.stream.youtube.download_audio", + lambda url, dest: downloads.append(url), + ) + monkeypatch.setattr( + "aai_cli.commands.stream.client.stream_audio", + lambda *a, **k: pytest.fail("must not stream without credentials"), + ) + result = runner.invoke(app, ["stream", "https://youtu.be/abc"]) + assert result.exit_code == 4 # not authenticated + assert downloads == [] # nothing was fetched before the credential check + + +def test_stream_sample_rate_must_be_positive(): + config.set_api_key("default", "sk_live") + result = runner.invoke(app, ["stream", "--sample-rate", "0"]) + assert result.exit_code == 2 + # CI forces color on (Rich under GITHUB_ACTIONS), interleaving style codes + # mid-message, so assert on the color-free render (see test_help_rendering.py). + plain = re.sub(r"\x1b\[[0-9;]*m", "", result.output) + assert "--sample-rate" in plain + + +def test_stream_sample_rate_floor_accepts_one_for_stdin(monkeypatch): + # min=1 exactly — and --sample-rate also declares the rate of raw PCM piped on + # stdin (it is not mic-only), so the declared value must reach the session params. + config.set_api_key("default", "sk_live") + seen = {} + + def fake_stream_audio(api_key, source, *, params, **_kwargs): + seen["rate"] = params.sample_rate + b"".join(source) # drain the StdinSource + + monkeypatch.setattr("aai_cli.commands.stream.client.stream_audio", fake_stream_audio) + result = runner.invoke(app, ["stream", "-", "--sample-rate", "1"], input=b"\x00\x00") + assert result.exit_code == 0 + assert seen["rate"] == 1 + + def test_stream_reads_raw_pcm_from_stdin(monkeypatch): config.set_api_key("default", "sk_live") seen = {} diff --git a/tests/test_streaming_diagnostics.py b/tests/test_streaming_diagnostics.py new file mode 100644 index 00000000..f597c70a --- /dev/null +++ b/tests/test_streaming_diagnostics.py @@ -0,0 +1,131 @@ +"""Direct tests for the shared realtime-session hygiene helpers +(aai_cli/streaming/diagnostics.py): library-logger silencing and the +actionable classification of rejected WebSocket handshakes.""" + +from __future__ import annotations + +import logging +import types + +import pytest + +from aai_cli.errors import APIError, NotAuthenticated +from aai_cli.streaming.diagnostics import ( + SDK_STREAMING_LOGGER, + handshake_error, + handshake_suggestion, + silence_streaming_logging, +) +from aai_cli.ws import WEBSOCKETS_LOGGERS + +# The logger the assemblyai SDK's sync streaming client actually emits through. +_SDK_CLIENT_LOGGER = "assemblyai.streaming.v3.client" + + +class _Spy(logging.Handler): + def __init__(self) -> None: + super().__init__() + self.records: list[logging.LogRecord] = [] + + def emit(self, record: logging.LogRecord) -> None: + self.records.append(record) + + +@pytest.fixture +def reset_levels(): + names = (SDK_STREAMING_LOGGER, _SDK_CLIENT_LOGGER, *WEBSOCKETS_LOGGERS) + previous = {name: logging.getLogger(name).level for name in names} + for name in names: + logging.getLogger(name).setLevel(logging.NOTSET) + yield + for name, level in previous.items(): + logging.getLogger(name).setLevel(level) + + +@pytest.mark.usefixtures("reset_levels") +def test_silence_streaming_logging_raises_sdk_and_websockets_loggers(): + silence_streaming_logging() + for name in (SDK_STREAMING_LOGGER, *WEBSOCKETS_LOGGERS): + assert logging.getLogger(name).level == logging.CRITICAL + assert not logging.getLogger(name).isEnabledFor(logging.ERROR) + + +@pytest.mark.usefixtures("reset_levels") +def test_silence_streaming_logging_suppresses_the_sdk_client_logger(): + # The SDK logs "Connection failed: …" at ERROR through a *child* of the silenced + # logger; prove a real record is dropped, not just that a level attribute moved. + spy = _Spy() + root = logging.getLogger() + root.addHandler(spy) + try: + logging.getLogger(_SDK_CLIENT_LOGGER).error("Connection failed: handshake rejected") + assert len(spy.records) == 1 # the spy sees the record before silencing + silence_streaming_logging() + logging.getLogger(_SDK_CLIENT_LOGGER).error("Connection failed: handshake rejected") + assert len(spy.records) == 1 # …and nothing new after + finally: + root.removeHandler(spy) + + +def test_sdk_streaming_logger_is_the_assemblyai_parent(): + assert SDK_STREAMING_LOGGER == "assemblyai.streaming" + + +def test_handshake_suggestion_names_whoami_env_and_host(): + text = handshake_suggestion("streaming.assemblyai.com") + assert "assembly whoami" in text + assert "--sandbox" in text + assert "network/proxy access to streaming.assemblyai.com" in text + + +def test_handshake_suggestion_falls_back_when_host_is_empty(): + # e.g. the TTS host is empty outside the sandbox; never render "access to ." + assert "network/proxy access to the streaming endpoint." in handshake_suggestion("") + + +class _SdkHandshake(Exception): + """Mimics the assemblyai SDK's StreamingError: the HTTP status on ``.code``.""" + + def __init__(self, status: int) -> None: + super().__init__(f"WebSocket handshake rejected (HTTP {status})") + self.code = status + + +class _WsHandshake(Exception): + """Mimics websockets' InvalidStatus: the status on ``.response.status_code``.""" + + def __init__(self, status: int) -> None: + super().__init__(f"server rejected WebSocket connection: HTTP {status}") + self.response = types.SimpleNamespace(status_code=status) + + +def test_handshake_401_is_not_authenticated_with_suggestion(): + err = handshake_error(_SdkHandshake(401), "Streaming error", host="h.example") + assert isinstance(err, NotAuthenticated) + assert err.exit_code == 4 + assert err.rejected_key is True # auto-login must not retry an env-provided key + assert err.message == "Streaming error: WebSocket handshake rejected (HTTP 401)" + assert err.suggestion == handshake_suggestion("h.example") + + +def test_handshake_403_is_api_error_with_suggestion(): + # 403 also covers WAF/region/plan blocks, so it stays exit 1 — but suggests. + err = handshake_error(_WsHandshake(403), "Could not connect", host="h.example") + assert isinstance(err, APIError) + assert err.exit_code == 1 + assert err.message == "Could not connect: server rejected WebSocket connection: HTTP 403" + assert err.suggestion == handshake_suggestion("h.example") + + +def test_handshake_status_read_from_websockets_response_shape(): + err = handshake_error(_WsHandshake(401), "Could not connect", host="h.example") + assert isinstance(err, NotAuthenticated) + + +def test_non_handshake_errors_return_none(): + assert handshake_error(RuntimeError("socket dropped"), "Streaming error", host="h") is None + # A WebSocket close code (e.g. 1008 policy violation) is not a handshake status. + closed = types.SimpleNamespace(code=1008) + assert handshake_error(closed, "Streaming error", host="h") is None + # Other HTTP statuses (e.g. a 500 on the upgrade) are not auth-shaped. + assert handshake_error(_WsHandshake(500), "Streaming error", host="h") is None diff --git a/tests/test_telemetry.py b/tests/test_telemetry.py index 0bbcfbfb..cf667653 100644 --- a/tests/test_telemetry.py +++ b/tests/test_telemetry.py @@ -72,9 +72,44 @@ def test_is_enabled_requires_token_and_consent(monkeypatch): assert telemetry.is_enabled() is False +# --- consent source ------------------------------------------------------------- + + +def test_consent_source_default(): + assert telemetry.consent_source() == "default" + + +@pytest.mark.parametrize("enabled", [True, False]) +def test_consent_source_persisted_choice(enabled): + config.set_telemetry_enabled(enabled=enabled) + assert telemetry.consent_source() == "config" + + +@pytest.mark.parametrize("var", ["AAI_TELEMETRY_DISABLED", "DO_NOT_TRACK"]) +def test_consent_source_env_kill_switch_wins_over_config(monkeypatch, var): + config.set_telemetry_enabled(enabled=True) # the env switch outranks the choice + monkeypatch.setenv(var, "1") + assert telemetry.consent_source() == f"env:{var}" + + +def test_consent_source_env_ordering_matches_consent_granted(monkeypatch): + monkeypatch.setenv("AAI_TELEMETRY_DISABLED", "1") + monkeypatch.setenv("DO_NOT_TRACK", "1") + assert telemetry.consent_source() == "env:AAI_TELEMETRY_DISABLED" + + # --- config-backed telemetry state ------------------------------------------- +def test_has_device_id_probes_without_minting(tmp_config): + assert config.has_device_id() is False + # Probing must not itself mint/persist an id. + config_file = tmp_config / "config.toml" + assert not config_file.exists() or "device_id" not in config_file.read_text() + config.get_device_id() + assert config.has_device_id() is True + + def test_telemetry_enabled_roundtrip(): assert config.get_telemetry_enabled() is None config.set_telemetry_enabled(enabled=False) @@ -349,3 +384,76 @@ def explode(event): monkeypatch.setattr(telemetry, "dispatch", explode) with telemetry.track("aai doctor"): pass # must not raise + + +# --- first-run disclosure ------------------------------------------------------- + + +def test_first_run_notice_prints_once_on_device_id_mint(events, monkeypatch, capsys): + monkeypatch.setattr(sys, "argv", ["assembly", "doctor"]) + with telemetry.track("assembly doctor"): + pass + err = capsys.readouterr().err + assert "Anonymous usage data is collected" in err + assert "'assembly telemetry disable'" in err + assert "DO_NOT_TRACK=1" in err + assert err.count("Anonymous usage data") == 1 + # The device id persists, so a second run stays silent — at most once ever. + with telemetry.track("assembly doctor"): + pass + assert "Anonymous usage data" not in capsys.readouterr().err + + +@pytest.mark.parametrize( + "argv", + [["assembly", "-q", "doctor"], ["assembly", "transcribe", "x.wav", "--json"]], + ids=["quiet", "json"], +) +def test_first_run_notice_suppressed_for_quiet_and_json(events, monkeypatch, capsys, argv): + monkeypatch.setattr(sys, "argv", argv) + with telemetry.track("assembly doctor"): + pass + assert "Anonymous usage data" not in capsys.readouterr().err + # Suppression still consumes the one-time mint; the disclosure never shows up later. + assert config.has_device_id() is True + + +def test_first_run_notice_not_minted_while_telemetry_inert(monkeypatch, capsys): + monkeypatch.setattr(sys, "argv", ["assembly", "doctor"]) + with telemetry.track("assembly doctor"): # no token -> inert, nothing collected + pass + assert "Anonymous usage data" not in capsys.readouterr().err + assert config.has_device_id() is False + + +@pytest.mark.parametrize("exc", [OSError("disk full"), CLIError("corrupt config")]) +def test_first_run_notice_failures_never_break_the_command(events, monkeypatch, exc): + def explode(): + raise exc + + monkeypatch.setattr(config, "has_device_id", explode) + with telemetry.track("assembly doctor"): + pass # must not raise + + +@pytest.mark.parametrize( + ("raw_args", "suppressed"), + [ + (["--quiet"], True), + (["-q", "doctor"], True), + (["transcribe", "x.wav", "--json"], True), + (["-j"], True), + (["-o", "json"], True), + (["-o", "json", "extra"], True), + (["--output", "json"], True), + (["--output=json"], True), + (["-ojson"], True), + (["-o", "text"], False), + (["-o"], False), + (["transcribe", "x.wav"], False), + ([], False), + ], + ids=repr, +) +def test_notice_suppression_matches_quiet_and_json_forms(raw_args, suppressed): + assert telemetry._notice_suppressed(raw_args) is suppressed diff --git a/tests/test_telemetry_command.py b/tests/test_telemetry_command.py index 1cf323a0..79f985cf 100644 --- a/tests/test_telemetry_command.py +++ b/tests/test_telemetry_command.py @@ -1,6 +1,7 @@ """Tests for `assembly telemetry status/enable/disable` and the run_command integration.""" import json +import re from typer.testing import CliRunner @@ -16,6 +17,9 @@ def _human(monkeypatch): def _capture_events(monkeypatch, *, token="pub_test"): monkeypatch.setenv(telemetry.ENV_CLIENT_TOKEN, token) + # Pre-mint the device id: the one-time first-run disclosure (covered in + # test_telemetry.py) would otherwise interleave with the output assertions here. + config.get_device_id() captured = [] monkeypatch.setattr(telemetry, "dispatch", captured.append) return captured @@ -27,6 +31,7 @@ def test_status_json_when_inert(): assert json.loads(result.output) == { "enabled": False, "consent": "granted", + "source": "default", "token_configured": False, } @@ -38,6 +43,7 @@ def test_status_json_when_enabled(monkeypatch): assert json.loads(result.output) == { "enabled": True, "consent": "granted", + "source": "default", "token_configured": True, } @@ -50,6 +56,23 @@ def test_status_json_when_opted_out(monkeypatch): assert json.loads(result.output) == { "enabled": False, "consent": "denied", + "source": "config", + "token_configured": True, + } + + +def test_status_json_source_for_env_kill_switch(monkeypatch): + # The docstring promises status says *why*: an env kill-switch silently beating + # a persisted `telemetry enable` must be visible as the source. + _capture_events(monkeypatch) + config.set_telemetry_enabled(enabled=True) + monkeypatch.setenv("AAI_TELEMETRY_DISABLED", "1") + result = runner.invoke(app, ["telemetry", "status", "--json"]) + assert result.exit_code == 0 + assert json.loads(result.output) == { + "enabled": False, + "consent": "denied", + "source": "env:AAI_TELEMETRY_DISABLED", "token_configured": True, } @@ -59,8 +82,10 @@ def test_status_human_disabled(monkeypatch): result = runner.invoke(app, ["telemetry", "status"]) assert result.exit_code == 0 assert "Telemetry is disabled." in result.output - assert "Consent: granted. Intake token configured: no." in result.output - assert "assembly telemetry disable" in result.output + assert "Consent: granted (source: default). Intake token configured: no." in result.output + # When already disabled the actionable direction is re-enabling, not opting out. + assert "Re-enable with 'assembly telemetry enable'." in result.output + assert "Opt out any time" not in result.output def test_status_human_enabled(monkeypatch): @@ -69,7 +94,21 @@ def test_status_human_enabled(monkeypatch): result = runner.invoke(app, ["telemetry", "status"]) assert result.exit_code == 0 assert "Telemetry is enabled." in result.output - assert "Consent: granted. Intake token configured: yes." in result.output + assert "Consent: granted (source: default). Intake token configured: yes." in result.output + assert "Opt out any time: 'assembly telemetry disable'" in result.output + assert "Re-enable with" not in result.output + + +def test_status_human_says_why_when_env_overrides_persisted_enable(monkeypatch): + _human(monkeypatch) + _capture_events(monkeypatch) + config.set_telemetry_enabled(enabled=True) + monkeypatch.setenv("DO_NOT_TRACK", "1") + result = runner.invoke(app, ["telemetry", "status"]) + assert result.exit_code == 0 + assert "Telemetry is disabled." in result.output + assert "(source: env:DO_NOT_TRACK)" in result.output + assert "Re-enable with 'assembly telemetry enable'." in result.output def test_disable_persists_and_confirms_json(): @@ -105,6 +144,18 @@ def test_flush_is_hidden_plumbing(): assert "flush" not in result.output +def test_bare_telemetry_shows_help_not_missing_command(): + result = runner.invoke(app, ["telemetry"]) + assert result.exit_code == 2 + # CI forces color on (Rich under GITHUB_ACTIONS), interleaving style codes + # mid-message, so assert on the color-free render (see test_help_rendering.py). + plain = re.sub(r"\x1b\[[0-9;]*m", "", result.output) + assert "Missing command" not in plain + assert "Usage: assembly telemetry" in plain + assert "status" in plain + assert "disable" in plain + + # --- run_command integration ------------------------------------------------- diff --git a/tests/test_transcribe.py b/tests/test_transcribe.py index dcd00251..fb7bea2a 100644 --- a/tests/test_transcribe.py +++ b/tests/test_transcribe.py @@ -28,7 +28,7 @@ def _auth(): config.set_api_key("default", "sk_live") -def _login_result(): +def _login_result(*, json_mode=False): return LoginResult( api_key="sk_from_oauth", session_jwt="jwt", session_token="tok", account_id=7 ) diff --git a/tests/test_transcribe_flags.py b/tests/test_transcribe_flags.py index 71aec1f0..6cff130b 100644 --- a/tests/test_transcribe_flags.py +++ b/tests/test_transcribe_flags.py @@ -248,6 +248,149 @@ def test_transcribe_speakers_expected_with_config_speaker_labels_is_accepted(moc assert tx.call_args.kwargs["config"].speakers_expected == 2 +@pytest.mark.parametrize("value", ["-0.1", "1.5", "99"]) +def test_transcribe_temperature_out_of_range_exits_2(mocker, value): + # The API documents temperature as 0 (most deterministic) to 1 (least); reject + # out-of-range values client-side instead of letting them flow to the request. + _auth() + tx = mocker.patch("aai_cli.commands.transcribe.client.transcribe", autospec=True) + result = runner.invoke(app, ["transcribe", "audio.mp3", "--temperature", value]) + assert result.exit_code == 2 + tx.assert_not_called() + + +@pytest.mark.parametrize("value", ["0", "1"]) +def test_transcribe_temperature_bounds_are_inclusive(mocker, value): + _auth() + tx = mocker.patch( + "aai_cli.commands.transcribe.client.transcribe", + autospec=True, + return_value=_fake_transcript(mocker), + ) + result = runner.invoke(app, ["transcribe", "audio.mp3", "--temperature", value]) + assert result.exit_code == 0 + assert tx.call_args.kwargs["config"].temperature == float(value) + + +def test_transcribe_negative_audio_end_exits_2(mocker): + _auth() + tx = mocker.patch("aai_cli.commands.transcribe.client.transcribe", autospec=True) + result = runner.invoke(app, ["transcribe", "audio.mp3", "--audio-end", "-100"]) + assert result.exit_code == 2 + tx.assert_not_called() + + +def test_transcribe_audio_end_zero_is_accepted(mocker): + _auth() + tx = mocker.patch( + "aai_cli.commands.transcribe.client.transcribe", + autospec=True, + return_value=_fake_transcript(mocker), + ) + result = runner.invoke(app, ["transcribe", "audio.mp3", "--audio-end", "0"]) + assert result.exit_code == 0 + tx.assert_called_once() + + +def test_transcribe_json_with_non_json_output_field_exits_2(mocker): + # --json means "the full JSON payload" (same as -o json); -o text contradicts it + # and must not silently win. + _auth() + tx = mocker.patch("aai_cli.commands.transcribe.client.transcribe", autospec=True) + result = runner.invoke(app, ["transcribe", "audio.mp3", "-o", "text", "--json"]) + assert result.exit_code == 2 + assert "--json conflicts with -o text" in result.output + tx.assert_not_called() + + +def test_transcribe_json_with_o_json_is_accepted(mocker): + import json + + _auth() + mocker.patch( + "aai_cli.commands.transcribe.client.transcribe", + autospec=True, + return_value=_fake_transcript(mocker), + ) + result = runner.invoke(app, ["transcribe", "audio.mp3", "-o", "json", "--json"]) + assert result.exit_code == 0 + assert json.loads(result.output.strip())["id"] == "t_1" + + +def test_transcribe_warns_on_non_audio_extension(mocker, tmp_path): + _auth() + (tmp_path / "notes.txt").write_bytes(b"fake") + mocker.patch( + "aai_cli.commands.transcribe.client.transcribe", + autospec=True, + return_value=_fake_transcript(mocker), + ) + result = runner.invoke(app, ["transcribe", "notes.txt"]) + assert result.exit_code == 0 # a warning, never an error — the server decides + assert "doesn't look like audio" in result.output + assert "'.txt'" in result.output + + +def test_transcribe_non_audio_warning_suppressed_by_quiet(mocker, tmp_path): + _auth() + (tmp_path / "notes.txt").write_bytes(b"fake") + mocker.patch( + "aai_cli.commands.transcribe.client.transcribe", + autospec=True, + return_value=_fake_transcript(mocker), + ) + result = runner.invoke(app, ["-q", "transcribe", "notes.txt"]) + assert result.exit_code == 0 + assert "doesn't look like audio" not in result.output + + +def test_transcribe_non_audio_warning_is_structured_under_json(mocker, tmp_path): + import json + + _auth() + (tmp_path / "notes.txt").write_bytes(b"fake") + mocker.patch( + "aai_cli.commands.transcribe.client.transcribe", + autospec=True, + return_value=_fake_transcript(mocker), + ) + result = runner.invoke(app, ["transcribe", "notes.txt", "--json"]) + assert result.exit_code == 0 + warning = next( + json.loads(line) for line in result.output.splitlines() if line.startswith('{"warning"') + ) + assert "doesn't look like audio" in warning["warning"] + + +@pytest.mark.parametrize("name", ["audio.mp3", "noext"]) +def test_transcribe_no_warning_for_audio_or_extensionless_files(mocker, tmp_path, name): + # Known audio extensions and extensionless files (we can't judge those) stay quiet. + _auth() + (tmp_path / name).write_bytes(b"fake") + mocker.patch( + "aai_cli.commands.transcribe.client.transcribe", + autospec=True, + return_value=_fake_transcript(mocker), + ) + result = runner.invoke(app, ["transcribe", name]) + assert result.exit_code == 0 + assert "doesn't look like audio" not in result.output + + +@pytest.mark.parametrize("argv", [["https://example.com/notes.txt"], ["--sample"]]) +def test_transcribe_no_warning_for_urls_or_sample(mocker, argv): + # Remote sources aren't local files; the extension heuristic doesn't apply. + _auth() + mocker.patch( + "aai_cli.commands.transcribe.client.transcribe", + autospec=True, + return_value=_fake_transcript(mocker), + ) + result = runner.invoke(app, ["transcribe", *argv]) + assert result.exit_code == 0 + assert "doesn't look like audio" not in result.output + + def test_transcribe_unknown_pii_policy_exits_2_and_lists_valid(mocker): _auth() tx = mocker.patch("aai_cli.commands.transcribe.client.transcribe", autospec=True) diff --git a/tests/test_transcribe_out.py b/tests/test_transcribe_out.py index 2f170117..52c43d55 100644 --- a/tests/test_transcribe_out.py +++ b/tests/test_transcribe_out.py @@ -100,6 +100,44 @@ def test_transcribe_out_with_llm_is_a_usage_error(tmp_path): assert not out.exists() +def test_transcribe_out_missing_parent_dir_fails_before_transcribing(tmp_path): + # The --out path is validated up front: a bad directory must not surface as an + # internal error after a billed, possibly long transcription. + _auth() + out = tmp_path / "no" / "such" / "dir" / "x.txt" + with patch(_TRANSCRIBE) as tx: + result = runner.invoke(app, ["transcribe", "audio.mp3", "--out", str(out)]) + assert result.exit_code == 2 + assert "doesn't exist" in result.output + tx.assert_not_called() + assert not out.exists() + + +def test_transcribe_out_unwritable_parent_dir_fails_before_transcribing(tmp_path, monkeypatch): + import os + + from aai_cli import transcribe_exec + + _auth() + out = tmp_path / "x.txt" + calls = [] + real_access = os.access + + def fake_access(path, mode, **kwargs): + if path == out.parent: + calls.append(mode) + return False + return real_access(path, mode, **kwargs) + + monkeypatch.setattr(transcribe_exec.os, "access", fake_access) + with patch(_TRANSCRIBE) as tx: + result = runner.invoke(app, ["transcribe", "audio.mp3", "--out", str(out)]) + assert result.exit_code == 2 + assert "isn't writable" in result.output + tx.assert_not_called() + assert calls == [os.W_OK] # the writability probe, not a read/exec check + + def test_transcribe_out_rejects_path_traversal(tmp_path): # A --out path with a `..` segment is rejected with a clean usage error, # before anything is written. diff --git a/tests/test_transcripts.py b/tests/test_transcripts.py index 6bbece7e..fbe6af3d 100644 --- a/tests/test_transcripts.py +++ b/tests/test_transcripts.py @@ -9,12 +9,22 @@ runner = CliRunner() -def _login_result(): +def _login_result(*, json_mode=False): return LoginResult( api_key="sk_from_oauth", session_jwt="jwt", session_token="tok", account_id=7 ) +def test_transcripts_help_lists_list_before_get(): + # Subcommand order matches `assembly sessions --help`: list first, then get. + result = runner.invoke(app, ["transcripts", "--help"]) + assert result.exit_code == 0 + lines = result.output.splitlines() + list_idx = next(i for i, line in enumerate(lines) if "List recent transcripts" in line) + get_idx = next(i for i, line in enumerate(lines) if "Fetch a past transcript" in line) + assert list_idx < get_idx + + def test_get_prints_transcript_text(mocker): config.set_api_key("default", "sk_live") fake = mocker.MagicMock() @@ -125,6 +135,19 @@ def test_list_empty_shows_human_empty_state(monkeypatch, mocker): assert "No transcripts yet." in result.output +def test_get_malformed_id_is_rejected_before_auth(monkeypatch, mocker): + # No key configured: the cheap local id check must win over auth, so the user + # is told to fix the id instead of being sent through login first. + monkeypatch.setattr("aai_cli.context._interactive_session", lambda: True) + login = mocker.patch("aai_cli.context.run_login_flow", side_effect=AssertionError("no login")) + get = mocker.patch("aai_cli.commands.transcripts.client.get_transcript", autospec=True) + result = runner.invoke(app, ["transcripts", "get", "not-a-real-id!!"]) + assert result.exit_code == 2 + assert "doesn't look like a transcript id" in result.output + get.assert_not_called() + login.assert_not_called() + + def test_get_output_invalid_field_exits_2(): config.set_api_key("default", "sk_live") result = runner.invoke(app, ["transcripts", "get", "t_42", "-o", "bogus"]) diff --git a/tests/test_tts_session.py b/tests/test_tts_session.py index 5d0d5c14..5ac6621e 100644 --- a/tests/test_tts_session.py +++ b/tests/test_tts_session.py @@ -255,8 +255,31 @@ class Forbidden(Exception): def _connect(*_a, **_k): raise Forbidden("Unauthorized") # 403 -> NOT a rejected key - with pytest.raises(APIError): + with pytest.raises(APIError) as exc: + session.synthesize("k", session.SpeakConfig(text="hi"), connect=_connect) + assert "Could not connect to the TTS service" in exc.value.message + # The rejected handshake carries the actionable next steps. + assert exc.value.suggestion is not None + assert "assembly whoami" in exc.value.suggestion + assert "--sandbox" in exc.value.suggestion + + +def test_synthesize_handshake_401_is_not_authenticated_with_suggestion(): + class Resp: + status_code = 401 + + class Rejected(Exception): + response = Resp() + + def _connect(*_a, **_k): + raise Rejected("server rejected WebSocket connection: HTTP 401") + + with pytest.raises(NotAuthenticated) as exc: session.synthesize("k", session.SpeakConfig(text="hi"), connect=_connect) + assert exc.value.exit_code == 4 + assert exc.value.rejected_key is True + assert exc.value.suggestion is not None + assert "assembly whoami" in exc.value.suggestion def test_synthesize_error_frame_without_details_says_unknown(): diff --git a/tests/test_youtube.py b/tests/test_youtube.py index 26112ecd..7cd71b90 100644 --- a/tests/test_youtube.py +++ b/tests/test_youtube.py @@ -214,7 +214,7 @@ def prepare_filename(self, info): assert "no audio file" in exc.value.message -def test_download_audio_error_raises_cli_error(tmp_path, monkeypatch): +def _raising_ydl(message): class FakeYDL: def __init__(self, opts): pass @@ -226,16 +226,51 @@ def __exit__(self, *exc): return False def extract_info(self, url, download): - raise RuntimeError("network down") + raise RuntimeError(message) def prepare_filename(self, info): return "" - _fake_ytdlp(monkeypatch, FakeYDL) + return FakeYDL + + +def test_download_audio_error_raises_cli_error(tmp_path, monkeypatch): + _fake_ytdlp(monkeypatch, _raising_ydl("network down")) with pytest.raises(CLIError) as exc: youtube.download_audio("https://youtu.be/x", tmp_path) assert exc.value.error_type == "youtube_error" assert exc.value.exit_code == 1 + # A message without boilerplate passes through untouched. + assert exc.value.message == "Could not download audio from https://youtu.be/x: network down" + + +_YTDLP_BOILERPLATE = ( + "please report this issue on https://github.com/yt-dlp/yt-dlp/issues?q= , filling " + "out the appropriate issue template. Confirm you are on the latest version using yt-dlp -U" +) + + +def test_download_audio_trims_ytdlp_bug_report_boilerplate(tmp_path, monkeypatch): + # yt-dlp appends report-a-bug boilerplate to extractor errors; only the + # meaningful part should reach the user, without the "ERROR: " prefix. + message = f"ERROR: [youtube] abc: Video unavailable; {_YTDLP_BOILERPLATE}" + _fake_ytdlp(monkeypatch, _raising_ydl(message)) + with pytest.raises(CLIError) as exc: + youtube.download_audio("https://youtu.be/x", tmp_path) + assert exc.value.message == ( + "Could not download audio from https://youtu.be/x: [youtube] abc: Video unavailable" + ) + assert "report this issue" not in exc.value.message + assert "latest version" not in exc.value.message + + +def test_download_audio_all_boilerplate_message_falls_back_to_raw_text(tmp_path, monkeypatch): + # When trimming would leave nothing, keep the original message over an empty error. + message = _YTDLP_BOILERPLATE[0].upper() + _YTDLP_BOILERPLATE[1:] + _fake_ytdlp(monkeypatch, _raising_ydl(message)) + with pytest.raises(CLIError) as exc: + youtube.download_audio("https://youtu.be/x", tmp_path) + assert message in exc.value.message def test_download_audio_missing_ytdlp_raises(tmp_path, monkeypatch): diff --git a/uv.lock b/uv.lock index 9973a051..258f044a 100644 --- a/uv.lock +++ b/uv.lock @@ -42,6 +42,7 @@ dependencies = [ [package.dev-dependencies] dev = [ + { name = "click" }, { name = "coverage" }, { name = "deptry" }, { name = "diff-cover" }, @@ -93,6 +94,7 @@ requires-dist = [ [package.metadata.requires-dev] dev = [ + { name = "click", specifier = ">=8.1" }, { name = "coverage", specifier = ">=7.0" }, { name = "deptry", specifier = ">=0.23.0" }, { name = "diff-cover", specifier = ">=9.0.0" },