Skip to content

Commit 60961c3

Browse files
committed
Fix codex review findings
1 parent 9eb9557 commit 60961c3

File tree

5 files changed

+52
-35
lines changed

5 files changed

+52
-35
lines changed

src/specfact_cli/modules/module_registry/module-package.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: module-registry
2-
version: 0.1.4
2+
version: 0.1.5
33
commands:
44
- module
55
command_help:
@@ -15,5 +15,5 @@ publisher:
1515
description: 'Manage modules: search, list, show, install, and upgrade.'
1616
license: Apache-2.0
1717
integrity:
18-
checksum: sha256:d1c9a6082275c8b073b594214849bbdb0260dd6eba1e76cedebb90dd4d411653
19-
signature: 2KCvzV/LqrHro8uhCCCD70bPtuxHtYSPLMr5CPHv2WS8KrculbtS282zdSXuUxcxDoavvjvF7h2LuTLKzMLqBA==
18+
checksum: sha256:4837d40c55ebde6eba87b434c3ec3ae3d0d842eb6a6984d4212ffbc6fd26eac2
19+
signature: m2tJyNfaHOnil3dsT5NxUB93+4nnVJHBaF7QzQf/DC8F/LG7oJJMWHU063HY9x2/d9hFVXLwItf9TNgNjnirDQ==

src/specfact_cli/modules/module_registry/src/commands.py

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -263,19 +263,16 @@ def uninstall(
263263
@beartype
264264
def alias_create(
265265
alias_name: str = typer.Argument(..., help="Alias (command name) to map"),
266-
module_id: str = typer.Argument(..., help="Namespaced module id (e.g. acme-corp/backlog-pro)"),
266+
command_name: str = typer.Argument(..., help="Command name to invoke (e.g. backlog, module)"),
267267
force: bool = typer.Option(False, "--force", help="Allow alias to shadow built-in command"),
268268
) -> None:
269-
"""Create an alias mapping a command name to a namespaced module."""
270-
if "/" not in module_id.strip():
271-
console.print("[red]module_id must be namespace/name (e.g. acme-corp/backlog-pro).[/red]")
272-
raise typer.Exit(1)
269+
"""Create an alias mapping a custom name to a registered command."""
273270
try:
274-
create_alias(alias_name.strip(), module_id.strip(), force=force)
271+
create_alias(alias_name.strip(), command_name.strip(), force=force)
275272
except ValueError as exc:
276273
console.print(f"[red]{exc}[/red]")
277274
raise typer.Exit(1) from exc
278-
console.print(f"[green]Alias[/green] {alias_name!r} -> {module_id!r}")
275+
console.print(f"[green]Alias[/green] {alias_name!r} -> {command_name!r}")
279276

280277

281278
@alias_app.command(name="list")
@@ -288,7 +285,7 @@ def alias_list() -> None:
288285
return
289286
table = Table(title="Aliases")
290287
table.add_column("Alias", style="cyan")
291-
table.add_column("Module", style="green")
288+
table.add_column("Command", style="green")
292289
for alias, mod in sorted(aliases.items()):
293290
table.add_row(alias, mod)
294291
console.print(table)
@@ -312,7 +309,7 @@ def alias_remove(
312309
@beartype
313310
def add_registry_cmd(
314311
url: str = typer.Argument(..., help="Registry index URL (e.g. https://company.com/index.json)"),
315-
id: str = typer.Option(None, "--id", help="Registry id (default: derived from URL)"),
312+
id: str | None = typer.Option(None, "--id", help="Registry id (default: derived from URL)"),
316313
priority: int | None = typer.Option(None, "--priority", help="Priority (default: next available)"),
317314
trust: str = typer.Option("prompt", "--trust", help="Trust level: always, prompt, or never"),
318315
) -> None:

src/specfact_cli/registry/alias_manager.py

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
"""Alias storage and resolution: command name -> namespaced module ID."""
1+
"""Alias storage and resolution: alias -> command name."""
22

33
from __future__ import annotations
44

@@ -30,12 +30,12 @@ def _builtin_command_names() -> set[str]:
3030

3131
@beartype
3232
@require(lambda alias: alias.strip() != "", "alias must be non-empty")
33-
@require(lambda module_id: "/" in module_id and len(module_id.strip()) > 0, "module_id must be namespace/name")
33+
@require(lambda command_name: command_name.strip() != "", "command_name must be non-empty")
3434
@ensure(lambda: True, "no postcondition on void")
35-
def create_alias(alias: str, module_id: str, force: bool = False) -> None:
36-
"""Store alias -> module_id in aliases.json. Warn or raise if alias shadows built-in."""
35+
def create_alias(alias: str, command_name: str, force: bool = False) -> None:
36+
"""Store alias -> command_name in aliases.json. Warn or raise if alias shadows built-in."""
3737
alias = alias.strip()
38-
module_id = module_id.strip()
38+
command_name = command_name.strip()
3939
path = get_aliases_path()
4040
path.parent.mkdir(parents=True, exist_ok=True)
4141
builtin = _builtin_command_names()
@@ -45,7 +45,7 @@ def create_alias(alias: str, module_id: str, force: bool = False) -> None:
4545
data = {}
4646
if path.exists():
4747
data = json.loads(path.read_text(encoding="utf-8"))
48-
data[alias] = module_id
48+
data[alias] = command_name
4949
path.write_text(json.dumps(data, indent=2), encoding="utf-8")
5050
if alias in builtin:
5151
logger.warning("Alias will shadow built-in module: %s", alias)
@@ -54,7 +54,7 @@ def create_alias(alias: str, module_id: str, force: bool = False) -> None:
5454
@beartype
5555
@ensure(lambda result: isinstance(result, dict), "returns dict")
5656
def list_aliases() -> dict[str, str]:
57-
"""Return all alias -> module_id mappings. Empty dict if file missing."""
57+
"""Return all alias -> command name mappings. Empty dict if file missing."""
5858
path = get_aliases_path()
5959
if not path.exists():
6060
return {}
@@ -87,9 +87,8 @@ def remove_alias(alias: str) -> None:
8787
@require(lambda invoked_name: isinstance(invoked_name, str), "invoked_name must be str")
8888
@ensure(lambda result: isinstance(result, str) and len(result) > 0, "returns non-empty string")
8989
def resolve_command(invoked_name: str) -> str:
90-
"""If invoked_name is an alias, return the module command name (last segment of module_id); else return invoked_name."""
90+
"""If invoked_name is an alias, return the stored command name; else return invoked_name."""
9191
aliases = list_aliases()
9292
if invoked_name in aliases:
93-
module_id = aliases[invoked_name]
94-
return module_id.rsplit("/", 1)[-1] if "/" in module_id else module_id
93+
return aliases[invoked_name]
9594
return invoked_name

src/specfact_cli/registry/marketplace_client.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,28 @@ def download_module(
7373
) -> Path:
7474
"""Download module tarball and verify SHA-256 checksum from registry metadata."""
7575
logger = get_bridge_logger(__name__)
76-
registry_index = index if index is not None else fetch_registry_index()
76+
if index is not None:
77+
registry_index = index
78+
else:
79+
from specfact_cli.registry.custom_registries import fetch_all_indexes
80+
81+
registry_index = None
82+
for _reg_id, idx in fetch_all_indexes(timeout=timeout):
83+
if not isinstance(idx, dict):
84+
continue
85+
mods = idx.get("modules") or []
86+
if not isinstance(mods, list):
87+
continue
88+
for c in mods:
89+
if isinstance(c, dict) and c.get("id") == module_id:
90+
if version and c.get("latest_version") != version:
91+
continue
92+
registry_index = idx
93+
break
94+
if registry_index is not None:
95+
break
96+
if registry_index is None:
97+
registry_index = fetch_registry_index()
7798
if not registry_index:
7899
raise ValueError("Cannot install from marketplace (offline)")
79100

tests/unit/registry/test_alias_manager.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,22 @@ def test_get_aliases_path_returns_specfact_registry_path() -> None:
2626

2727

2828
def test_create_alias_stores_mapping(tmp_path: Path) -> None:
29-
"""create_alias() writes alias -> module_id to aliases.json."""
29+
"""create_alias() writes alias -> command name to aliases.json."""
3030
with patch("specfact_cli.registry.alias_manager.get_aliases_path", return_value=tmp_path / "aliases.json"):
31-
create_alias("my-backlog", "acme-corp/backlog-pro")
31+
create_alias("my-backlog", "backlog-pro")
3232
assert (tmp_path / "aliases.json").exists()
3333
data = json.loads((tmp_path / "aliases.json").read_text())
34-
assert data == {"my-backlog": "acme-corp/backlog-pro"}
34+
assert data == {"my-backlog": "backlog-pro"}
3535

3636

3737
def test_list_aliases_returns_all_aliases(tmp_path: Path) -> None:
38-
"""list_aliases() returns dict of alias -> module_id."""
38+
"""list_aliases() returns dict of alias -> command name."""
3939
aliases_file = tmp_path / "aliases.json"
4040
tmp_path.mkdir(parents=True, exist_ok=True)
41-
aliases_file.write_text(json.dumps({"backlog": "acme/backlog-pro", "generate": "specfact/generate"}))
41+
aliases_file.write_text(json.dumps({"backlog": "backlog-pro", "generate": "generate"}))
4242
with patch("specfact_cli.registry.alias_manager.get_aliases_path", return_value=aliases_file):
4343
result = list_aliases()
44-
assert result == {"backlog": "acme/backlog-pro", "generate": "specfact/generate"}
44+
assert result == {"backlog": "backlog-pro", "generate": "generate"}
4545

4646

4747
def test_list_aliases_returns_empty_when_file_missing() -> None:
@@ -64,10 +64,10 @@ def test_remove_alias_deletes_mapping(tmp_path: Path) -> None:
6464

6565

6666
def test_resolve_command_returns_module_command_name_when_aliased() -> None:
67-
"""resolve_command() returns the command name for the aliased module (last segment of module_id)."""
68-
with patch("specfact_cli.registry.alias_manager.list_aliases", return_value={"backlog": "acme-corp/backlog-pro"}):
67+
"""resolve_command() returns the stored command name for the alias."""
68+
with patch("specfact_cli.registry.alias_manager.list_aliases", return_value={"backlog": "backlog-pro"}):
6969
assert resolve_command("backlog") == "backlog-pro"
70-
with patch("specfact_cli.registry.alias_manager.list_aliases", return_value={"gen": "specfact/generate"}):
70+
with patch("specfact_cli.registry.alias_manager.list_aliases", return_value={"gen": "generate"}):
7171
assert resolve_command("gen") == "generate"
7272

7373

@@ -86,7 +86,7 @@ def test_create_alias_raises_when_shadowing_builtin_without_force(tmp_path: Path
8686
patch("specfact_cli.registry.alias_manager._builtin_command_names", return_value={"backlog", "module"}),
8787
pytest.raises(ValueError, match="shadow"),
8888
):
89-
create_alias("backlog", "acme/backlog-pro", force=False)
89+
create_alias("backlog", "backlog-pro", force=False)
9090

9191

9292
def test_create_alias_with_force_stores_even_when_shadowing(tmp_path: Path) -> None:
@@ -95,6 +95,6 @@ def test_create_alias_with_force_stores_even_when_shadowing(tmp_path: Path) -> N
9595
patch("specfact_cli.registry.alias_manager.get_aliases_path", return_value=tmp_path / "aliases.json"),
9696
patch("specfact_cli.registry.alias_manager._builtin_command_names", return_value={"backlog"}),
9797
):
98-
create_alias("backlog", "acme/backlog-pro", force=True)
98+
create_alias("backlog", "backlog-pro", force=True)
9999
data = json.loads((tmp_path / "aliases.json").read_text())
100-
assert data.get("backlog") == "acme/backlog-pro"
100+
assert data.get("backlog") == "backlog-pro"

0 commit comments

Comments
 (0)