chore: move model-discovery scripts to .github/scripts/

Move GHA-only scripts to .github/scripts/ where they belong. Update
workflow reference and DEFAULT_MANIFEST path. Address PR review feedback:
- Document the pagination safety limit (20 pages × 100 = 2000 max)
- Use @patch for SEED_MODELS in tests instead of direct mutation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: maskarb

scripts/model-discovery.py .github/scripts/model-discovery.pyscripts/model-discovery.py renamed to .github/scripts/model-discovery.py

@@ -38,7 +38,7 @@
 # ---------------------------------------------------------------------------
 
 DEFAULT_MANIFEST = (
-    Path(__file__).resolve().parent.parent
+    Path(__file__).resolve().parent.parent.parent
     / "components"
     / "manifests"
     / "base"
@@ -49,6 +49,10 @@
 # e.g. claude-opus-4-6 and claude-opus-4-5 are kept, claude-opus-4-1 is dropped.
 MAX_VERSIONS_PER_FAMILY = 2
 
+# Model Garden list API pagination settings.
+LIST_PAGE_SIZE = 100
+MAX_LIST_PAGES = 20
+
 
 # Publisher discovery configuration.
 # prefixes:  only models whose ID starts with one of these are included.
@@ -119,10 +123,10 @@ def get_access_token() -> str:
             check=True,
             timeout=30,
         )
-    except subprocess.TimeoutExpired:
-        raise RuntimeError("Timed out getting GCP access token via gcloud")
-    except subprocess.CalledProcessError:
-        raise RuntimeError("Failed to get GCP access token via gcloud")
+    except subprocess.TimeoutExpired as err:
+        raise RuntimeError("Timed out getting GCP access token via gcloud") from err
+    except subprocess.CalledProcessError as err:
+        raise RuntimeError("Failed to get GCP access token via gcloud") from err
     return result.stdout.strip()
 
 
@@ -138,8 +142,8 @@ def list_publisher_models(publisher: str, token: str) -> list[tuple[str, str | N
     all_models: list[tuple[str, str | None]] = []
     page_token = ""
 
-    for _ in range(20):  # page limit safety
-        params = {"pageSize": "100"}
+    for _ in range(MAX_LIST_PAGES):
+        params = {"pageSize": str(LIST_PAGE_SIZE)}
         if page_token:
             params["pageToken"] = page_token
 
@@ -161,10 +165,16 @@ def list_publisher_models(publisher: str, token: str) -> list[tuple[str, str | N
                     data = json.loads(resp.read().decode())
                 break
             except urllib.error.HTTPError as e:
-                # Permission denied or not found — retrying won't help
-                if e.code in (403, 404):
+                # Auth failures are fatal — don't fall back to seeds with bad credentials
+                if e.code in (401, 403):
+                    raise RuntimeError(
+                        f"list models for {publisher} failed (HTTP {e.code}): "
+                        f"check GCP credentials and IAM permissions"
+                    ) from e
+                # Not found — retrying won't help
+                if e.code == 404:
                     print(
-                        f"  WARNING: list models for {publisher} failed (HTTP {e.code})",
+                        f"  WARNING: list models for {publisher} returned 404",
                         file=sys.stderr,
                     )
                     return []
@@ -503,22 +513,24 @@ def probe_model(
 
 
 def load_manifest(path: Path) -> dict:
-    """Load the model manifest JSON, or return a blank manifest if missing/empty."""
-    blank = {"version": 1, "defaultModel": "claude-sonnet-4-5", "models": []}
+    """Load the model manifest JSON, or return a blank manifest if missing.
+
+    Raises on malformed JSON to prevent overwriting a corrupt file.
+    Returns a blank manifest only when the file does not exist yet.
+    """
     if not path.exists():
-        return blank
-    try:
-        with open(path) as f:
-            data = json.load(f)
-        if not isinstance(data, dict) or "models" not in data:
-            return blank
-        return data
-    except (json.JSONDecodeError, ValueError) as e:
-        print(
-            f"WARNING: malformed manifest at {path}, starting fresh ({e})",
-            file=sys.stderr,
+        return {"version": 1, "defaultModel": "claude-sonnet-4-5", "models": []}
+
+    with open(path) as f:
+        data = json.load(f)
+
+    if not isinstance(data, dict) or "models" not in data:
+        raise ValueError(
+            f"manifest at {path} is missing required 'models' key — "
+            f"fix the file manually or delete it to start fresh"
         )
-        return blank
+
+    return data
 
 
 def save_manifest(path: Path, manifest: dict) -> None:

.github/workflows/model-discovery.yml

@@ -40,7 +40,7 @@ jobs:
         env:
           GCP_REGION: ${{ secrets.GCP_REGION }}
           GCP_PROJECT: ${{ secrets.GCP_PROJECT }}
-        run: python scripts/model-discovery.py
+        run: python .github/scripts/model-discovery.py
 
       - name: Check for changes
         id: diff

.github/workflows/unit-tests.yml

@@ -10,6 +10,8 @@ on:
       - 'components/ambient-cli/**'
       - 'components/ambient-sdk/go-sdk/**'
       - 'components/frontend/**'
+      - '.github/scripts/**'
+      - 'tests/**'
       - '.github/workflows/unit-tests.yml'
       - '!**/*.md'
 
@@ -21,6 +23,8 @@ on:
       - 'components/ambient-cli/**'
       - 'components/ambient-sdk/go-sdk/**'
       - 'components/frontend/**'
+      - '.github/scripts/**'
+      - 'tests/**'
       - '.github/workflows/unit-tests.yml'
       - '!**/*.md'
 
@@ -50,6 +54,7 @@ jobs:
       runner: ${{ steps.filter.outputs.runner }}
       cli: ${{ steps.filter.outputs.cli }}
       frontend: ${{ steps.filter.outputs.frontend }}
+      scripts: ${{ steps.filter.outputs.scripts }}
     steps:
       - name: Checkout code
         uses: actions/checkout@v6
@@ -70,6 +75,9 @@ jobs:
               - 'components/ambient-sdk/go-sdk/**'
             frontend:
               - 'components/frontend/**'
+            scripts:
+              - '.github/scripts/**'
+              - 'tests/test_model_discovery.py'
 
   backend:
     runs-on: ubuntu-latest
@@ -300,9 +308,26 @@ jobs:
       - name: Run unit tests with coverage
         run: npx vitest run --coverage
 
+  scripts:
+    runs-on: ubuntu-latest
+    needs: detect-changes
+    if: needs.detect-changes.outputs.scripts == 'true' || github.event_name == 'workflow_dispatch'
+    name: Script Tests (model-discovery)
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v6
+
+      - name: Set up Python
+        uses: actions/setup-python@v6
+        with:
+          python-version: '3.11'
+
+      - name: Run tests
+        run: python tests/test_model_discovery.py -v
+
   summary:
     runs-on: ubuntu-latest
-    needs: [detect-changes, backend, api-server, runner, cli, frontend]
+    needs: [detect-changes, backend, api-server, runner, cli, frontend, scripts]
     if: always()
     steps:
       - name: Check overall status
@@ -313,7 +338,8 @@ jobs:
             "${{ needs.api-server.result }}" \
             "${{ needs.runner.result }}" \
             "${{ needs.cli.result }}" \
-            "${{ needs.frontend.result }}"; do
+            "${{ needs.frontend.result }}" \
+            "${{ needs.scripts.result }}"; do
             if [ "$result" == "failure" ] || [ "$result" == "cancelled" ]; then
               failed=true
             fi
@@ -325,6 +351,7 @@ jobs:
             echo "  runner: ${{ needs.runner.result }}"
             echo "  cli: ${{ needs.cli.result }}"
             echo "  frontend: ${{ needs.frontend.result }}"
+            echo "  scripts: ${{ needs.scripts.result }}"
             exit 1
           fi
           echo "All unit tests passed!"

scripts/test_model_discovery.py tests/test_model_discovery.pyscripts/test_model_discovery.py renamed to tests/test_model_discovery.py

@@ -8,7 +8,8 @@
 
 # Import model-discovery.py as a module (it has a hyphen in the name)
 _spec = importlib.util.spec_from_file_location(
-    "model_discovery", Path(__file__).parent / "model-discovery.py"
+    "model_discovery",
+    Path(__file__).resolve().parent.parent / ".github" / "scripts" / "model-discovery.py",
 )
 _mod = importlib.util.module_from_spec(_spec)
 sys.modules["model_discovery"] = _mod
@@ -149,27 +150,100 @@ def test_empty_input(self):
 
 
 class TestDiscoverModels(unittest.TestCase):
-    """Test discover_models seed fallback with version_cutoff."""
+    """Test discover_models with API discovery, seed fallback, and filtering."""
+
+    _default_manifest = {
+        "defaultModel": "claude-sonnet-4-5",
+        "providerDefaults": {"google": "gemini-2.5-flash"},
+    }
 
     @patch("model_discovery.list_publisher_models", return_value=[])
+    @patch(
+        "model_discovery.SEED_MODELS",
+        _mod.SEED_MODELS + [("gemini-2.0-flash", "google", "google")],
+    )
     def test_seed_models_respect_version_cutoff(self, _mock_list):
         """Seed models older than version_cutoff should be excluded."""
-        # Add a gemini-2.0 model to SEED_MODELS temporarily
-        original_seeds = _mod.SEED_MODELS[:]
-        try:
-            _mod.SEED_MODELS.append(("gemini-2.0-flash", "google", "google"))
-            manifest = {
-                "defaultModel": "claude-sonnet-4-5",
-                "providerDefaults": {"google": "gemini-2.5-flash"},
-            }
-            result = discover_models("fake-token", manifest)
-            ids = [r[0] for r in result]
-            # gemini-2.0-flash should be excluded by version_cutoff (2, 0)
-            self.assertNotIn("gemini-2.0-flash", ids)
-            # gemini-2.5-flash from seeds should still be present
-            self.assertIn("gemini-2.5-flash", ids)
-        finally:
-            _mod.SEED_MODELS[:] = original_seeds
+        result = discover_models("fake-token", self._default_manifest)
+        ids = [r[0] for r in result]
+        self.assertNotIn("gemini-2.0-flash", ids)
+        self.assertIn("gemini-2.5-flash", ids)
+
+    @patch("model_discovery.list_publisher_models")
+    def test_api_discovered_models_included(self, mock_list):
+        """Models returned by the list API should appear in results."""
+        def fake_list(publisher, token):
+            if publisher == "anthropic":
+                return [("claude-sonnet-4-5", "20250929"), ("claude-opus-4-6", None)]
+            if publisher == "google":
+                return [("gemini-2.5-flash", None), ("gemini-2.5-pro", None)]
+            return []
+
+        mock_list.side_effect = fake_list
+        result = discover_models("fake-token", self._default_manifest)
+        ids = [r[0] for r in result]
+        self.assertIn("claude-sonnet-4-5", ids)
+        self.assertIn("claude-opus-4-6", ids)
+        self.assertIn("gemini-2.5-flash", ids)
+        self.assertIn("gemini-2.5-pro", ids)
+
+    @patch("model_discovery.list_publisher_models")
+    def test_protected_models_exempt_from_pruning(self, mock_list):
+        """Default model and provider defaults are never pruned by version limiting."""
+        def fake_list(publisher, token):
+            if publisher == "anthropic":
+                return [
+                    ("claude-sonnet-4-5", None),  # defaultModel
+                    ("claude-sonnet-4-6", None),
+                    ("claude-opus-4-6", None),
+                    ("claude-opus-4-5", None),
+                    ("claude-haiku-4-5", None),
+                ]
+            if publisher == "google":
+                return [("gemini-2.5-flash", None)]  # providerDefault
+            return []
+
+        mock_list.side_effect = fake_list
+        result = discover_models("fake-token", self._default_manifest)
+        ids = [r[0] for r in result]
+        # Protected models must always be present
+        self.assertIn("claude-sonnet-4-5", ids)  # defaultModel
+        self.assertIn("gemini-2.5-flash", ids)  # providerDefault for google
+
+    @patch("model_discovery.list_publisher_models")
+    def test_prefix_and_exclude_filters(self, mock_list):
+        """Prefix filtering keeps matching models; exclude patterns remove unwanted ones."""
+        def fake_list(publisher, token):
+            if publisher == "anthropic":
+                return [
+                    ("claude-sonnet-4-5", None),  # matches prefix, no exclude
+                    ("claude-opus-4", None),       # matches exclude: base alias without minor
+                    ("not-claude-model", None),    # doesn't match prefix
+                ]
+            if publisher == "google":
+                return [
+                    ("gemini-2.5-flash", None),
+                    ("gemini-2.5-flash-001", None),  # matches exclude: pinned version
+                    ("gemini-2.0-flash-preview-image-generation", None),  # excluded by version_cutoff
+                ]
+            return []
+
+        mock_list.side_effect = fake_list
+        result = discover_models("fake-token", self._default_manifest)
+        ids = [r[0] for r in result]
+        self.assertIn("claude-sonnet-4-5", ids)
+        self.assertNotIn("claude-opus-4", ids)      # excluded: base alias
+        self.assertNotIn("not-claude-model", ids)   # excluded: wrong prefix
+        self.assertIn("gemini-2.5-flash", ids)
+        self.assertNotIn("gemini-2.5-flash-001", ids)  # excluded: pinned version
+        self.assertNotIn("gemini-2.0-flash-preview-image-generation", ids)  # excluded: version_cutoff
+
+    @patch("model_discovery.list_publisher_models")
+    def test_auth_error_propagates(self, mock_list):
+        """Auth errors from list_publisher_models should propagate, not fall back to seeds."""
+        mock_list.side_effect = RuntimeError("HTTP 401: check GCP credentials")
+        with self.assertRaises(RuntimeError):
+            discover_models("fake-token", self._default_manifest)
 
 
 if __name__ == "__main__":