Skip to content

Commit 8fc05e2

Browse files
alexkromanalexkroman-assemblyclaude
authored
Fix CLI error-handling papercuts and make the gate pass on macOS (#163)
## Summary Manual QA across the command surface (run via parallel subagents) surfaced a cluster of **user-input errors mislabeled as internal "report a bug" failures**, plus a few smaller inconsistencies. While verifying the fixes through `scripts/check.sh`, two **macOS-only gate failures** also turned up and are fixed here so the gate is green on both macOS and Linux. ## CLI fixes | Command | Before | After | |---|---|---| | `webhooks listen --port` | out-of-range port → `Unexpected error … report a bug` | `min=0/max=65535` → clean exit-2 validation error | | `init <tmpl> file/sub` | raw `[Errno 20] Not a directory` mid-scaffold | rejected up front as a clean usage error | | `stream --redact-pii-sub` | bad value leaked a pydantic dump + `errors.pydantic.dev` URL | now a `PIISubstitutionPolicy` enum → clean `[hash\|entity_name]` choice error | | `clip --out-dir <file>` | "doesn't exist" (misleading) | "is not a directory" | | `config path` | failed on a corrupt `config.toml` — the one command you'd use to find the broken file | reports the location anyway (deferred parse error; real commands still re-raise it; a bad `--env` still wins) | | `llm` (bad key) | exit 1 + raw 401 dict body | exit-4 `auth_failure` like `transcribe`; entitlement/proxy 403s keep their exit-1 passthrough | | account commands | two confusingly-different no-credentials messages | reworded to explain *why* account data needs a browser session vs an API key | | `telemetry enable` | "✓ Telemetry enabled" even when `DO_NOT_TRACK`/`AAI_TELEMETRY_DISABLED` overrides it | appends a note that the env kill-switch keeps it off | ## Gate fixes (`scripts/check.sh`) — macOS parity - **`brew audit`**: Homebrew 6+ disabled `brew audit [path …]`; a formula must be audited *by name*. The step now copies `Formula/assembly.rb` into an ephemeral local tap and audits it by name (works on macOS + Linuxbrew, old + new brew), with cleanup that survives `set -e`. - **"no new escape hatches"**: the baseline count used `git grep -E` while the working tree used `rg`, which disagree on `\b` — macOS's ERE engine silently ignores it, so a *pre-existing* `time.sleep` made the working count exceed the baseline and failed the gate on macOS only. Both sides now use one matcher, `git grep -P` (PCRE), via shared `hatch_base`/`hatch_work` helpers (`--untracked` counts newly-added files the way `rg` did). - Also fixes `test_validate_out_rejects_the_input_via_hard_link` to tolerate case-insensitive filesystems (macOS). ## Verification `./scripts/check.sh` → **All checks passed** (exit 0), including: - 2556 tests pass - 100% patch coverage on all changed source files - mutation gate: 16/16 mutants killed on changed lines - brew audit, escape-hatch gate, build + twine — all green 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Alex Kroman <alex@assemblyai.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4bc28ea commit 8fc05e2

22 files changed

Lines changed: 378 additions & 70 deletions

aai_cli/app/context.py

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ class AppState:
2626
profile: str | None = None
2727
env: str | None = None
2828
quiet: bool = False
29+
# Set by the root callback when config.toml is unreadable during environment
30+
# resolution: most commands re-raise it (run_command), but `config path` — the
31+
# command you reach for to *find* the broken file — tolerates it.
32+
deferred_config_error: CLIError | None = None
2933

3034
def resolve_profile(self) -> str:
3135
"""The profile to act on: explicit --profile, else the active profile.
@@ -64,10 +68,12 @@ def resolve_session(self) -> tuple[int, str]:
6468
# never satisfy these endpoints — they authenticate with the browser
6569
# session, not the API key — so spell out the only fix that works.
6670
raise NotAuthenticated(
67-
"These commands need a browser login. Run 'assembly login' (without --api-key).",
71+
"Account commands read account-level data and authenticate with your "
72+
"browser-login session, which this profile doesn't have.",
6873
suggestion=(
69-
"Run 'assembly login' to sign in via your browser — an API key alone "
70-
"can't access account commands."
74+
"Run 'assembly login' to sign in via your browser. Unlike transcription "
75+
"commands (which work with an API key), account data like balance, usage, "
76+
"and keys needs that session."
7177
),
7278
)
7379
# Registered like the API key in config.resolve_api_key: -v/-vv diagnostics
@@ -197,16 +203,34 @@ def run_command(
197203
*,
198204
json: bool = False,
199205
auto_login: bool = True,
206+
tolerate_unreadable_config: bool = False,
200207
) -> None:
201-
"""Execute a command body, mapping CLIError to clean output + exit code."""
208+
"""Execute a command body, mapping CLIError to clean output + exit code.
209+
210+
`tolerate_unreadable_config` lets a command (only `config path`) run even when the
211+
root callback deferred a corrupt-config error, so it can still report the file's
212+
location; every other command re-raises that error here.
213+
"""
202214
state: AppState = ctx.obj
203215
json_mode = output.resolve_json(explicit=json)
216+
deferred = state.deferred_config_error
217+
if deferred is not None and not tolerate_unreadable_config:
218+
# The root callback couldn't read config.toml. Surface that for ordinary
219+
# commands (which depend on it) the same way the callback used to — emit and
220+
# exit, without telemetry/update-check, both of which would just re-parse it.
221+
_fail(deferred, json_mode=json_mode)
204222
try:
205-
# Inside the try so telemetry sees the raw CLIError (and its error_type)
206-
# before it's folded into a typer.Exit below.
207-
with telemetry.track(ctx.command_path):
223+
if deferred is not None:
224+
# `config path` opted in (tolerate_unreadable_config): it reports a
225+
# contents-independent location, so run just the body and skip the
226+
# telemetry/update-check wrappers that re-parse the broken config.
208227
fn(state, json_mode)
209-
update_check.maybe_notify(json_mode=json_mode)
228+
else:
229+
# Inside the try so telemetry sees the raw CLIError (and its error_type)
230+
# before it's folded into a typer.Exit below.
231+
with telemetry.track(ctx.command_path):
232+
fn(state, json_mode)
233+
update_check.maybe_notify(json_mode=json_mode)
210234
except NotAuthenticated as err:
211235
if not auto_login or not _should_auto_login(err):
212236
_fail(err, json_mode=json_mode)

aai_cli/app/init_exec.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,24 @@ def _install_step(
141141
], will_launch
142142

143143

144+
def _reject_file_ancestor(target: Path) -> None:
145+
"""Reject a target that descends through an existing file (e.g. ``somefile/app``).
146+
147+
``scaffold`` calls ``target.mkdir(parents=True)``, which raises a raw
148+
``NotADirectoryError`` mid-scaffold when a parent component is a regular file —
149+
surfacing as an "Unexpected error … report a bug" line for what is really a bad
150+
path. Catch it up front as a clean usage error instead.
151+
"""
152+
for ancestor in target.parents:
153+
if ancestor.exists():
154+
if not ancestor.is_dir():
155+
raise UsageError(
156+
f"{ancestor} is not a directory, so {target} can't be created.",
157+
suggestion="Pick a target whose parent directories are real directories.",
158+
)
159+
return
160+
161+
144162
def _resolve_target(
145163
directory: str | None, chosen: str, *, here: bool, force: bool
146164
) -> tuple[Path, bool]:
@@ -155,6 +173,7 @@ def _resolve_target(
155173
target = _resolve_dir(directory, chosen, here=here)
156174
if target.exists() and not target.is_dir():
157175
raise UsageError(f"{target} exists and is not a directory.")
176+
_reject_file_ancestor(target)
158177
conflict = scaffold.target_conflict(target)
159178
if conflict and not force:
160179
raise CLIError(

aai_cli/commands/clip/_exec.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,19 @@ def _transcript_segments(
193193

194194

195195
def _validate_out_dir(out_dir: Path | None) -> None:
196-
if out_dir is not None and not out_dir.is_dir():
196+
if out_dir is None or out_dir.is_dir():
197+
return
198+
# Distinguish a missing path from one that exists but isn't a directory: the old
199+
# "doesn't exist" wording was misleading when --out-dir pointed at a regular file.
200+
if out_dir.exists():
197201
raise UsageError(
198-
f"--out-dir doesn't exist: {out_dir}",
199-
suggestion="Create it first, or point --out-dir at an existing directory.",
202+
f"--out-dir is not a directory: {out_dir}",
203+
suggestion="Point --out-dir at a directory, not a file.",
200204
)
205+
raise UsageError(
206+
f"--out-dir doesn't exist: {out_dir}",
207+
suggestion="Create it first, or point --out-dir at an existing directory.",
208+
)
201209

202210

203211
def _validate_selection(opts: ClipOptions) -> None:

aai_cli/commands/config.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,9 @@ def body(_state: AppState, json_mode: bool) -> None:
112112
# unwrapped (`cd "$(assembly config path | xargs dirname)"`).
113113
output.emit_text(str(file))
114114

115-
run_command(ctx, body, json=json_out)
115+
# The location is independent of the file's contents, so report it even when the
116+
# config is unreadable — this is the command you'd use to go fix the broken file.
117+
run_command(ctx, body, json=json_out, tolerate_unreadable_config=True)
116118

117119

118120
@app.command(

aai_cli/commands/stream/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from pathlib import Path
44

55
import typer
6+
from assemblyai import PIISubstitutionPolicy
67
from assemblyai.streaming.v3 import Encoding, NoiseSuppressionModel, SpeechModel
78

89
from aai_cli import command_registry, help_panels, options
@@ -210,7 +211,7 @@ def stream(
210211
help="Comma-separated PII policies",
211212
rich_help_panel=help_panels.OPT_GUARDRAILS,
212213
),
213-
redact_pii_sub: str | None = typer.Option(
214+
redact_pii_sub: PIISubstitutionPolicy | None = typer.Option(
214215
None,
215216
"--redact-pii-sub",
216217
help="Replace redacted PII with: hash or entity_name",

aai_cli/commands/stream/_exec.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from dataclasses import dataclass
1414
from pathlib import Path
1515

16+
from assemblyai import PIISubstitutionPolicy
1617
from assemblyai.streaming.v3 import Encoding, NoiseSuppressionModel, SpeechModel
1718

1819
from aai_cli import code_gen
@@ -67,7 +68,7 @@ class StreamOptions:
6768
filter_profanity: bool | None
6869
redact_pii: bool | None
6970
redact_pii_policy: str | None
70-
redact_pii_sub: str | None
71+
redact_pii_sub: PIISubstitutionPolicy | None
7172
webhook_url: str | None
7273
webhook_auth_header: str | None
7374
llm_prompt: list[str] | None
@@ -111,7 +112,7 @@ def base_flags(self) -> dict[str, object]:
111112
"voice_focus_threshold": self.voice_focus_threshold,
112113
"redact_pii": self.redact_pii,
113114
"redact_pii_policies": config_builder.split_csv(self.redact_pii_policy),
114-
"redact_pii_sub": self.redact_pii_sub,
115+
"redact_pii_sub": config_builder.enum_value(self.redact_pii_sub),
115116
"inactivity_timeout": self.inactivity_timeout,
116117
"webhook_url": self.webhook_url,
117118
"prompt": self.prompt,

aai_cli/commands/telemetry.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,24 @@ def enable(
9292

9393
def body(_state: AppState, json_mode: bool) -> None:
9494
config.set_telemetry_enabled(enabled=True)
95-
output.emit(
96-
{"telemetry_enabled": True},
97-
lambda _d: output.success("Telemetry enabled."),
98-
json_mode=json_mode,
99-
)
95+
# An env kill-switch (AAI_TELEMETRY_DISABLED / DO_NOT_TRACK) outranks the stored
96+
# choice, so persisting "enabled" wouldn't actually turn telemetry back on while
97+
# it's set — say so instead of an unqualified success.
98+
source = telemetry.consent_source()
99+
env_var = source.removeprefix("env:") if source.startswith("env:") else None
100+
101+
def render(_d: dict[str, bool]) -> object:
102+
line = output.success("Telemetry enabled.")
103+
if env_var is None:
104+
return line
105+
return output.stack(
106+
line,
107+
output.hint(
108+
f"Note: {env_var} is set, which keeps telemetry off until you unset it."
109+
),
110+
)
111+
112+
output.emit({"telemetry_enabled": True}, render, json_mode=json_mode)
100113

101114
run_command(ctx, body, json=json_out)
102115

aai_cli/commands/webhooks/__init__.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@
3737
def listen(
3838
ctx: typer.Context,
3939
port: int = typer.Option(
40-
8989, "--port", help="Local listener port (the first free port from here)"
40+
8989,
41+
"--port",
42+
min=0,
43+
max=65535,
44+
help="Local listener port (the first free port from here)",
4145
),
4246
forward_to: str | None = typer.Option(
4347
None, "--forward-to", help="Re-POST each delivery to this URL (e.g. your local app)"

aai_cli/core/llm.py

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from typing import TYPE_CHECKING
66

77
from aai_cli.core import environments
8-
from aai_cli.core.errors import APIError, UsageError
8+
from aai_cli.core.errors import APIError, UsageError, auth_failure
99

1010
if TYPE_CHECKING:
1111
from openai import OpenAI
@@ -116,12 +116,18 @@ def _client(api_key: str) -> OpenAI:
116116
)
117117

118118

119+
def _is_entitlement_denial(exc: object) -> bool:
120+
"""True when a gateway 401/403 reads as a plan-entitlement block rather than a
121+
bad key or an intercepting proxy."""
122+
text = f"{exc} {getattr(exc, 'body', None) or ''}".lower()
123+
return any(hint in text for hint in _ENTITLEMENT_HINTS)
124+
125+
119126
def _denial_suggestion(exc: object) -> str:
120127
"""Pick the suggestion for a gateway 401/403: point at billing only when the
121128
response actually mentions the plan entitlement, otherwise at key/network —
122129
a corporate-proxy 403 must not send users to the billing page."""
123-
text = f"{exc} {getattr(exc, 'body', None) or ''}".lower()
124-
if any(hint in text for hint in _ENTITLEMENT_HINTS):
130+
if _is_entitlement_denial(exc):
125131
return _PAID_PLAN_SUGGESTION
126132
return _ACCESS_DENIED_SUGGESTION
127133

@@ -159,9 +165,14 @@ def complete(
159165
)
160166
except (openai.AuthenticationError, openai.PermissionDeniedError) as exc:
161167
# The gateway returns 401/403 for an invalid key, a proxy block, and a
162-
# plan entitlement block ("no access to LLM Gateway"), so surface its
163-
# actual message and pick the suggestion from what it says — only an
164-
# entitlement message should point at billing.
168+
# plan entitlement block ("no access to LLM Gateway"). A plain 401
169+
# (AuthenticationError) with no entitlement hint is just a rejected key, so
170+
# surface the same clean exit-4 auth_failure transcribe gives instead of
171+
# echoing the gateway's raw 401 body. A 403 (proxy or entitlement) keeps the
172+
# gateway's own message and picks the suggestion from what it says — only an
173+
# entitlement message should point at billing, never a corporate-proxy 403.
174+
if isinstance(exc, openai.AuthenticationError) and not _is_entitlement_denial(exc):
175+
raise auth_failure() from exc
165176
raise APIError(
166177
f"LLM Gateway access denied: {exc}",
167178
suggestion=_denial_suggestion(exc),

aai_cli/main.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,19 @@ def main(
190190
try:
191191
environments.set_active(state.resolve_environment())
192192
except CLIError as err:
193-
output.emit_error(err, json_mode=json_mode)
194-
raise typer.Exit(code=err.exit_code) from None
193+
if err.error_type != "invalid_config":
194+
output.emit_error(err, json_mode=json_mode)
195+
raise typer.Exit(code=err.exit_code) from None
196+
# A corrupt config.toml can't tell us the profile's stored env. Defer the error
197+
# (run_command re-raises it for real commands) and fall back to the explicit
198+
# --env or the default, so `assembly config path` can still locate the file.
199+
# An explicit bad --env still wins — surface it now rather than the deferred one.
200+
state.deferred_config_error = err
201+
try:
202+
environments.set_active(environments.resolve(state.env, None))
203+
except CLIError as env_err:
204+
output.emit_error(env_err, json_mode=json_mode)
205+
raise typer.Exit(code=env_err.exit_code) from None
195206
active_env = environments.active()
196207
_LOG.debug("environment: %s (%s)", active_env.name, active_env.api_base)
197208
for warning in (conflict_warning, state.env_override_warning()):

0 commit comments

Comments
 (0)