refactor(cli): address review nits on device-auth PR

- split `init` into manual-key and interactive helpers
- document that access_valid checks persisted expiry, not the token
- rename single-letter local in redacted()
- cover 500 alongside 404 in supports_device_login metadata probe
- add config edge-case tests: stale apiKey drop, garbage/string
  accessExpiresAt, empty-env-var popping, refresh-rotation fallback,
  missing-token access_valid

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

honcho-cli/src/honcho_cli/commands/setup.py

@@ -123,18 +123,26 @@ def init(
     # Non-interactive (JSON/piped) or an explicit --api-key: manual-key path.
     # Device login needs a human at a browser, so it's TTY-only.
     if use_json() or api_key:
-        final_key = _prompt_api_key(key_val)
-        final_url = _prompt_url(url_val)
-        if final_key != file_key or final_url != file_url:
-            CLIConfig(base_url=final_url, api_key=final_key).save()
-            if not use_json():
-                _console.print(f"  {ICON_OK} [dim]Saved to {CONFIG_FILE}[/dim]")
-        _check_connection(final_url, final_key)
-        if use_json():
-            print_result({"apiKey": _redact(final_key), "baseUrl": final_url})
-        return
+        _init_manual_key(key_val, url_val, file_key, file_url)
+    else:
+        _init_interactive(key_val, url_val, file_url)
 
-    # Interactive: URL first (device flow needs the host), then auth method.
+
+def _init_manual_key(key_val: str, url_val: str, file_key: str, file_url: str) -> None:
+    """Non-interactive path: confirm/save apiKey + URL, no device login."""
+    final_key = _prompt_api_key(key_val)
+    final_url = _prompt_url(url_val)
+    if final_key != file_key or final_url != file_url:
+        CLIConfig(base_url=final_url, api_key=final_key).save()
+        if not use_json():
+            _console.print(f"  {ICON_OK} [dim]Saved to {CONFIG_FILE}[/dim]")
+    _check_connection(final_url, final_key)
+    if use_json():
+        print_result({"apiKey": _redact(final_key), "baseUrl": final_url})
+
+
+def _init_interactive(key_val: str, url_val: str, file_url: str) -> None:
+    """Interactive path: URL first (device flow needs the host), then auth method."""
     final_url = _prompt_url(url_val)
     existing = CLIConfig.load()
     has_creds = bool(key_val) or bool(existing.oauth and existing.oauth.access_token)

honcho-cli/src/honcho_cli/config.py

@@ -84,7 +84,12 @@ class OAuthTokens:
     scope: str = ""
 
     def access_valid(self, skew: int = 60) -> bool:
-        """True while the access token is present and not within ``skew`` of expiry."""
+        """True while the access token is present and not within ``skew`` of expiry.
+
+        Checks the expiry timestamp recorded at mint time, not the token
+        itself — the server is the real authority, so a wrong answer here
+        costs at most an extra refresh or a 401.
+        """
         return bool(self.access_token) and time.time() < self.access_expires_at - skew
 
     @classmethod
@@ -223,7 +228,7 @@ class CLIConfig:
         Only includes fields that have a value set — per-command fields
         (workspace_id, peer_id, session_id) are omitted when empty.
         """
-        d: dict[str, str] = {}
+        result: dict[str, str] = {}
         for fld in fields(self):
             val = getattr(self, fld.name)
             if not val:
@@ -231,12 +236,12 @@ class CLIConfig:
             if fld.name == "api_key":
                 # Show ``***<last4>`` only — enough to compare keys without
                 # leaking the header or body of the JWT.
-                d[fld.name] = _redact_token(val)
+                result[fld.name] = _redact_token(val)
             elif fld.name == "oauth":
-                d[fld.name] = _redact_token(val.access_token)
+                result[fld.name] = _redact_token(val.access_token)
             else:
-                d[fld.name] = val
-        return d
+                result[fld.name] = val
+        return result
 
 
 def get_client_kwargs(config: CLIConfig) -> dict:

honcho-cli/tests/test_config.py

@@ -7,6 +7,7 @@ from pathlib import Path
 
 import pytest
 from honcho_cli.config import CLIConfig, OAuthTokens, _config_dir
+from honcho_cli.oauth import TokenResponse
 
 
 @pytest.fixture
@@ -60,6 +61,31 @@ class TestLoad:
         assert loaded.api_key == "env-key"
         assert loaded.base_url == "http://localhost:8000"
 
+    def test_empty_env_var_popped_from_environ(self, cfg_path, monkeypatch):
+        """Empty HONCHO_* vars are removed so the SDK doesn't crash on them."""
+        cfg_path.write_text(json.dumps({"apiKey": "file-key"}))
+        monkeypatch.setenv("HONCHO_API_KEY", "")
+        loaded = CLIConfig.load()
+        assert "HONCHO_API_KEY" not in os.environ
+        assert loaded.api_key == "file-key"
+
+    def test_garbage_access_expires_at_treated_as_expired(self, cfg_path):
+        """Hand-edited/corrupt expiry degrades to the refresh path, not a crash."""
+        cfg_path.write_text(json.dumps(
+            {"oauth": {"accessToken": "x", "accessExpiresAt": "not-a-number"}}
+        ))
+        loaded = CLIConfig.load()
+        assert loaded.oauth is not None
+        assert loaded.oauth.access_valid() is False
+
+    def test_numeric_string_access_expires_at_parses(self, cfg_path):
+        cfg_path.write_text(json.dumps(
+            {"oauth": {"accessToken": "x", "accessExpiresAt": "12345"}}
+        ))
+        loaded = CLIConfig.load()
+        assert loaded.oauth is not None
+        assert loaded.oauth.access_expires_at == 12345.0
+
 
 class TestSave:
     def test_writes_only_cli_owned_keys(self, cfg_path):
@@ -155,6 +181,12 @@ class TestOAuth:
         CLIConfig(base_url="http://localhost:8000").save()
         assert "oauth" not in json.loads(cfg_path.read_text())
 
+    def test_stale_api_key_is_dropped(self, cfg_path):
+        """Device login must clear an old manual key, or it wins over the fresh token forever."""
+        cfg_path.write_text(json.dumps({"apiKey": "old-manual-key"}))
+        CLIConfig(base_url="http://localhost:8000", oauth=self._tokens(9999999999)).save()
+        assert "apiKey" not in json.loads(cfg_path.read_text())
+
     def test_resolved_api_key_prefers_manual_key(self, cfg_path):
         cfg = CLIConfig(api_key="manual", oauth=self._tokens(9999999999))
         assert cfg.resolved_api_key() == "manual"
@@ -169,6 +201,25 @@ class TestOAuth:
         # inside the default 60s skew window → treated as invalid
         assert not self._tokens(time.time() + 30).access_valid()
 
+    def test_access_valid_false_without_token(self):
+        """A missing token is invalid even with a far-future expiry."""
+        tokens = OAuthTokens(access_token="", access_expires_at=time.time() + 3600)
+        assert tokens.access_valid() is False
+
+    def test_from_response_keeps_prior_refresh_token_when_not_rotated(self):
+        """Refresh-token rotation is optional (RFC 6749 §5.1) — keep the old one."""
+        resp = TokenResponse(
+            access_token="new-at", refresh_token="", expires_in=3600, scope=""
+        )
+        tokens = OAuthTokens.from_response(
+            resp,
+            client_id="honcho-cli",
+            scope_fallback="write",
+            refresh_fallback="prior-rt",
+        )
+        assert tokens.refresh_token == "prior-rt"
+        assert tokens.scope == "write"
+
     def test_redacted_masks_oauth_token(self):
         red = CLIConfig(oauth=self._tokens(1234)).redacted()
         assert red["oauth"] == "***at-x"

honcho-cli/tests/test_oauth.py

@@ -78,8 +78,9 @@ class TestSupportsDeviceLogin:
         with patch("honcho_cli.oauth.httpx.get", return_value=FakeResponse(200, body)):
             assert oauth.supports_device_login("https://api.honcho.dev") is False
 
-    def test_false_on_non_200(self):
-        with patch("honcho_cli.oauth.httpx.get", return_value=FakeResponse(404, "")):
+    @pytest.mark.parametrize("status", [404, 500])
+    def test_false_on_non_200(self, status):
+        with patch("honcho_cli.oauth.httpx.get", return_value=FakeResponse(status, "")):
             assert oauth.supports_device_login("http://localhost:8000") is False
 
     def test_false_on_connection_error(self):