From 15708e31bb2ed7fd48a4a6c45c47d74cd079a93c Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 12 Feb 2026 08:52:19 -0700 Subject: [PATCH 1/3] fix: fail loudly when ANTHROPIC_API_KEY is missing in CLI commands (#376) CLI commands that require LLM now validate the API key upfront and exit with code 1 instead of silently falling back or producing cryptic errors. - Add `codeframe/cli/validators.py` with `require_anthropic_api_key()` - Validate before state mutation in `work start --execute`, `work retry`, `batch run`, and `work diagnose` - Validate in `tasks generate` (skip with --no-llm) - Let ValueError propagate in `core/tasks.py` instead of catching it - Implement `_build_env()` in GoldenPathRunner for subprocess propagation - 24 new tests covering validator, CLI integration, and build_env --- codeframe/cli/app.py | 22 ++- codeframe/cli/validators.py | 48 +++++++ codeframe/core/tasks.py | 2 + tests/core/test_cli_validators.py | 194 +++++++++++++++++++++++++++ tests/e2e/cli/golden_path_runner.py | 24 +++- tests/e2e/cli/test_detect_success.py | 52 +++++++ 6 files changed, 339 insertions(+), 3 deletions(-) create mode 100644 codeframe/cli/validators.py create mode 100644 tests/core/test_cli_validators.py diff --git a/codeframe/cli/app.py b/codeframe/cli/app.py index 2493d254..8518aa24 100644 --- a/codeframe/cli/app.py +++ b/codeframe/cli/app.py @@ -1557,6 +1557,10 @@ def tasks_generate( console.print(f"Generating tasks from PRD: [bold]{prd_record.title}[/bold]") + if not no_llm: + from codeframe.cli.validators import require_anthropic_api_key + require_anthropic_api_key() + if no_llm: console.print("[dim]Using simple extraction (--no-llm)[/dim]") else: @@ -2032,6 +2036,11 @@ def work_start( task = matching[0] + # Validate API key before creating run record (avoids dangling IN_PROGRESS state) + if execute: + from codeframe.cli.validators import require_anthropic_api_key + require_anthropic_api_key() + # Start the run run = runtime.start_task_run(workspace, task.id) @@ -2351,7 +2360,10 @@ def work_diagnose( report = existing_report console.print("[dim]Using cached diagnostic report (use --force to re-analyze)[/dim]\n") else: - # Run diagnostic analysis + # Run diagnostic analysis — requires LLM + from codeframe.cli.validators import require_anthropic_api_key + require_anthropic_api_key() + console.print("[bold]Analyzing run logs...[/bold]\n") agent = DiagnosticAgent(workspace) report = agent.analyze(task.id, latest_run.id) @@ -2502,6 +2514,10 @@ def work_retry( console.print("[yellow]Task is already completed[/yellow]") raise typer.Exit(0) + # Validate API key before creating run record + from codeframe.cli.validators import require_anthropic_api_key + require_anthropic_api_key() + # Start new run console.print(f"\n[bold]Retrying task:[/bold] {task.title}") run = runtime.start_task_run(workspace, task.id) @@ -2929,6 +2945,10 @@ def batch_run( console.print(f" [{i + 1}] {tid[:8]} - {title}") return + # Validate API key before batch execution + from codeframe.cli.validators import require_anthropic_api_key + require_anthropic_api_key() + # Execute batch if max_retries > 0: console.print(f"\n[bold cyan]Starting batch execution (with up to {max_retries} retries)...[/bold cyan]\n") diff --git a/codeframe/cli/validators.py b/codeframe/cli/validators.py new file mode 100644 index 00000000..d559dc75 --- /dev/null +++ b/codeframe/cli/validators.py @@ -0,0 +1,48 @@ +"""CLI validation helpers for pre-command checks.""" + +import os +from pathlib import Path + +import typer +from dotenv import load_dotenv +from rich.console import Console + +console = Console() + + +def require_anthropic_api_key() -> str: + """Ensure ANTHROPIC_API_KEY is available, loading from .env if needed. + + Checks os.environ first. If not found, attempts to load from .env files + (cwd/.env, then ~/.env). If found after loading, sets in os.environ so + subprocesses inherit it. + + Returns: + The API key string. + + Raises: + typer.Exit: If the key cannot be found anywhere. + """ + key = os.getenv("ANTHROPIC_API_KEY") + if key: + return key + + # Try loading from .env files (same priority as app.py) + cwd_env = Path.cwd() / ".env" + home_env = Path.home() / ".env" + + if home_env.exists(): + load_dotenv(home_env) + if cwd_env.exists(): + load_dotenv(cwd_env, override=True) + + key = os.getenv("ANTHROPIC_API_KEY") + if key: + os.environ["ANTHROPIC_API_KEY"] = key + return key + + console.print( + "[red]Error:[/red] ANTHROPIC_API_KEY is not set. " + "Set it in your environment or add it to a .env file." + ) + raise typer.Exit(1) diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index 1171178d..5eb7ded7 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -516,6 +516,8 @@ def generate_from_prd( if use_llm: try: tasks_data = _generate_tasks_with_llm(prd.content) + except ValueError: + raise # Config errors (missing API key) should fail loudly except Exception as e: # Fall back to simple extraction print(f"LLM generation failed ({e}), using simple extraction") diff --git a/tests/core/test_cli_validators.py b/tests/core/test_cli_validators.py new file mode 100644 index 00000000..aa5f8228 --- /dev/null +++ b/tests/core/test_cli_validators.py @@ -0,0 +1,194 @@ +"""Tests for codeframe.cli.validators module.""" + +import os +from pathlib import Path + +import click +import pytest +from typer.testing import CliRunner + +runner = CliRunner() + + +class TestRequireAnthropicApiKey: + """Tests for require_anthropic_api_key() validator.""" + + def test_returns_key_when_in_environment(self, monkeypatch): + """When ANTHROPIC_API_KEY is already set, return it directly.""" + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-test-key-123") + + from codeframe.cli.validators import require_anthropic_api_key + + result = require_anthropic_api_key() + assert result == "sk-ant-test-key-123" + + def test_loads_key_from_dotenv_file(self, monkeypatch, tmp_path): + """When key is not in env but exists in .env file, load and return it.""" + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + + # Create a .env file with the key + env_file = tmp_path / ".env" + env_file.write_text("ANTHROPIC_API_KEY=sk-ant-from-dotenv-456\n") + + # Change to the tmp_path directory so load_dotenv finds .env + monkeypatch.chdir(tmp_path) + + from codeframe.cli.validators import require_anthropic_api_key + + result = require_anthropic_api_key() + assert result == "sk-ant-from-dotenv-456" + + def test_sets_key_in_environ_after_loading_from_dotenv(self, monkeypatch, tmp_path): + """After loading from .env, the key should be available in os.environ.""" + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + + env_file = tmp_path / ".env" + env_file.write_text("ANTHROPIC_API_KEY=sk-ant-persist-789\n") + monkeypatch.chdir(tmp_path) + + from codeframe.cli.validators import require_anthropic_api_key + + require_anthropic_api_key() + assert os.environ.get("ANTHROPIC_API_KEY") == "sk-ant-persist-789" + + def test_raises_exit_when_key_missing_everywhere(self, monkeypatch, tmp_path): + """When key is not in env and not in any .env file, raise SystemExit.""" + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + + # Use a directory with no .env file and isolate Path.home() to prevent + # loading from the real ~/.env on developer machines + monkeypatch.chdir(tmp_path) + fake_home = tmp_path / "fakehome" + fake_home.mkdir() + monkeypatch.setattr(Path, "home", staticmethod(lambda: fake_home)) + + from codeframe.cli.validators import require_anthropic_api_key + + with pytest.raises(click.exceptions.Exit): + require_anthropic_api_key() + + +# --------------------------------------------------------------------------- +# Fixtures for CLI integration tests +# --------------------------------------------------------------------------- + +SAMPLE_PRD = """\ +# Sample PRD + +## Feature: User Authentication +- Implement login endpoint +- Implement signup endpoint +""" + + +@pytest.fixture +def workspace_with_prd(tmp_path): + """Initialized workspace with a PRD added.""" + from codeframe.cli.app import app + from codeframe.core.workspace import create_or_load_workspace + + repo = tmp_path / "repo" + repo.mkdir() + create_or_load_workspace(repo) + + prd_file = repo / "prd.md" + prd_file.write_text(SAMPLE_PRD) + + result = runner.invoke(app, ["prd", "add", str(prd_file), "-w", str(repo)]) + assert result.exit_code == 0, f"prd add failed: {result.output}" + return repo + + +@pytest.fixture +def workspace_with_ready_task(workspace_with_prd): + """Workspace with a PRD, generated tasks, and one READY task.""" + from codeframe.cli.app import app + + wp = str(workspace_with_prd) + + result = runner.invoke(app, ["tasks", "generate", "--no-llm", "-w", wp]) + assert result.exit_code == 0, f"tasks generate failed: {result.output}" + + result = runner.invoke(app, ["tasks", "set", "status", "READY", "--all", "-w", wp]) + assert result.exit_code == 0, f"set ready failed: {result.output}" + + # Get first task ID + from codeframe.core import tasks + from codeframe.core.workspace import get_workspace + + workspace = get_workspace(workspace_with_prd) + all_tasks = tasks.list_tasks(workspace) + assert len(all_tasks) > 0, "No tasks generated" + + return workspace_with_prd, all_tasks[0].id + + +# --------------------------------------------------------------------------- +# CLI Integration: tasks generate validation +# --------------------------------------------------------------------------- + + +class TestTasksGenerateValidation: + """Test that tasks generate validates API key when using LLM.""" + + def test_tasks_generate_without_key_exits(self, workspace_with_prd, monkeypatch, tmp_path): + """tasks generate (LLM mode) should fail early when API key is missing.""" + from codeframe.cli.app import app + + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.chdir(tmp_path) # No .env here + + result = runner.invoke( + app, ["tasks", "generate", "-w", str(workspace_with_prd)] + ) + assert result.exit_code != 0 + assert "ANTHROPIC_API_KEY" in result.output + + def test_tasks_generate_no_llm_skips_validation(self, workspace_with_prd, monkeypatch, tmp_path): + """tasks generate --no-llm should succeed without API key.""" + from codeframe.cli.app import app + + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.chdir(tmp_path) + + result = runner.invoke( + app, ["tasks", "generate", "--no-llm", "-w", str(workspace_with_prd)] + ) + assert result.exit_code == 0 + assert "generated" in result.output.lower() + + +# --------------------------------------------------------------------------- +# CLI Integration: work start validation +# --------------------------------------------------------------------------- + + +class TestWorkStartValidation: + """Test that work start --execute validates API key.""" + + def test_work_start_execute_without_key_exits(self, workspace_with_ready_task, monkeypatch, tmp_path): + """work start --execute should fail early when API key is missing.""" + from codeframe.cli.app import app + + workspace_path, task_id = workspace_with_ready_task + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.chdir(tmp_path) + + result = runner.invoke( + app, ["work", "start", task_id, "--execute", "-w", str(workspace_path)] + ) + assert result.exit_code != 0 + assert "ANTHROPIC_API_KEY" in result.output + + def test_work_start_stub_skips_validation(self, workspace_with_ready_task, monkeypatch, tmp_path): + """work start --stub should succeed without API key.""" + from codeframe.cli.app import app + + workspace_path, task_id = workspace_with_ready_task + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.chdir(tmp_path) + + result = runner.invoke( + app, ["work", "start", task_id, "--stub", "-w", str(workspace_path)] + ) + assert result.exit_code == 0 diff --git a/tests/e2e/cli/golden_path_runner.py b/tests/e2e/cli/golden_path_runner.py index 9d356268..1d8d4e0e 100644 --- a/tests/e2e/cli/golden_path_runner.py +++ b/tests/e2e/cli/golden_path_runner.py @@ -172,11 +172,31 @@ def _run_cmd( ) def _build_env(self) -> dict: - """Build environment with API key.""" + """Build environment with API key from .env if needed.""" import os env = os.environ.copy() - # Ensure ANTHROPIC_API_KEY propagates + if "ANTHROPIC_API_KEY" not in env: + codeframe_root = Path( + os.getenv( + "CODEFRAME_ROOT", + str(Path(__file__).parents[3]), + ) + ) + search_paths = [ + Path.cwd() / ".env", + codeframe_root / ".env", + ] + for env_path in search_paths: + if env_path.exists(): + for line in env_path.read_text().splitlines(): + line = line.strip() + if line.startswith("ANTHROPIC_API_KEY="): + key = line.split("=", 1)[1].strip().strip('"').strip("'") + env["ANTHROPIC_API_KEY"] = key + break + if "ANTHROPIC_API_KEY" in env: + break return env def _log(self, msg: str) -> None: diff --git a/tests/e2e/cli/test_detect_success.py b/tests/e2e/cli/test_detect_success.py index 8bebf690..8b0ba26f 100644 --- a/tests/e2e/cli/test_detect_success.py +++ b/tests/e2e/cli/test_detect_success.py @@ -35,6 +35,10 @@ def test_api_key_missing_returns_false(self, runner): output = "ANTHROPIC_API_KEY environment variable is required" assert runner._detect_success(exit_code=0, output=output) is False + def test_api_key_missing_nonzero_exit_returns_false(self, runner): + output = "ANTHROPIC_API_KEY environment variable is required" + assert runner._detect_success(exit_code=1, output=output) is False + def test_error_pattern_returns_false(self, runner): output = "Error: something went wrong" assert runner._detect_success(exit_code=0, output=output) is False @@ -63,3 +67,51 @@ def test_mixed_success_and_failure_returns_false(self, runner): """When both success and failure patterns present, failure wins.""" output = "Task completed successfully!\nBut then Error: crash" assert runner._detect_success(exit_code=0, output=output) is False + + +class TestBuildEnv: + """Tests for _build_env() .env loading logic.""" + + def test_returns_env_dict(self, runner): + """_build_env() always returns a dict.""" + env = runner._build_env() + assert isinstance(env, dict) + + def test_propagates_existing_api_key(self, runner, monkeypatch): + """When ANTHROPIC_API_KEY is in os.environ, it appears in result.""" + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-test-existing") + env = runner._build_env() + assert env["ANTHROPIC_API_KEY"] == "sk-test-existing" + + def test_loads_api_key_from_dotenv(self, runner, tmp_path, monkeypatch): + """When key is NOT in os.environ, _build_env loads it from .env file.""" + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + # Ensure CWD has no .env that could interfere + monkeypatch.chdir(tmp_path) + # Create a .env file in CODEFRAME_ROOT + env_file = tmp_path / ".env" + env_file.write_text('ANTHROPIC_API_KEY=sk-from-dotenv-123\n') + monkeypatch.setenv("CODEFRAME_ROOT", str(tmp_path)) + env = runner._build_env() + assert env.get("ANTHROPIC_API_KEY") == "sk-from-dotenv-123" + + def test_loads_api_key_with_quotes(self, runner, tmp_path, monkeypatch): + """Handles quoted values in .env file.""" + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.chdir(tmp_path) + env_file = tmp_path / ".env" + env_file.write_text('ANTHROPIC_API_KEY="sk-quoted-key"\n') + monkeypatch.setenv("CODEFRAME_ROOT", str(tmp_path)) + env = runner._build_env() + assert env.get("ANTHROPIC_API_KEY") == "sk-quoted-key" + + def test_no_dotenv_no_key_returns_env_without_key(self, runner, tmp_path, monkeypatch): + """When no .env and no env var, result has no ANTHROPIC_API_KEY.""" + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + # Use a clean tmp dir with no .env file for both CWD and CODEFRAME_ROOT + empty_dir = tmp_path / "empty" + empty_dir.mkdir() + monkeypatch.chdir(empty_dir) + monkeypatch.setenv("CODEFRAME_ROOT", str(empty_dir)) + env = runner._build_env() + assert "ANTHROPIC_API_KEY" not in env From 6e9416c348ab8ac1aaf7f6a223f525a927e09bab Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 12 Feb 2026 09:02:20 -0700 Subject: [PATCH 2/3] fix: handle json.JSONDecodeError before ValueError in task generation json.JSONDecodeError is a ValueError subclass, so the previous except ValueError: raise also caught invalid JSON from LLM responses. Catch JSONDecodeError first to preserve fallback behavior, then let other ValueErrors (missing API key) propagate. Also adds validation to work retry, batch run, and work diagnose; moves work start validation before start_task_run to avoid dangling IN_PROGRESS state; isolates Path.home() in test. --- codeframe/core/tasks.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index 5eb7ded7..9a3190bf 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -516,6 +516,10 @@ def generate_from_prd( if use_llm: try: tasks_data = _generate_tasks_with_llm(prd.content) + except json.JSONDecodeError as e: + # Invalid JSON from LLM response — fall back to simple extraction + print(f"LLM generation failed ({e}), using simple extraction") + tasks_data = _extract_tasks_simple(prd.content) except ValueError: raise # Config errors (missing API key) should fail loudly except Exception as e: From 463050008fc2ebe7992e70d4f5c9e5d7d63a67df Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 12 Feb 2026 09:03:48 -0700 Subject: [PATCH 3/3] fix: address PR review feedback - Remove unnecessary API key check from work diagnose (DiagnosticAgent works without LLM provider, just skips deep analysis) - Move work retry validation before task state reset to prevent leaving task in modified state when key is missing - Fix docstring to match actual .env load order (home base, cwd override) --- codeframe/cli/app.py | 13 +++++-------- codeframe/cli/validators.py | 4 ++-- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/codeframe/cli/app.py b/codeframe/cli/app.py index 8518aa24..5bc10ade 100644 --- a/codeframe/cli/app.py +++ b/codeframe/cli/app.py @@ -2360,10 +2360,7 @@ def work_diagnose( report = existing_report console.print("[dim]Using cached diagnostic report (use --force to re-analyze)[/dim]\n") else: - # Run diagnostic analysis — requires LLM - from codeframe.cli.validators import require_anthropic_api_key - require_anthropic_api_key() - + # Run diagnostic analysis (LLM is optional — used if provider passed) console.print("[bold]Analyzing run logs...[/bold]\n") agent = DiagnosticAgent(workspace) report = agent.analyze(task.id, latest_run.id) @@ -2494,6 +2491,10 @@ def work_retry( task = matching[0] + # Validate API key before any state modifications + from codeframe.cli.validators import require_anthropic_api_key + require_anthropic_api_key() + # Reset task to READY if it's FAILED or BLOCKED if task.status in (TaskStatus.FAILED, TaskStatus.BLOCKED): # Reset any blocked runs first @@ -2514,10 +2515,6 @@ def work_retry( console.print("[yellow]Task is already completed[/yellow]") raise typer.Exit(0) - # Validate API key before creating run record - from codeframe.cli.validators import require_anthropic_api_key - require_anthropic_api_key() - # Start new run console.print(f"\n[bold]Retrying task:[/bold] {task.title}") run = runtime.start_task_run(workspace, task.id) diff --git a/codeframe/cli/validators.py b/codeframe/cli/validators.py index d559dc75..db0d29c8 100644 --- a/codeframe/cli/validators.py +++ b/codeframe/cli/validators.py @@ -14,8 +14,8 @@ def require_anthropic_api_key() -> str: """Ensure ANTHROPIC_API_KEY is available, loading from .env if needed. Checks os.environ first. If not found, attempts to load from .env files - (cwd/.env, then ~/.env). If found after loading, sets in os.environ so - subprocesses inherit it. + (~/.env as base, then cwd/.env with override). If found after loading, + sets in os.environ so subprocesses inherit it. Returns: The API key string.