From 0a8e5cb388b8971789a188bf0f488460eb9b4061 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Sun, 5 Apr 2026 09:40:20 +0000 Subject: [PATCH 1/2] fix(cli): display resource name in project show linked resources list - What was implemented - Added _resolve_resource_names() helper in src/cleveragents/cli/commands/project.py that queries the Resource Registry to map resource ULIDs to their namespaced names, with graceful fallback to None when the registry is unavailable or a resource has no name - Updated _project_spec_dict() to accept an optional resource_names dict parameter and include resource_name alongside resource_id in JSON/YAML output formats - Updated show command to resolve resource names before display, showing human-readable names (e.g. local/my-git-repo) instead of raw ULIDs, falling back to ULID when name is unavailable - Added BDD feature file features/project_show_resource_name.feature with 6 regression scenarios - Added step definitions features/steps/project_show_resource_name_steps.py - Key design decisions - Graceful degradation: if the Resource Registry is unavailable, the show command still works and falls back to displaying the raw ULID - Resources without names (auto-discovered) also fall back to ULID display - JSON/YAML output includes both resource_id and resource_name for completeness - The fix is minimal and non-breaking: _project_spec_dict() only includes resource_name when resource_names dict is explicitly passed - Rationale and implementation notes - _resolve_resource_names() provides a bounded, resilient means to enrich output with human-readable names without breaking on registry failures - The show command uses resolved names for display while preserving IDs as the underlying data source - Outputs (JSON/YAML) expose both IDs and names when available, ensuring downstream consumers have full context - Modules/Components Affected - src/cleveragents/cli/commands/project.py - tests/BDD: features/project_show_resource_name.feature - tests/BDD steps: features/steps/project_show_resource_name_steps.py - Backwards compatibility - Non-breaking: if resource_names is not provided or the registry is unavailable, behavior remains compatible by falling back to ULIDs - Testing - Added regression scenarios via the new feature file and step definitions to validate name resolution and fallback behavior ISSUES CLOSED: #2943 --- features/project_show_resource_name.feature | 71 ++++ .../steps/project_show_resource_name_steps.py | 337 ++++++++++++++++++ src/cleveragents/cli/commands/project.py | 100 +++++- 3 files changed, 495 insertions(+), 13 deletions(-) create mode 100644 features/project_show_resource_name.feature create mode 100644 features/steps/project_show_resource_name_steps.py diff --git a/features/project_show_resource_name.feature b/features/project_show_resource_name.feature new file mode 100644 index 000000000..aaa5a13f0 --- /dev/null +++ b/features/project_show_resource_name.feature @@ -0,0 +1,71 @@ +# Regression tests for bug #2943: agents project show must display resource +# names instead of raw ULIDs for linked resources. +Feature: Project show displays resource names for linked resources + As a developer using the agents CLI + I want "agents project show" to display human-readable resource names + So that I can identify linked resources without looking up ULIDs manually + + Background: + Given a fresh project-show-resource-name database is initialised + + @tdd_issue @tdd_issue_2943 + Scenario: Show displays resource name instead of ULID for a named resource + Given a project "local/my-app" exists in the resource-name DB + And a named resource "local/my-git-repo" is linked to project "local/my-app" in the resource-name DB + When I show project "local/my-app" via the resource-name CLI + Then the resource-name show output should contain "local/my-git-repo" + And the resource-name show exit code should be 0 + + @tdd_issue @tdd_issue_2943 + Scenario: Show falls back to ULID when resource has no name + Given a project "local/unnamed-res-proj" exists in the resource-name DB + And an unnamed resource is linked to project "local/unnamed-res-proj" in the resource-name DB + When I show project "local/unnamed-res-proj" via the resource-name CLI + Then the resource-name show output should contain the resource ULID + And the resource-name show exit code should be 0 + + @tdd_issue @tdd_issue_2943 + Scenario: Show displays resource name with read-only marker + Given a project "local/ro-proj" exists in the resource-name DB + And a named read-only resource "local/ro-resource" is linked to project "local/ro-proj" in the resource-name DB + When I show project "local/ro-proj" via the resource-name CLI + Then the resource-name show output should contain "local/ro-resource" + And the resource-name show output should contain "read-only" + And the resource-name show exit code should be 0 + + @tdd_issue @tdd_issue_2943 + Scenario: Show displays resource name with alias when set + Given a project "local/alias-proj" exists in the resource-name DB + And resource "local/aliased-resource" with alias "my-alias" is linked to project "local/alias-proj" in the resource-name DB + When I show project "local/alias-proj" via the resource-name CLI + Then the resource-name show output should contain "local/aliased-resource" + And the resource-name show output should contain "my-alias" + And the resource-name show exit code should be 0 + + @tdd_issue @tdd_issue_2943 + Scenario: JSON output includes both resource_id and resource_name + Given a project "local/json-proj" exists in the resource-name DB + And a named resource "local/json-resource" is linked to project "local/json-proj" in the resource-name DB + When I show project "local/json-proj" as JSON via the resource-name CLI + Then the resource-name show output should contain "resource_id" + And the resource-name show output should contain "resource_name" + And the resource-name show output should contain "local/json-resource" + And the resource-name show exit code should be 0 + + @tdd_issue @tdd_issue_2943 + Scenario: YAML output includes both resource_id and resource_name + Given a project "local/yaml-proj" exists in the resource-name DB + And a named resource "local/yaml-resource" is linked to project "local/yaml-proj" in the resource-name DB + When I show project "local/yaml-proj" as YAML via the resource-name CLI + Then the resource-name show output should contain "resource_id" + And the resource-name show output should contain "resource_name" + And the resource-name show output should contain "local/yaml-resource" + And the resource-name show exit code should be 0 + + @tdd_issue @tdd_issue_2943 + Scenario: Show gracefully handles registry unavailable by falling back to ULID + Given a project "local/fallback-proj" exists in the resource-name DB + And a resource is linked to project "local/fallback-proj" in the resource-name DB + When I show project "local/fallback-proj" with registry unavailable via the resource-name CLI + Then the resource-name show output should contain the resource ULID + And the resource-name show exit code should be 0 diff --git a/features/steps/project_show_resource_name_steps.py b/features/steps/project_show_resource_name_steps.py new file mode 100644 index 000000000..8fcb6542b --- /dev/null +++ b/features/steps/project_show_resource_name_steps.py @@ -0,0 +1,337 @@ +"""Step definitions for project_show_resource_name.feature. + +Regression tests for bug #2943: agents project show must display resource +names instead of raw ULIDs for linked resources. + +Uses the same monkey-patching pattern as project_cli_commands_steps.py to +exercise the actual CLI command functions under coverage. +""" + +from __future__ import annotations + +from io import StringIO +from typing import Any +from unittest.mock import MagicMock + +from behave import given, then, when # type: ignore[import-untyped] +from rich.console import Console +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +# --------------------------------------------------------------------------- +# Database / service bootstrap +# --------------------------------------------------------------------------- + + +class _SharedSession: + """Wraps a real SQLAlchemy Session but makes ``close()`` a no-op.""" + + def __init__(self, real: Any) -> None: + object.__setattr__(self, "_real", real) + + def close(self) -> None: + """No-op so the shared session stays usable across calls.""" + + def __getattr__(self, name: str) -> Any: + return getattr(object.__getattribute__(self, "_real"), name) + + def __setattr__(self, name: str, value: Any) -> None: + setattr(object.__getattribute__(self, "_real"), name, value) + + +def _init_rn_db(context: Any) -> None: + """Create a fresh in-memory database and wire up repos + service.""" + from cleveragents.application.services.resource_registry_service import ( + ResourceRegistryService, + ) + from cleveragents.infrastructure.database.models import Base + from cleveragents.infrastructure.database.repositories import ( + NamespacedProjectRepository, + ProjectResourceLinkRepository, + ) + + engine = create_engine( + "sqlite:///:memory:", + echo=False, + connect_args={"check_same_thread": False}, + ) + Base.metadata.create_all(engine) + + real_session = sessionmaker( + bind=engine, expire_on_commit=False, autoflush=True, autocommit=False + )() + wrapper = _SharedSession(real_session) + + def _shared_factory() -> Any: + return wrapper + + context._rn_engine = engine + context._rn_factory = _shared_factory + context._rn_project_repo = NamespacedProjectRepository( + session_factory=_shared_factory + ) + context._rn_link_repo = ProjectResourceLinkRepository( + session_factory=_shared_factory + ) + context._rn_resource_svc = ResourceRegistryService(session_factory=_shared_factory) + context._rn_resource_svc.bootstrap_builtin_types() + context._rn_output = "" + context._rn_failed = False + context._rn_last_resource_id: str | None = None + + +# --------------------------------------------------------------------------- +# Monkey-patching helpers +# --------------------------------------------------------------------------- + +_ORIG_FNS: dict[str, Any] = {} + + +def _patch_project_mod(context: Any, registry_override: Any = None) -> None: + """Monkey-patch the DI look-up helpers in project module.""" + import cleveragents.cli.commands.project as project_mod + + _ORIG_FNS["repo"] = project_mod._get_namespaced_project_repo + _ORIG_FNS["link"] = project_mod._get_resource_link_repo + _ORIG_FNS["svc"] = project_mod._get_resource_registry_service + + project_mod._get_namespaced_project_repo = lambda: context._rn_project_repo + project_mod._get_resource_link_repo = lambda: context._rn_link_repo + if registry_override is not None: + project_mod._get_resource_registry_service = lambda: registry_override + else: + project_mod._get_resource_registry_service = lambda: context._rn_resource_svc + + +def _unpatch_project_mod() -> None: + """Restore original helpers.""" + import cleveragents.cli.commands.project as project_mod + + project_mod._get_namespaced_project_repo = _ORIG_FNS["repo"] + project_mod._get_resource_link_repo = _ORIG_FNS["link"] + project_mod._get_resource_registry_service = _ORIG_FNS["svc"] + + +def _capture_show( + context: Any, + project_name: str, + output_format: str = "rich", + registry_override: Any = None, +) -> None: + """Invoke the show CLI function, capturing output.""" + import contextlib + + import typer + + import cleveragents.cli.commands.project as project_mod + + buf = StringIO() + fake_console = Console(file=buf, width=200, no_color=True) + + orig_console = project_mod.console + orig_err = project_mod.err_console + project_mod.console = fake_console + project_mod.err_console = fake_console + + _patch_project_mod(context, registry_override=registry_override) + failed = False + try: + with contextlib.redirect_stdout(buf): + project_mod.show(project=project_name, output_format=output_format) + except (SystemExit, typer.Exit, typer.Abort): + failed = True + except Exception: + failed = True + finally: + project_mod.console = orig_console + project_mod.err_console = orig_err + _unpatch_project_mod() + + context._rn_output = buf.getvalue() + context._rn_failed = failed + + +# --------------------------------------------------------------------------- +# Reusable helpers +# --------------------------------------------------------------------------- + + +def _create_project(context: Any, name: str) -> Any: + from cleveragents.domain.models.core.project import ( + NamespacedProject, + parse_namespaced_name, + ) + + parsed = parse_namespaced_name(name) + proj = NamespacedProject( + name=parsed.name, + namespace=parsed.namespace, + server=parsed.server, + ) + context._rn_project_repo.create(proj) + return proj + + +def _register_named_resource(context: Any, name: str) -> str: + """Register a resource with a namespaced name and return its resource_id.""" + res = context._rn_resource_svc.register_resource( + type_name="git-checkout", + name=name, + location="/tmp/test-resource", + description=f"Test resource {name}", + ) + context._rn_last_resource_id = res.resource_id + return res.resource_id + + +def _register_unnamed_resource(context: Any) -> str: + """Register a resource without a name (auto-discovered) and return its resource_id.""" + res = context._rn_resource_svc.register_resource( + type_name="git-checkout", + location="/tmp/unnamed-resource", + description="Unnamed test resource", + ) + context._rn_last_resource_id = res.resource_id + return res.resource_id + + +# --------------------------------------------------------------------------- +# Background +# --------------------------------------------------------------------------- + + +@given("a fresh project-show-resource-name database is initialised") +def step_init_rn_db(context: Any) -> None: + _init_rn_db(context) + + +# --------------------------------------------------------------------------- +# Given steps +# --------------------------------------------------------------------------- + + +@given('a project "{name}" exists in the resource-name DB') +def step_project_exists_rn(context: Any, name: str) -> None: + _create_project(context, name) + + +@given( + 'a named resource "{res_name}" is linked to project "{proj_name}" in the resource-name DB' +) +def step_named_resource_linked_rn(context: Any, res_name: str, proj_name: str) -> None: + rid = _register_named_resource(context, res_name) + context._rn_link_repo.create_link( + project_name=proj_name, + resource_id=rid, + ) + + +@given('an unnamed resource is linked to project "{proj_name}" in the resource-name DB') +def step_unnamed_resource_linked_rn(context: Any, proj_name: str) -> None: + rid = _register_unnamed_resource(context) + context._rn_link_repo.create_link( + project_name=proj_name, + resource_id=rid, + ) + + +@given( + 'a named read-only resource "{res_name}" is linked to project "{proj_name}" in the resource-name DB' +) +def step_named_ro_resource_linked_rn( + context: Any, res_name: str, proj_name: str +) -> None: + rid = _register_named_resource(context, res_name) + context._rn_link_repo.create_link( + project_name=proj_name, + resource_id=rid, + read_only=True, + ) + + +@given( + 'resource "{res_name}" with alias "{alias}" is linked to project "{proj_name}" in the resource-name DB' +) +def step_named_aliased_resource_linked_rn( + context: Any, res_name: str, alias: str, proj_name: str +) -> None: + rid = _register_named_resource(context, res_name) + context._rn_link_repo.create_link( + project_name=proj_name, + resource_id=rid, + alias=alias, + ) + + +@given('a resource is linked to project "{proj_name}" in the resource-name DB') +def step_resource_linked_rn(context: Any, proj_name: str) -> None: + """Link a resource (named or unnamed) to the project for fallback tests.""" + safe = proj_name.replace("/", "-") + rid = _register_named_resource(context, f"local/res-{safe}") + context._rn_link_repo.create_link( + project_name=proj_name, + resource_id=rid, + ) + + +# --------------------------------------------------------------------------- +# When steps +# --------------------------------------------------------------------------- + + +@when('I show project "{name}" via the resource-name CLI') +def step_show_project_rn(context: Any, name: str) -> None: + _capture_show(context, name) + + +@when('I show project "{name}" as JSON via the resource-name CLI') +def step_show_project_json_rn(context: Any, name: str) -> None: + _capture_show(context, name, output_format="json") + + +@when('I show project "{name}" as YAML via the resource-name CLI') +def step_show_project_yaml_rn(context: Any, name: str) -> None: + _capture_show(context, name, output_format="yaml") + + +@when('I show project "{name}" with registry unavailable via the resource-name CLI') +def step_show_project_registry_unavailable_rn(context: Any, name: str) -> None: + """Simulate registry unavailability by using a mock that raises on every call.""" + broken_registry = MagicMock() + broken_registry.show_resource.side_effect = Exception("Registry unavailable") + _capture_show(context, name, registry_override=broken_registry) + + +# --------------------------------------------------------------------------- +# Then steps +# --------------------------------------------------------------------------- + + +@then('the resource-name show output should contain "{text}"') +def step_rn_output_contains(context: Any, text: str) -> None: + assert text.lower() in context._rn_output.lower(), ( + f"Expected output to contain '{text}', got:\n{context._rn_output}" + ) + + +@then("the resource-name show output should contain the resource ULID") +def step_rn_output_contains_ulid(context: Any) -> None: + rid = context._rn_last_resource_id + assert rid is not None, "No resource ULID stored in context" + assert rid in context._rn_output, ( + f"Expected output to contain ULID '{rid}', got:\n{context._rn_output}" + ) + + +@then("the resource-name show exit code should be 0") +def step_rn_exit_code_zero(context: Any) -> None: + assert not context._rn_failed, ( + f"Expected command to succeed but it failed. Output:\n{context._rn_output}" + ) + + +@then("the resource-name show exit code should not be 0") +def step_rn_exit_code_nonzero(context: Any) -> None: + assert context._rn_failed, ( + f"Expected command to fail but it succeeded. Output:\n{context._rn_output}" + ) diff --git a/src/cleveragents/cli/commands/project.py b/src/cleveragents/cli/commands/project.py index 9fb55de18..c3cfeb551 100644 --- a/src/cleveragents/cli/commands/project.py +++ b/src/cleveragents/cli/commands/project.py @@ -18,6 +18,7 @@ Based on ADR-009 (CLI Framework) and implementation_plan.md task B0.cli.projects from __future__ import annotations +import logging import re from datetime import UTC, datetime from pathlib import Path @@ -39,6 +40,8 @@ from cleveragents.core.exceptions import ( ValidationError, ) +logger = logging.getLogger(__name__) + # Create sub-app for project commands app = typer.Typer(help="Project management commands") file_filter_app = typer.Typer( @@ -151,24 +154,88 @@ def _store_project_extras( session.close() -def _project_spec_dict(project: Any) -> dict[str, object]: +def _resolve_resource_names( + resource_ids: list[str], +) -> dict[str, str | None]: + """Resolve resource ULIDs to their namespaced names via the Resource Registry. + + Returns a mapping of ``resource_id -> resource_name`` for each ID. + Resources without a name (auto-discovered) map to ``None``. + If the Resource Registry is unavailable, all IDs map to ``None`` + so callers can fall back to displaying the raw ULID. + + NOTE: This function performs one registry lookup per resource ID (N+1 pattern). + For the ``show`` command this is acceptable because a project typically has a + small number of linked resources (1-5) and this is a read-only CLI display path, + not a hot loop. If the ResourceRegistryService ever gains a batch lookup method + (e.g. ``show_resources(ids: list[str])``), this should be updated to use it. + + Args: + resource_ids: List of resource ULIDs to resolve. + + Returns: + Dict mapping each resource_id to its name (or ``None`` on failure). + """ + result: dict[str, str | None] = {rid: None for rid in resource_ids} + if not resource_ids: + return result + try: + registry = _get_resource_registry_service() + for rid in resource_ids: + try: + resource = registry.show_resource(rid) + # Normalise empty-string names to None so all output formats + # (Rich display and JSON/YAML) treat them identically as + # "no name available" and fall back to the ULID. + result[rid] = resource.name or None + except (NotFoundError, CleverAgentsError) as exc: + # Resource not found or a known registry error — keep None + # (fallback to ULID display). + logger.debug("Could not resolve resource name for %s: %s", rid, exc) + result[rid] = None + except Exception: + # Registry service unavailable (e.g. DI container not initialised, + # network partition) — log a warning and fall back to ULID display + # for all resources so the show command continues to function. + logger.warning( + "Resource Registry unavailable; falling back to ULID display for all" + " linked resources", + exc_info=True, + ) + return result + + +def _project_spec_dict( + project: Any, + resource_names: dict[str, str | None] | None = None, +) -> dict[str, object]: """Return project data as a dict using spec field names. Keys: namespaced_name, namespace, name, description, linked_resources, created_at, updated_at. + + Each entry in ``linked_resources`` includes both ``resource_id`` and + ``resource_name`` so that JSON/YAML output formats expose both values. + + Args: + project: The project domain object. + resource_names: Optional pre-resolved mapping of resource_id to name. + When provided, ``resource_name`` is populated from this map. + When ``None``, names are not resolved (``resource_name`` is omitted). """ linked: list[dict[str, object]] = [] for lr in project.linked_resources: - linked.append( - { - "resource_id": lr.resource_id, - "read_only": lr.project_read_only, - "alias": lr.alias, - "linked_at": lr.linked_at.isoformat() - if hasattr(lr.linked_at, "isoformat") - else str(lr.linked_at), - } - ) + entry: dict[str, object] = { + "resource_id": lr.resource_id, + "read_only": lr.project_read_only, + "alias": lr.alias, + "linked_at": lr.linked_at.isoformat() + if hasattr(lr.linked_at, "isoformat") + else str(lr.linked_at), + } + if resource_names is not None: + entry["resource_name"] = resource_names.get(lr.resource_id) + linked.append(entry) return { "namespaced_name": project.namespaced_name, @@ -894,7 +961,12 @@ def show( err_console.print(f"[red]Project not found:[/red] {project}") raise typer.Exit(1) from exc - data = _project_spec_dict(proj) + # Resolve resource names from the Resource Registry so the display + # shows human-readable names instead of raw ULIDs (bug #2943). + resource_ids = [lr.resource_id for lr in proj.linked_resources] + res_names = _resolve_resource_names(resource_ids) + + data = _project_spec_dict(proj, resource_names=res_names) if output_format.lower() == OutputFormat.RICH: lines: list[str] = [ @@ -912,7 +984,9 @@ def show( for lr in proj.linked_resources: ro_marker = " [dim](read-only)[/dim]" if lr.project_read_only else "" alias_marker = f" alias={lr.alias}" if lr.alias else "" - lines.append(f" - {lr.resource_id}{ro_marker}{alias_marker}") + # Display resource name when available; fall back to ULID + display_name = res_names.get(lr.resource_id) or lr.resource_id + lines.append(f" - {display_name}{ro_marker}{alias_marker}") else: lines.append("\n[bold]Linked Resources:[/bold] (none)") -- 2.52.0 From 4393de4830f15aea552defbb27fe702cd655ea3d Mon Sep 17 00:00:00 2001 From: controller-ci-rerun Date: Sat, 30 May 2026 15:43:37 -0400 Subject: [PATCH 2/2] chore: re-trigger CI [controller] -- 2.52.0