fix: address fourth self-upgrade review round

- self_upgrade: label a pinned target older than the installed version as
  "Downgrading" rather than "Upgrading" so `--tag <older>` is not mistaken
  for a forward upgrade.
- resolver: drop the unused `typing.Optional` import and annotate the
  `--tag` option as `str | None`, consistent with the rest of the module
  (verified Typer resolves it on the supported Python versions).
- _is_github_credential_env_key: add `_PASSWORD` and `_CREDENTIALS` to the
  recognized credential suffixes and document that only these shapes are
  scrubbed (not blanket coverage).
- tests: assert the precise exit code (1) for the re-raised transient
  OSError path; skip the InvalidMetadataError test on Pythons where the
  real exception is absent instead of fabricating it; update the pinned
  downgrade test to expect the "Downgrading" label.

Author: chordpli

src/specify_cli/_version.py

@@ -24,7 +24,6 @@
 from dataclasses import dataclass
 from enum import Enum
 from pathlib import Path
-from typing import Optional
 
 import typer
 from packaging.version import InvalidVersion, Version
@@ -244,7 +243,14 @@ class _DetectionSignals:
     resolved_method: _InstallMethod
 
 
-_GITHUB_CREDENTIAL_SUFFIXES = ("_TOKEN", "_SECRET", "_KEY", "_PAT")
+_GITHUB_CREDENTIAL_SUFFIXES = (
+    "_TOKEN",
+    "_SECRET",
+    "_KEY",
+    "_PAT",
+    "_PASSWORD",
+    "_CREDENTIALS",
+)
 _UNRESOLVED_ENV_VAR_RE = re.compile(r"\$\w+|\$\{\w+\}|%[^%]+%")
 
 
@@ -260,9 +266,11 @@ def _is_github_credential_env_key(key: str) -> bool:
       ``GITHUB_REPOSITORY``) the installer subprocess does not consume.
     - Otherwise the key is scrubbed only when it contains an underscore-delimited
       ``_GITHUB_`` segment *and* ends with a credential suffix
-      (``_TOKEN``/``_SECRET``/``_KEY``/``_PAT``) — e.g. ``HOMEBREW_GITHUB_API_TOKEN``.
-      Un-delimited variants such as a hypothetical ``GITHUBTOKEN`` are not matched
-      by this branch; no real tool sets such a name.
+      (``_TOKEN``/``_SECRET``/``_KEY``/``_PAT``/``_PASSWORD``/``_CREDENTIALS``) —
+      e.g. ``HOMEBREW_GITHUB_API_TOKEN``. Un-delimited variants such as a
+      hypothetical ``GITHUBTOKEN`` are not matched by this branch; no real tool
+      sets such a name. Only these recognized shapes are scrubbed — this is not
+      blanket coverage of every conceivable secret name.
     """
     upper = key.upper()
     if upper.startswith(("GH_", "GITHUB_")):
@@ -1205,7 +1213,7 @@ def self_upgrade(
         help="Print the preview (method, current, target, installer argv) and "
         "exit 0 without launching the installer subprocess.",
     ),
-    tag: Optional[str] = typer.Option(
+    tag: str | None = typer.Option(
         None,
         "--tag",
         help="Pin the target version (vX.Y.Z[suffix]). Without --tag, the "
@@ -1339,10 +1347,20 @@ def self_upgrade(
             raise typer.Exit(0)
 
     # One-line pre-execution notice so the user sees exactly what will run
-    # before the installer's own output starts streaming.
+    # before the installer's own output starts streaming. A pinned target older
+    # than the installed version is a downgrade — say so explicitly so
+    # `--tag <older>` does not masquerade as a forward upgrade.
+    installed_version = _parse_version_text(plan.current_version)
+    verb = (
+        "Downgrading"
+        if tag is not None
+        and installed_version is not None
+        and target_version < installed_version
+        else "Upgrading"
+    )
     argv_str = _render_argv(plan.installer_argv) if plan.installer_argv else ""
     console.print(
-        f"Upgrading specify-cli {plan.current_version} → {plan.target_tag} "
+        f"{verb} specify-cli {plan.current_version} → {plan.target_tag} "
         f"via {_method_label(plan.method)}: {argv_str}",
         soft_wrap=True,
     )

tests/test_self_upgrade_detection.py

@@ -7,6 +7,8 @@
 from pathlib import Path
 from unittest.mock import patch
 
+import pytest
+
 import specify_cli
 from specify_cli import app
 
@@ -499,7 +501,9 @@ def test_pinned_older_tag_still_runs_installer(
         assert result.exit_code == 0
         out = strip_ansi(result.output)
         assert "Already on latest release" not in out
-        assert "Upgrading specify-cli 0.7.6 → v0.7.5 via uv tool:" in out
+        # A pinned older tag is a downgrade and must be labelled as such.
+        assert "Downgrading specify-cli 0.7.6 → v0.7.5 via uv tool:" in out
+        assert "Upgrading specify-cli" not in out
         assert mock_run.call_count == 2
 
     def test_pinned_rc_tag_uses_canonical_version_equality_for_noop(
@@ -729,20 +733,18 @@ def fake_run(argv, *args, **kwargs):
 
 
 class TestEditableInstallMetadata:
+    @pytest.mark.skipif(
+        not hasattr(importlib.metadata, "InvalidMetadataError"),
+        reason=(
+            "importlib.metadata.InvalidMetadataError does not exist on this "
+            "Python; _editable_direct_url_path only catches it when present, so "
+            "fabricating it would exercise a path that cannot fire in production"
+        ),
+    )
     def test_editable_marker_false_when_metadata_is_invalid(self):
-        invalid_metadata_error = getattr(importlib.metadata, "InvalidMetadataError", None)
-        if invalid_metadata_error is None:
-            class _FakeInvalidMetadataError(Exception):
-                pass
-
-            invalid_metadata_error = _FakeInvalidMetadataError
-
-        with patch.object(
-            importlib.metadata,
-            "InvalidMetadataError",
-            invalid_metadata_error,
-            create=True,
-        ), patch(
+        invalid_metadata_error = importlib.metadata.InvalidMetadataError
+
+        with patch(
             "importlib.metadata.distribution",
             side_effect=invalid_metadata_error("bad metadata"),
         ):

tests/test_self_upgrade_execution.py

@@ -386,7 +386,10 @@ def test_transient_exec_oserror_is_not_treated_as_invalid_installer(
         ), patch("specify_cli._version.subprocess.run", side_effect=transient_error):
             mock_urlopen.return_value = mock_urlopen_response({"tag_name": "v0.7.6"})
             result = runner.invoke(app, ["self", "upgrade"])
-        assert result.exit_code != 3
+        # Transient/unknown OSErrors are re-raised rather than mapped to the
+        # invalid-installer exit 3, so the CLI surfaces them as an uncaught
+        # error: exit code 1 with the original OSError preserved.
+        assert result.exit_code == 1
         assert isinstance(result.exception, OSError)