Report post-redemption connect failures instead of raising tracebacks

Both paths fail *after* the control plane has consumed the single-use connect
token, so a raw traceback left the customer unable to tell whether to retry or
fetch a new token from the portal.

`http.client.IncompleteRead` (a truncated chunked reply) subclasses
`HTTPException`/`ValueError` -- neither `OSError` nor `URLError` -- so it escaped
both of `post_connect`'s handlers. A third clause is added *after* the transport
clause, because `RemoteDisconnected` is both a `ConnectionResetError` and a
`BadStatusLine` and means the peer never answered: that one keeps the retryable
503 copy, and a test pins the ordering.

`save_bootstrap` re-runs `validate_cloud_base_url` on both endpoints, which
resolves the host. A resolver that dies mid-connect raises `CloudUrlUnresolved`
and an endpoint that starts resolving to a rejected address raises a bare
`ValueError`; both are `ValueError`, so neither the `CloudSessionError` nor the
`OSError` handler in `connect()` covered them. Translated in `connect()` rather
than by dropping the re-validation, which is still wanted for `save_bootstrap`'s
other callers.

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

Author: Coding-Dev-Tools

CHANGELOG.md

@@ -19,7 +19,11 @@ All notable changes to Engraphis are documented here. Format loosely follows
   with a lapsed subscription). Session storage is pre-flighted before the exchange, so an
   unwritable state directory or a `cloud_session.json` replaced by a link fails the command
   *without* spending the single-use token — the customer fixes the path and retries with the
-  same token instead of returning to the portal for a new one. Also installed as
+  same token instead of returning to the portal for a new one. Faults that can only happen
+  *after* the exchange — a reply truncated mid-body (`http.client.IncompleteRead`), or an
+  endpoint that stops resolving before the session is written (`CloudUrlUnresolved`) — are
+  reported as errors that say the token was already used, rather than escaping as tracebacks
+  that leave the customer unable to tell whether to retry. Also installed as
   `engraphis-connect`.
 - An `engraphis` front-door command that dispatches to the existing `engraphis-<verb>`
   entry points, so the command the account portal displays is runnable as shown.

engraphis/device_connect.py

@@ -25,6 +25,7 @@
 """
 from __future__ import annotations
 
+import http.client
 import json
 import math
 import os
@@ -65,6 +66,17 @@
 #: The control plane answers with a small fixed record; anything larger is not ours.
 _MAX_RESPONSE_BYTES = 64 * 1024
 
+#: Copy for a reply that began but never completed.  Every other failure in this module can
+#: promise the connect token is untouched; this one cannot, because the request reached a
+#: control plane that started to answer, and a 200 consumes the token as it is written.
+#: Saying "try again" here would be a lie that costs the customer a second failed attempt,
+#: so the copy sends them to the one place that shows whether the device landed.
+_TRUNCATED_REPLY = (
+    "Engraphis Cloud started answering this connect request but the connection closed "
+    "before the reply was complete. Your connect token may already have been used: check "
+    "your account portal, and generate a new one if this device is not listed there."
+)
+
 #: The portal always mints tokens with this prefix.  Checking it locally turns a
 #: mistyped or truncated paste into an instant, free error instead of a round trip that
 #: consumes rate budget and returns the same opaque 401 as a genuinely dead token.
@@ -457,11 +469,25 @@ def post_connect(control_url: str, token: str, *, installation_client_id: str,
         raise _connect_http_error(status)
     except (urllib.error.URLError, TimeoutError, OSError) as exc:
         # ``exc`` may quote an internal host or a proxy URL; never reflect it.
+        #
+        # Deliberately ahead of the ``HTTPException`` clause: ``RemoteDisconnected`` is
+        # both a ``ConnectionResetError`` and a ``BadStatusLine``, and it means the peer
+        # closed without answering at all. Nothing was consumed, so "try again" is the
+        # correct advice for it and it must not inherit the token-may-be-spent copy.
         raise DeviceConnectError(
             "Engraphis Cloud is temporarily unreachable. Check your network and try "
             "again.",
             status=503,
         ) from exc
+    except http.client.HTTPException as exc:
+        # A truncated chunked body makes ``HTTPResponse.read`` raise ``IncompleteRead``,
+        # which subclasses ``HTTPException``/``ValueError`` and is neither an ``OSError``
+        # nor a ``URLError`` -- so it escaped both clauses above as a raw traceback. That
+        # happened at the worst moment: the control plane had already answered, so the
+        # single-use token was very likely spent, and the customer saw a stack trace
+        # instead of being told to check the portal. ``LineTooLong``/``BadStatusLine`` from
+        # reading the status line and headers land here for the same reason.
+        raise DeviceConnectError(_TRUNCATED_REPLY, status=502) from exc
     if len(raw) > _MAX_RESPONSE_BYTES:
         raise DeviceConnectError(
             "Engraphis Cloud returned an oversized connect response.", status=502
@@ -603,6 +629,20 @@ def connect(token: object, *, control_url: Optional[str] = None,
             "portal -- this one has been used." % session_path,
             status=409,
         ) from exc
+    except ValueError as exc:
+        # ``save_bootstrap`` re-runs ``validate_cloud_base_url`` on both endpoints, and
+        # that helper *resolves* the host. A resolver that dies between the pre-POST check
+        # and this line raises ``CloudUrlUnresolved``; an endpoint that starts resolving to
+        # a rejected address raises a bare ``ValueError``. Both are ``ValueError``, so
+        # neither the ``CloudSessionError`` nor the ``OSError`` clause above covered them
+        # and they escaped as a traceback -- again after the token was already spent.
+        raise DeviceConnectError(
+            "Engraphis Cloud accepted the token, but its endpoints could not be verified "
+            "in time to save the session, so nothing was written. Check your network, "
+            "then connect again with a new token from your account portal -- this one "
+            "has been used.",
+            status=409,
+        ) from exc
 
     summary = summarize(response)
     summary["control_url"] = resolved_control

tests/test_device_connect.py

@@ -7,6 +7,7 @@
 """
 from __future__ import annotations
 
+import http.client
 import json
 import os
 import urllib.error
@@ -17,6 +18,7 @@
 import pytest
 
 from engraphis import cloud_session, device_connect
+from engraphis.hosted_client import CloudUrlUnresolved
 from engraphis.private_state import UnsafeStateFile
 from scripts import connect as connect_cli
 
@@ -350,6 +352,132 @@ def open(self, request, timeout):
         device_connect.connect(TOKEN, control_url=CONTROL_URL, compute_url=COMPUTE_URL)
 
 
+# ------------------------------------------- failures *after* the token has been spent
+#
+# Once ``opener.open`` has returned, the control plane has answered and the single-use
+# connect token is gone.  Every fault from that point on has to arrive as a
+# ``DeviceConnectError`` that says so -- a traceback here is the worst possible outcome,
+# because the customer cannot tell whether to retry or fetch a new token.
+
+
+class _TruncatedBody:
+    """A 200 whose body stops mid-stream, the way a dropped chunked reply does."""
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, *exc) -> bool:
+        return False
+
+    def read(self, size: int = -1) -> bytes:
+        # What ``HTTPResponse._read_chunked`` raises for a truncated body.  It subclasses
+        # ``HTTPException``/``ValueError`` -- neither ``OSError`` nor ``URLError``.
+        raise http.client.IncompleteRead(b'{"organization_id":"org_al')
+
+
+@pytest.mark.parametrize("failure", [
+    http.client.IncompleteRead(b'{"organization_id":"org_al'),
+    http.client.LineTooLong("header line"),
+    http.client.BadStatusLine("garbage"),
+])
+def test_a_truncated_reply_reports_the_token_state_not_a_traceback(
+    monkeypatch, tmp_path, failure
+):
+    """``IncompleteRead`` is an ``HTTPException``, so the transport clause never saw it."""
+
+    class _Broken(_Opener):
+        def open(self, request, timeout):
+            self.calls.append({"body": request.data})
+            if isinstance(failure, http.client.IncompleteRead):
+                return _TruncatedBody()
+            raise failure
+
+    opener = _Broken()
+    _install_opener(monkeypatch, opener)
+
+    with pytest.raises(device_connect.DeviceConnectError) as caught:
+        device_connect.connect(TOKEN, control_url=CONTROL_URL, compute_url=COMPUTE_URL)
+
+    message = str(caught.value)
+    assert caught.value.status == 502
+    # The customer must be told the token may be gone rather than told to just retry.
+    assert "may already have been used" in message
+    assert TOKEN not in message
+    assert not (tmp_path / "cloud_session.json").exists()
+
+
+def test_a_connection_reset_before_any_reply_still_says_retry(monkeypatch):
+    """``RemoteDisconnected`` is *both* an ``OSError`` and an ``HTTPException``.
+
+    Nothing was read, so the token is untouched and "try again" remains the right copy;
+    it must not be swept into the token-may-be-spent message by the new clause.
+    """
+
+    _install_opener(
+        monkeypatch, _Opener(error=http.client.RemoteDisconnected("closed early"))
+    )
+
+    with pytest.raises(device_connect.DeviceConnectError) as caught:
+        device_connect.connect(TOKEN, control_url=CONTROL_URL, compute_url=COMPUTE_URL)
+
+    assert caught.value.status == 503
+    assert "temporarily unreachable" in str(caught.value)
+
+
+@pytest.mark.parametrize("failure", [
+    CloudUrlUnresolved("cloud service URL could not be resolved"),
+    ValueError("cloud service URL must not target private/reserved IP ranges"),
+])
+def test_endpoint_validation_failing_after_redemption_is_not_a_traceback(
+    monkeypatch, tmp_path, failure
+):
+    """``save_bootstrap`` re-resolves both endpoints *after* the POST spent the token.
+
+    ``CloudUrlUnresolved`` is a ``ValueError``, so neither the ``CloudSessionError`` nor
+    the ``OSError`` handler in ``connect()`` covers it: a resolver that dies mid-connect,
+    or a host that starts resolving to a rejected address, escaped as a raw traceback.
+    """
+
+    opener = _Opener(body=REGISTRATION)
+    _install_opener(monkeypatch, opener)
+
+    def _rejects(value):
+        raise failure
+
+    # Only the save path: the pre-POST checks in ``connect()`` already passed.
+    monkeypatch.setattr(cloud_session, "validate_cloud_base_url", _rejects)
+
+    with pytest.raises(device_connect.DeviceConnectError) as caught:
+        device_connect.connect(TOKEN, control_url=CONTROL_URL, compute_url=COMPUTE_URL)
+
+    message = str(caught.value)
+    assert caught.value.status == 409
+    assert opener.calls, "the token was spent; the copy has to admit it"
+    assert "has been used" in message
+    assert TOKEN not in message
+    assert not (tmp_path / "cloud_session.json").exists()
+
+
+def test_the_cli_reports_a_post_redemption_fault_instead_of_a_traceback(
+    monkeypatch, capsys
+):
+    _install_opener(monkeypatch, _Opener(body=REGISTRATION))
+
+    def _rejects(value):
+        raise CloudUrlUnresolved("cloud service URL could not be resolved")
+
+    monkeypatch.setattr(cloud_session, "validate_cloud_base_url", _rejects)
+
+    code = connect_cli.main([
+        "--token", TOKEN, "--control-url", CONTROL_URL, "--compute-url", COMPUTE_URL,
+    ])
+
+    err = capsys.readouterr().err
+    assert code == 1
+    assert "Traceback" not in err
+    assert "has been used" in err
+
+
 # ------------------------------------------------------ pre-flight on session storage
 #
 # The POST is the point of no return: the control plane consumes the single-use connect