diff --git a/src/fips_agents_cli/commands/patch.py b/src/fips_agents_cli/commands/patch.py index 400e9ff..d5e3784 100644 --- a/src/fips_agents_cli/commands/patch.py +++ b/src/fips_agents_cli/commands/patch.py @@ -8,6 +8,7 @@ from rich.table import Table from fips_agents_cli.tools.patching import ( + PatchUnsupportedForProjectType, check_for_updates, get_available_categories, get_project_type, @@ -48,7 +49,11 @@ def check(): console.print(f" Created: {template_info['project']['created_at']}\n") # Check for updates - updates = check_for_updates(project_root, template_info) + try: + updates = check_for_updates(project_root, template_info) + except PatchUnsupportedForProjectType as e: + console.print(f"\n[red]✗[/red] {e}") + sys.exit(1) if not updates: console.print("[green]✓[/green] Project is up to date with latest template!") @@ -203,13 +208,27 @@ def all_categories(dry_run: bool, skip_confirmation: bool): _, template_info = found project_type = get_project_type(template_info) + # Pre-clone fast-fail: enumerate built-in categories for the project type. + # If the type has no built-in set (e.g. sandbox), tell the user upfront — + # iterating manifest categories without a Click subcommand for each is a + # bigger refactor (see #50 / #45). + try: + categories = get_available_categories(project_type) + except ValueError: + console.print( + f"\n[red]✗[/red] Patching is not supported for project type " + f"'{project_type}'. The template repo for this project type does " + f"not yet ship a .fips-template.yaml manifest. See " + f"https://github.com/fips-agents/fips-agents-cli/issues/45." + ) + sys.exit(1) + if not skip_confirmation: confirm = click.confirm("This will update multiple files. Continue?", default=True) if not confirm: console.print("[yellow]Cancelled[/yellow]") sys.exit(0) - categories = get_available_categories(project_type) for category in categories: console.print(f"\n[bold]Processing category: {category}[/bold]") _patch_category(category, dry_run, skip_confirmation=skip_confirmation) diff --git a/src/fips_agents_cli/tools/patching.py b/src/fips_agents_cli/tools/patching.py index e4e6788..f9e40a9 100644 --- a/src/fips_agents_cli/tools/patching.py +++ b/src/fips_agents_cli/tools/patching.py @@ -26,6 +26,18 @@ # and warns the user. SUPPORTED_MANIFEST_SCHEMA_VERSION = 1 + +class PatchUnsupportedForProjectType(Exception): + """Raised when patch is invoked on a project whose template type has no + built-in category set and ships no usable .fips-template.yaml manifest. + + Today this hits ``sandbox`` projects (whose template repo has not yet + shipped a manifest) and any future project type added to ``create`` + before its template gains a manifest. Callers in the CLI layer catch + this and print a clean message instead of letting a traceback escape. + """ + + # File categories for MCP server projects MCP_FILE_CATEGORIES = { "generators": { @@ -368,6 +380,11 @@ def _resolve_categories( and well-formed. Falls back to the hardcoded constants keyed by ``template.type`` for any other case (file missing, malformed, unsupported schema version). + + Raises :class:`PatchUnsupportedForProjectType` when no manifest is + usable AND the project type has no built-in category set (today: + ``sandbox``). Callers in the CLI layer catch this and emit a clean + message instead of letting a traceback escape. """ manifest = _load_template_manifest(template_root) if manifest is not None: @@ -381,7 +398,17 @@ def _resolve_categories( ) project_type = get_project_type(template_info) - return get_categories_for_type(project_type) + try: + return get_categories_for_type(project_type) + except ValueError as e: + raise PatchUnsupportedForProjectType( + f"Patching is not supported for project type '{project_type}'. " + f"The template repo for this project type does not yet ship a " + f"{TEMPLATE_MANIFEST_FILENAME} manifest, and no built-in " + f"category set exists for it. See " + f"https://github.com/fips-agents/fips-agents-cli/issues/45 " + f"for the manifest schema." + ) from e def check_for_updates(project_path: Path, template_info: dict[str, Any]) -> dict[str, Any]: @@ -394,6 +421,11 @@ def check_for_updates(project_path: Path, template_info: dict[str, Any]) -> dict Returns: dict: Dictionary of categories with changed files + + Raises: + PatchUnsupportedForProjectType: When the project's template type has + no built-in category set and no usable manifest. Callers should + catch this and surface a clean message. """ template_url = template_info["template"]["url"] @@ -462,8 +494,16 @@ def patch_category( # manifest can override patterns/never_patch for known categories # but cannot introduce new category names — those would have no # Click subcommand registered for them anyway. - builtin_categories, _ = get_categories_for_type(project_type) - if category not in builtin_categories: + # + # When the project type has no built-in category set (sandbox today), + # we can't fast-fail — the manifest is the only source of truth, so + # we have to clone first and let _resolve_categories do its thing. + try: + builtin_categories, _ = get_categories_for_type(project_type) + except ValueError: + builtin_categories = None + + if builtin_categories is not None and category not in builtin_categories: available = ", ".join(builtin_categories.keys()) or "(none)" return ( False, @@ -482,7 +522,10 @@ def patch_category( # Resolve categories from manifest (when present) or hardcoded constants. # Done post-clone so a template's .fips-template.yaml can override the # patterns / ask_before_patch / never_patch for built-in categories. - file_categories, never_patch = _resolve_categories(template_root, template_info) + try: + file_categories, never_patch = _resolve_categories(template_root, template_info) + except PatchUnsupportedForProjectType as e: + return False, str(e) if category not in file_categories: # Manifest dropped a built-in category. Rare but possible. diff --git a/tests/test_patch.py b/tests/test_patch.py index a66ce77..778e6f1 100644 --- a/tests/test_patch.py +++ b/tests/test_patch.py @@ -487,6 +487,50 @@ def test_manifest_works_for_unsupported_project_type(self, tmp_path): assert never == [] +class TestPatchUnsupportedForProjectType: + """Issue #50: when the project type has no built-in category set and + no usable manifest, _resolve_categories raises a clean exception + that the CLI layer can catch — no Python tracebacks for sandbox + users (today) or for any future template that hasn't shipped a + manifest yet. + """ + + @pytest.mark.parametrize("project_type", ["sandbox", "bogus"]) + def test_resolve_categories_raises_when_no_manifest_and_unsupported_type( + self, tmp_path, project_type + ): + # No manifest in tmp_path, project type has no constants — must raise + info = {"template": {"type": project_type}} + with pytest.raises(patching.PatchUnsupportedForProjectType, match=project_type): + patching._resolve_categories(tmp_path, info) + + def test_resolve_categories_raises_when_manifest_malformed_and_unsupported(self, tmp_path): + # Manifest present but useless (missing required fields). Falls back + # to constants which raise — that ValueError must convert to the + # cleaner PatchUnsupportedForProjectType. + _write_manifest(tmp_path, "schema_version: 1\npatch:\n not_categories: 42\n") + info = {"template": {"type": "sandbox"}} + with pytest.raises(patching.PatchUnsupportedForProjectType, match="sandbox"): + patching._resolve_categories(tmp_path, info) + + def test_error_message_mentions_manifest_filename(self, tmp_path): + info = {"template": {"type": "sandbox"}} + try: + patching._resolve_categories(tmp_path, info) + except patching.PatchUnsupportedForProjectType as e: + assert ".fips-template.yaml" in str(e) + else: # pragma: no cover + pytest.fail("expected PatchUnsupportedForProjectType") + + def test_supported_types_still_work_without_manifest(self, tmp_path): + # Regression: agent / mcp-server / workflow must still fall back to + # constants without the new exception interfering. + for project_type in ("agent", "workflow", "mcp-server"): + info = {"template": {"type": project_type}} + cats, _ = patching._resolve_categories(tmp_path, info) + assert cats # non-empty + + # --------------------------------------------------------------------------- # Unit tests — _clone_template_for_patch handles subdirs # --------------------------------------------------------------------------- @@ -727,3 +771,129 @@ def fake_clone(url, target_path, branch=None): # Built-in agent categories that are NOT in the manifest must NOT show assert "chart" not in result.output assert "claude" not in result.output + + +# --------------------------------------------------------------------------- +# E2E test — issue #50: graceful error when project type has no manifest +# --------------------------------------------------------------------------- + + +def _make_fake_sandbox_project(project_root: Path) -> None: + """Build a minimal scaffolded sandbox project. Sandbox is the canonical + 'no manifest, no built-in category set' case today. + """ + project_root.mkdir(parents=True, exist_ok=True) + (project_root / "Makefile").write_text("# sandbox makefile\n") + (project_root / "pyproject.toml").write_text('[project]\nname = "my-sandbox"\n') + + info = { + "generator": {"tool": "fips-agents-cli", "version": "0.0.0-test"}, + "template": { + "url": "https://github.com/fips-agents/code-sandbox", + "type": "sandbox", + "commit": "abcdef123456", + "full_commit": "abcdef1234567890", + }, + "project": {"name": "my-sandbox", "created_at": "2026-01-01T00:00:00+00:00"}, + } + (project_root / ".template-info").write_text(json.dumps(info, indent=2)) + + +class TestPatchUnsupportedTypeE2E: + """Issue #50: gateway / ui / sandbox projects whose template ships no + manifest must produce clean errors, not Python tracebacks. + + Uses sandbox as the test subject because gateway / ui / agent / mcp + all ship manifests as of v0.12.0 — sandbox is the only type still + bare in the wild. + """ + + @pytest.fixture + def sandbox_project(self, tmp_path): + project = tmp_path / "my-sandbox" + _make_fake_sandbox_project(project) + return project + + def test_patch_check_exits_cleanly_with_helpful_message( + self, sandbox_project, monkeypatch, cli_runner + ): + # Clone returns a bare template with no manifest — the worst case + def fake_clone(url, target_path, branch=None): + target_path.mkdir(parents=True, exist_ok=True) + (target_path / "Makefile").write_text("# upstream\n") + return "deadbeef0000" + + monkeypatch.setattr(patching, "clone_template", fake_clone) + monkeypatch.chdir(sandbox_project) + + result = cli_runner.invoke(cli, ["patch", "check"]) + assert result.exit_code == 1, result.output + # No Python traceback escaped + assert "Traceback" not in result.output + # Clean message, mentions the project type and the manifest filename + assert "sandbox" in result.output + assert ".fips-template.yaml" in result.output + + def test_patch_category_exits_cleanly_with_helpful_message( + self, sandbox_project, monkeypatch, cli_runner + ): + def fake_clone(url, target_path, branch=None): + target_path.mkdir(parents=True, exist_ok=True) + return "deadbeef0000" + + monkeypatch.setattr(patching, "clone_template", fake_clone) + monkeypatch.chdir(sandbox_project) + + # Use any registered subcommand — `chart` is fine, the failure mode + # is "no built-in categories at all", not a category mismatch. + result = cli_runner.invoke(cli, ["patch", "chart"]) + assert result.exit_code == 1, result.output + assert "Traceback" not in result.output + assert "sandbox" in result.output + assert ".fips-template.yaml" in result.output + + def test_patch_all_exits_cleanly_with_helpful_message( + self, sandbox_project, monkeypatch, cli_runner + ): + # `patch all` fails at pre-clone (get_available_categories) for + # unsupported types — should not bubble ValueError. + called = {"clone": False} + + def fake_clone(url, target_path, branch=None): + called["clone"] = True + return "x" + + monkeypatch.setattr(patching, "clone_template", fake_clone) + monkeypatch.chdir(sandbox_project) + + result = cli_runner.invoke(cli, ["patch", "all"]) + assert result.exit_code == 1, result.output + assert "Traceback" not in result.output + assert "sandbox" in result.output + # Pre-clone fast-fail — should not have cloned anything + assert called["clone"] is False + + def test_manifest_makes_sandbox_patchable(self, sandbox_project, monkeypatch, cli_runner): + # Once the sandbox template ships a manifest, `patch check` works + # — this test future-proofs the fix. + def fake_clone(url, target_path, branch=None): + target_path.mkdir(parents=True, exist_ok=True) + (target_path / "Makefile").write_text("# upstream sandbox makefile\n") + (target_path / ".fips-template.yaml").write_text("""\ +schema_version: 1 +patch: + categories: + build: + patterns: [Makefile] + ask_before_patch: true + never_patch: [] +""") + return "deadbeef0000" + + monkeypatch.setattr(patching, "clone_template", fake_clone) + monkeypatch.chdir(sandbox_project) + + result = cli_runner.invoke(cli, ["patch", "check"]) + assert result.exit_code == 0, result.output + # Drift in build surfaces because the project's Makefile differs + assert "build" in result.output