Skip to content

Commit de576dc

Browse files
mnriemCopilot
andcommitted
Fix catalog validation precedence and selected workflow paths
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent e84c0a0 commit de576dc

14 files changed

Lines changed: 513 additions & 82 deletions

File tree

‎src/specify_cli/bundles/references.py‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,17 +123,15 @@ def _resolved_in_catalog(root: Path, component: ComponentRef) -> bool | str | No
123123
except Exception as exc: # noqa: BLE001 - report malformed catalog errors
124124
from ..extensions import ExtensionCatalogFetchError
125125
from ..presets._catalog import PresetCatalogFetchError
126-
from ..workflows.catalog import StepCatalogError, WorkflowCatalogError
126+
from ..workflows.catalog import (
127+
StepCatalogFetchError,
128+
WorkflowCatalogFetchError,
129+
)
127130

128-
if isinstance(exc, (ExtensionCatalogFetchError, PresetCatalogFetchError)):
129-
return None
130-
if isinstance(exc, WorkflowCatalogError) and str(exc) == (
131-
"All configured catalogs failed to fetch."
132-
):
133-
return None
134-
if isinstance(exc, StepCatalogError) and str(exc) == (
135-
"All configured step catalogs failed to fetch."
136-
):
131+
if isinstance(exc, (
132+
ExtensionCatalogFetchError, PresetCatalogFetchError,
133+
WorkflowCatalogFetchError, StepCatalogFetchError,
134+
)):
137135
return None
138136
return f"Catalog lookup failed: {exc}"
139137
return None

‎src/specify_cli/extensions/__init__.py‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4808,7 +4808,7 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
48084808
) from e
48094809

48104810
def _get_merged_extensions(
4811-
self, force_refresh: bool = False
4811+
self, force_refresh: bool = False, *, extension_id: str | None = None
48124812
) -> List[Dict[str, Any]]:
48134813
"""Fetch and merge extensions from all active catalogs.
48144814
@@ -4817,8 +4817,9 @@ def _get_merged_extensions(
48174817
- _catalog_name: name of the source catalog
48184818
- _install_allowed: whether installation is allowed from this catalog
48194819
4820-
Catalogs that fail to fetch are skipped. Raises ExtensionError only if
4821-
ALL catalogs fail.
4820+
An ID lookup stops at its first matching source and refuses malformed
4821+
higher-priority catalogs. Untargeted searches continue past malformed
4822+
sources so other catalog results remain discoverable.
48224823
48234824
Args:
48244825
force_refresh: If True, bypass all caches
@@ -4827,7 +4828,8 @@ def _get_merged_extensions(
48274828
List of merged extension dicts
48284829
48294830
Raises:
4830-
ExtensionError: If all catalogs fail to fetch
4831+
ExtensionError: If no catalog is readable, or an ID lookup
4832+
encounters malformed catalog data.
48314833
"""
48324834
import sys
48334835

@@ -4846,13 +4848,19 @@ def _get_merged_extensions(
48464848
and validation_error is None
48474849
):
48484850
validation_error = e
4851+
if extension_id is not None and isinstance(
4852+
e, ExtensionCatalogValidationError
4853+
):
4854+
raise
48494855
print(
48504856
f"Warning: Could not fetch catalog '{catalog_entry.name}': {e}",
48514857
file=sys.stderr,
48524858
)
48534859
continue
48544860

48554861
for ext_id, ext_data in catalog_data.get("extensions", {}).items():
4862+
if extension_id is not None and ext_id != extension_id:
4863+
continue
48564864
# Per-entry guard: ``_fetch_single_catalog`` already validates
48574865
# that ``catalog_data["extensions"]`` is a mapping, but it
48584866
# does not (and should not) validate every entry shape there
@@ -4862,6 +4870,11 @@ def _get_merged_extensions(
48624870
# the valid entries without crashing on ``**ext_data``.
48634871
# Mirrors ``integrations/catalog.py:245``.
48644872
if not isinstance(ext_data, dict):
4873+
if extension_id is not None:
4874+
raise ExtensionCatalogValidationError(
4875+
f"Invalid extension catalog entry for '{ext_id}' "
4876+
f"from {catalog_entry.url}: expected a JSON object"
4877+
)
48654878
continue
48664879
if ext_id not in merged: # Higher-priority catalog wins
48674880
merged[ext_id] = {
@@ -4870,6 +4883,8 @@ def _get_merged_extensions(
48704883
"_catalog_name": catalog_entry.name,
48714884
"_install_allowed": catalog_entry.install_allowed,
48724885
}
4886+
if extension_id is not None:
4887+
return list(merged.values())
48734888

48744889
if not any_success and active_catalogs:
48754890
if validation_error is not None:
@@ -5093,7 +5108,7 @@ def get_extension_info(
50935108
Extension metadata (annotated with ``_catalog_name`` and
50945109
``_install_allowed``) or None if not found.
50955110
"""
5096-
all_extensions = self._get_merged_extensions()
5111+
all_extensions = self._get_merged_extensions(extension_id=extension_id)
50975112
for ext_data in all_extensions:
50985113
if ext_data["id"] == extension_id:
50995114
from ._catalog_versions import select_release
@@ -5105,7 +5120,7 @@ def get_extension_versions(self, extension_id: str) -> list[str]:
51055120
"""List versions advertised by the winning catalog source."""
51065121
from ._catalog_versions import available_versions
51075122

5108-
for ext_data in self._get_merged_extensions():
5123+
for ext_data in self._get_merged_extensions(extension_id=extension_id):
51095124
if ext_data["id"] == extension_id:
51105125
return available_versions(ext_data)
51115126
return []

‎src/specify_cli/workflows/catalog/__init__.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,17 @@ def register(app: typer.Typer) -> None:
2929
"WorkflowCatalog",
3030
"WorkflowCatalogEntry",
3131
"WorkflowCatalogError",
32+
"WorkflowCatalogFetchError",
33+
"WorkflowCatalogValidationError",
3234
"WorkflowRegistry",
3335
"WorkflowValidationError",
3436
}
3537
_STEP_COMPATIBILITY_EXPORTS = {
3638
"StepCatalog",
3739
"StepCatalogEntry",
3840
"StepCatalogError",
41+
"StepCatalogFetchError",
42+
"StepCatalogValidationError",
3943
"StepRegistry",
4044
"StepValidationError",
4145
}

‎src/specify_cli/workflows/catalog/_domain.py‎

Lines changed: 85 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,14 @@ class WorkflowCatalogError(Exception):
4343
"""Base error for workflow catalog operations."""
4444

4545

46+
class WorkflowCatalogFetchError(WorkflowCatalogError):
47+
"""A configured workflow catalog could not be fetched."""
48+
49+
50+
class WorkflowCatalogValidationError(WorkflowCatalogError):
51+
"""A workflow catalog supplied malformed metadata."""
52+
53+
4654
class WorkflowValidationError(WorkflowCatalogError):
4755
"""Validation error for catalog config or workflow data."""
4856

@@ -517,18 +525,36 @@ def _fetch_single_catalog(
517525
"""Fetch a single catalog, using cache when possible."""
518526
cache_file, meta_file = self._get_cache_paths(entry.url)
519527

528+
def validate_payload(data: Any) -> dict[str, Any]:
529+
if not isinstance(data, dict):
530+
raise WorkflowCatalogValidationError(
531+
f"Catalog from {entry.url} is not a valid JSON object."
532+
)
533+
if "workflows" in data and not isinstance(data["workflows"], (dict, list)):
534+
raise WorkflowCatalogValidationError(
535+
f"Catalog from {entry.url} has malformed workflows metadata."
536+
)
537+
return data
538+
520539
if not force_refresh and self._is_url_cache_valid(entry.url):
521540
try:
522541
with open(cache_file, encoding="utf-8") as f:
523542
cached = json.load(f)
524-
if isinstance(cached, dict):
525-
return cached
526-
except (UnicodeDecodeError, json.JSONDecodeError, OSError):
543+
return validate_payload(cached)
544+
except (
545+
UnicodeDecodeError,
546+
json.JSONDecodeError,
547+
OSError,
548+
WorkflowCatalogValidationError,
549+
):
527550
# Ignore invalid/unreadable cache and fall back to fetching from source.
528551
pass
529552

530553
# Fetch from URL — validate scheme before opening and after redirects
554+
from http.client import HTTPException
531555
from urllib.parse import urlparse
556+
557+
from specify_cli.authentication.http import RedirectPolicyError
532558
from specify_cli.authentication.http import open_url as _open_url
533559

534560
def _validate_catalog_url(url: str) -> None:
@@ -542,18 +568,18 @@ def _validate_catalog_url(url: str) -> None:
542568
hostname = parsed.hostname
543569
_ = parsed.port
544570
except (TypeError, ValueError):
545-
raise WorkflowCatalogError(
571+
raise WorkflowCatalogValidationError(
546572
f"Refusing to fetch catalog from malformed URL: {url}"
547573
) from None
548574
is_localhost = hostname in ("localhost", "127.0.0.1", "::1")
549575
if parsed.scheme != "https" and not (
550576
parsed.scheme == "http" and is_localhost
551577
):
552-
raise WorkflowCatalogError(
578+
raise WorkflowCatalogValidationError(
553579
f"Refusing to fetch catalog from non-HTTPS URL: {url}"
554580
)
555581
if not hostname:
556-
raise WorkflowCatalogError(
582+
raise WorkflowCatalogValidationError(
557583
f"Refusing to fetch catalog from URL with no hostname: {url}"
558584
)
559585

@@ -578,29 +604,39 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
578604
read_response_limited(
579605
resp,
580606
max_bytes=_max_json_catalog_bytes(),
581-
error_type=WorkflowCatalogError,
607+
error_type=WorkflowCatalogValidationError,
582608
label="workflow catalog",
583609
).decode("utf-8")
584610
)
585-
except Exception as exc:
611+
except (
612+
WorkflowCatalogValidationError,
613+
RedirectPolicyError,
614+
UnicodeError,
615+
json.JSONDecodeError,
616+
) as exc:
617+
raise WorkflowCatalogValidationError(
618+
f"Invalid workflow catalog from {entry.url}: {exc}"
619+
) from exc
620+
except (OSError, HTTPException) as exc:
586621
# Fall back to cache if available
587622
if cache_file.exists():
588623
try:
589624
with open(cache_file, encoding="utf-8") as f:
590625
cached = json.load(f)
591-
if isinstance(cached, dict):
592-
return cached
593-
except (json.JSONDecodeError, ValueError, OSError):
626+
return validate_payload(cached)
627+
except (
628+
json.JSONDecodeError,
629+
ValueError,
630+
OSError,
631+
WorkflowCatalogValidationError,
632+
):
594633
# Stale-cache read failed; let the original fetch error propagate.
595634
pass
596-
raise WorkflowCatalogError(
635+
raise WorkflowCatalogFetchError(
597636
f"Failed to fetch catalog from {entry.url}: {exc}"
598637
) from exc
599638

600-
if not isinstance(data, dict):
601-
raise WorkflowCatalogError(
602-
f"Catalog from {entry.url} is not a valid JSON object."
603-
)
639+
data = validate_payload(data)
604640

605641
# Write cache
606642
try:
@@ -615,26 +651,44 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
615651
return data
616652

617653
def _get_merged_workflows(
618-
self, force_refresh: bool = False
654+
self, force_refresh: bool = False, *, workflow_id: str | None = None
619655
) -> dict[str, dict[str, Any]]:
620-
"""Merge workflows from all active catalogs (lower priority number wins)."""
656+
"""Merge for search, or resolve one ID from the first valid winning source."""
621657
catalogs = self.get_active_catalogs()
622658
merged: dict[str, dict[str, Any]] = {}
623659
fetch_errors = 0
660+
validation_error: WorkflowCatalogValidationError | None = None
624661

625-
# Process later/higher-numbered entries first so earlier/lower-numbered
626-
# entries overwrite them on workflow ID conflicts.
627-
for entry in reversed(catalogs):
662+
# Search uses overwrite order; exact-ID lookup visits the highest
663+
# priority source first and stops at its matching entry.
664+
sources = catalogs if workflow_id is not None else reversed(catalogs)
665+
for entry in sources:
628666
try:
629667
data = self._fetch_single_catalog(entry, force_refresh)
630-
except WorkflowCatalogError:
668+
except WorkflowCatalogError as exc:
669+
if workflow_id is not None and isinstance(
670+
exc, WorkflowCatalogValidationError
671+
):
672+
raise
673+
if (
674+
isinstance(exc, WorkflowCatalogValidationError)
675+
and validation_error is None
676+
):
677+
validation_error = exc
631678
fetch_errors += 1
632679
continue
633680
workflows = data.get("workflows", {})
634681
# Handle both dict and list formats
635682
if isinstance(workflows, dict):
636683
for wf_id, wf_data in workflows.items():
684+
if workflow_id is not None and wf_id != workflow_id:
685+
continue
637686
if not isinstance(wf_data, dict):
687+
if workflow_id is not None:
688+
raise WorkflowCatalogValidationError(
689+
f"Invalid workflow catalog entry for '{wf_id}' "
690+
f"from {entry.url}: expected a JSON object"
691+
)
638692
continue
639693
wf_data["_catalog_name"] = entry.name
640694
wf_data["_install_allowed"] = entry.install_allowed
@@ -645,11 +699,17 @@ def _get_merged_workflows(
645699
continue
646700
wf_id = wf_data.get("id", "")
647701
if wf_id:
702+
if workflow_id is not None and wf_id != workflow_id:
703+
continue
648704
wf_data["_catalog_name"] = entry.name
649705
wf_data["_install_allowed"] = entry.install_allowed
650706
merged[wf_id] = wf_data
707+
if workflow_id is not None and workflow_id in merged:
708+
return merged
651709
if fetch_errors == len(catalogs) and catalogs:
652-
raise WorkflowCatalogError(
710+
if validation_error is not None:
711+
raise validation_error
712+
raise WorkflowCatalogFetchError(
653713
"All configured catalogs failed to fetch."
654714
)
655715
return merged
@@ -698,7 +758,7 @@ def get_workflow_info(
698758
"""Get the current or an exact advertised release from the winning source."""
699759
from ._versions import select_release
700760

701-
merged = self._get_merged_workflows()
761+
merged = self._get_merged_workflows(workflow_id=workflow_id)
702762
wf = merged.get(workflow_id)
703763
if wf is None:
704764
return None
@@ -716,7 +776,7 @@ def get_workflow_version_details(
716776
"""Return advertised versions and whether their source allows installation."""
717777
from ._versions import available_versions
718778

719-
merged = self._get_merged_workflows()
779+
merged = self._get_merged_workflows(workflow_id=workflow_id)
720780
wf = merged.get(workflow_id)
721781
if wf is None:
722782
return None

‎src/specify_cli/workflows/command_add.py‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,23 @@ def _workflow_package_has_companions(package_dir: cli.Path) -> bool:
3232

3333
def _prepare_workflow_add(
3434
project_root: cli.Path, source: str, *, dev: bool, from_url: str | None,
35-
version: str | None,
35+
version: str | None, selected_catalog: bool = False,
3636
) -> cli.Path:
3737
from . import load_custom_steps
3838

3939
if version is not None and (
40-
dev or from_url is not None or source.startswith(("http://", "https://"))
41-
or cli.Path(source).exists()
40+
dev or from_url is not None or (
41+
not selected_catalog and (
42+
source.startswith(("http://", "https://"))
43+
or cli.Path(source).exists()
44+
)
45+
)
4246
):
4347
cli.console.print(
4448
"[red]Error:[/red] --version requires a workflow ID from a catalog."
4549
)
4650
raise cli.typer.Exit(1)
47-
if version is not None:
51+
if version is not None or selected_catalog:
4852
cli._validate_workflow_id_or_exit(source)
4953
load_custom_steps(project_root)
5054
cli._open_workflow_registry(project_root)
@@ -62,10 +66,9 @@ def _install_preselected_workflow(
6266
"""Install a bundle-selected release with the normal workflow preflights."""
6367
project_root = cli._require_specify_project()
6468
workflows_dir = _prepare_workflow_add(
65-
project_root, workflow_id, dev=False, from_url=None, version=version,
69+
project_root, workflow_id, dev=False, from_url=None,
70+
version=version, selected_catalog=True,
6671
)
67-
if version is None:
68-
cli._validate_workflow_id_or_exit(workflow_id)
6972
cli._install_workflow_from_catalog(
7073
project_root, workflows_dir, workflow_id,
7174
requested_version=version, selected_info=selected_info,

‎src/specify_cli/workflows/step/catalog/__init__.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ def register(app: typer.Typer) -> None:
2929
"StepCatalog",
3030
"StepCatalogEntry",
3131
"StepCatalogError",
32+
"StepCatalogFetchError",
33+
"StepCatalogValidationError",
3234
"StepRegistry",
3335
"StepValidationError",
3436
}

0 commit comments

Comments
 (0)