From c3bbea4e64d89556bc446caedde28ad3b441b583 Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Tue, 16 Jun 2026 14:50:38 -0600 Subject: [PATCH] feat(services): mint consent links at request time, drop server-side browser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Optional hosted-service enable flows (SPL in apps/link, scout in apps/thinking) opened a browser on the journal host and only revealed a manual consent link if that open reported failure. On a headless/remote journal the host-side open reports bogus success, suppressing the link and stranding the UI at "setting up…". Now the enable route mints the nonce and builds the consent URL at request time, returns it in the 202 operation payload, and the background poll reuses that same nonce. The web UI opens it one-tap client-side (window.open) and always renders a persistent "continue to approve →" link; the start button is hidden while the operation is non-terminal and restored on terminal. The CLIs echo the consent URL unconditionally. Removes the server-side browser path entirely: the webbrowser imports, _open_browser/open_for_handoff/_tracked_opener, the open_browser params, and the browser_open_succeeded field on HandoffResult/OperationEntry and the API payload. The operation entry now owns portal_url from creation, and _update_entry_from_result no longer overwrites it on terminal. Request-time build failure (SPL instance resolution OSError) returns a real error_response instead of a stranded 202. Co-Authored-By: Claude Opus 4.8 (1M context) --- solstone/apps/link/call.py | 29 ++---- solstone/apps/link/copy.py | 2 +- solstone/apps/link/routes.py | 15 ++- .../apps/link/tests/test_private_link_call.py | 33 +++---- .../link/tests/test_private_link_routes.py | 53 +++++++--- solstone/apps/link/tests/test_reach_copy.py | 7 +- .../apps/link/tests/test_workspace_states.py | 21 ++++ solstone/apps/link/workspace.html | 15 +-- solstone/apps/thinking/call.py | 19 +++- solstone/apps/thinking/copy.py | 2 + solstone/apps/thinking/routes.py | 19 ++-- solstone/apps/thinking/static/thinking.js | 29 +++++- .../apps/thinking/tests/test_scout_lane.py | 4 +- .../apps/thinking/tests/test_scout_routes.py | 8 +- .../thinking/tests/test_workspace_html.py | 10 ++ solstone/apps/thinking/workspace.html | 1 + solstone/think/services/operations.py | 68 ++----------- solstone/think/services/outcomes.py | 2 +- solstone/think/services/portal_client.py | 17 ++++ solstone/think/services/scout_handoff.py | 63 ++++-------- solstone/think/services/spl_handoff.py | 67 ++++--------- tests/services/test_operations.py | 41 ++++---- tests/services/test_scout_handoff.py | 17 ++-- tests/services/test_spl_handoff.py | 97 +++++++++++++------ tests/test_thinking_call_parity.py | 5 +- 25 files changed, 340 insertions(+), 304 deletions(-) diff --git a/solstone/apps/link/call.py b/solstone/apps/link/call.py index d048104fa..773ad34ae 100644 --- a/solstone/apps/link/call.py +++ b/solstone/apps/link/call.py @@ -40,7 +40,7 @@ PRIVATE_LINK_SETUP_SUCCESS = ( "solstone private link is on. your devices can reach home from anywhere." ) PRIVATE_LINK_SETUP_FAILED = "couldn't finish setting up solstone private link." -PRIVATE_LINK_BROWSER_FALLBACK = "couldn't open your browser. open this link to finish:" +PRIVATE_LINK_PORTAL_CTA = "continue to approve →" PRIVATE_LINK_NEEDS_SUBSCRIPTION = ( "private link needs an active subscription before it can turn on. " "your consent is saved; set one up, then enable private link again:" @@ -134,20 +134,13 @@ def _post_private_link(path: str) -> dict[str, Any]: raise -def _maybe_echo_private_link_portal( - operation: dict[str, Any], - *, - already_echoed: bool, -) -> bool: - if already_echoed: - return True - if operation.get("browser_open_succeeded") is not False: - return False +def _maybe_echo_private_link_portal(operation: Any) -> None: + if not isinstance(operation, dict): + return portal_url = operation.get("portal_url") if not portal_url: - return False - typer.echo(f"{PRIVATE_LINK_BROWSER_FALLBACK} {portal_url}") - return True + return + typer.echo(f"{PRIVATE_LINK_PORTAL_CTA} {portal_url}") def _poll_private_link_until_terminal( @@ -157,7 +150,6 @@ def _poll_private_link_until_terminal( ) -> tuple[dict[str, Any], str | None, str | None]: deadline = time.monotonic() + max(0.0, wait_seconds) interval = max(0.0, poll_interval) - portal_echoed = False while True: status = _get_private_link_status() @@ -165,10 +157,6 @@ def _poll_private_link_until_terminal( if not isinstance(operation, dict): return status, None, None - portal_echoed = _maybe_echo_private_link_portal( - operation, - already_echoed=portal_echoed, - ) phase = str(operation.get("phase") or "") if phase in PRIVATE_LINK_TERMINAL_PHASES: guidance = operation.get("guidance") @@ -249,7 +237,10 @@ def private_link_setup( """Set up solstone private link.""" typer.echo(PRIVATE_LINK_SETTING_UP) - _post_private_link("/app/link/private-link/enable") + response = _post_private_link("/app/link/private-link/enable") + _maybe_echo_private_link_portal( + response.get("operation") if isinstance(response, dict) else None + ) status, phase, operation_guidance = _poll_private_link_until_terminal( wait_seconds=wait_seconds, poll_interval=poll_interval, diff --git a/solstone/apps/link/copy.py b/solstone/apps/link/copy.py index d0a10afae..ae3e6ca7e 100644 --- a/solstone/apps/link/copy.py +++ b/solstone/apps/link/copy.py @@ -112,7 +112,7 @@ REACH_SPL_CONNECTING_NOTE = "your home is connecting. this is usually quick." CHECK_AGAIN_LABEL = "check again" PRIVATE_LINK_DISABLE_CTA = "turn off solstone private link" PRIVATE_LINK_SETTING_UP = "setting up solstone private link…" -PRIVATE_LINK_BROWSER_FALLBACK = "couldn't open your browser. open this link to finish:" +PRIVATE_LINK_PORTAL_CTA = "continue to approve →" PRIVATE_LINK_SETUP_SUCCESS = ( "solstone private link is on. your devices can reach home from anywhere." ) diff --git a/solstone/apps/link/routes.py b/solstone/apps/link/routes.py index 99cbb8bdf..435607b45 100644 --- a/solstone/apps/link/routes.py +++ b/solstone/apps/link/routes.py @@ -374,10 +374,11 @@ def _private_link_status() -> dict[str, Any]: def _start_operation_response( service: str, kind: str, - flow: Callable[[Callable[[str], bool]], operations.HandoffResult], + portal_url: str | None, + flow: Callable[[], operations.HandoffResult], ) -> tuple[Response, int]: try: - operation = operations.start_operation(service, kind, flow) + operation = operations.start_operation(service, kind, portal_url, flow) except operations.OperationBusyError: return error_response(SERVICE_BUSY, detail="operation already running") return jsonify({"success": True, "service": service, "operation": operation}), 202 @@ -445,10 +446,18 @@ def private_link_enable() -> tuple[Response, int]: INVALID_OPERATION_FOR_STATE, detail="solstone private link is already on", ) + try: + consent_url, nonce, base_url = spl_handoff.build_spl_handoff_url() + except OSError: + return error_response( + SERVICE_OPERATION_FAILED, + detail="couldn't prepare the consent link", + ) return _start_operation_response( "spl", "spl_enable", - lambda opener: spl_handoff.run_spl_handoff(open_browser=opener), + consent_url, + lambda: spl_handoff.run_spl_handoff(nonce=nonce, base_url=base_url), ) diff --git a/solstone/apps/link/tests/test_private_link_call.py b/solstone/apps/link/tests/test_private_link_call.py index b6c0b022d..3fff100a8 100644 --- a/solstone/apps/link/tests/test_private_link_call.py +++ b/solstone/apps/link/tests/test_private_link_call.py @@ -65,7 +65,7 @@ def test_private_link_setup_success( monkeypatch.setattr( link_routes.spl_handoff, "run_spl_handoff", - lambda **_kwargs: operations.HandoffResult("enabled", None, False, True, None), + lambda **_kwargs: operations.HandoffResult("enabled", None, False), ) result = _invoke("setup", "--wait-seconds", "1", "--poll-interval", "0.01") @@ -75,29 +75,28 @@ def test_private_link_setup_success( assert "solstone private link is on" in result.stdout -def test_private_link_setup_browser_fallback_prints_portal_url( +def test_private_link_setup_prints_consent_url( link_env, monkeypatch: pytest.MonkeyPatch, ) -> None: env = link_env() _configure_cli(env, monkeypatch) + consent_url = "https://services.test/enable/spl?nonce=NONCE&instance=INSTANCE" + monkeypatch.setattr( + link_routes.spl_handoff, + "build_spl_handoff_url", + lambda: (consent_url, "NONCE", "https://services.test"), + ) monkeypatch.setattr( link_routes.spl_handoff, "run_spl_handoff", - lambda **_kwargs: operations.HandoffResult( - "enabled", - None, - False, - False, - "http://portal/x", - ), + lambda **_kwargs: operations.HandoffResult("enabled", None, False), ) result = _invoke("setup", "--wait-seconds", "1", "--poll-interval", "0.01") assert result.exit_code == 0 - assert "couldn't open your browser" in result.stdout - assert "http://portal/x" in result.stdout + assert f"{link_call.PRIVATE_LINK_PORTAL_CTA} {consent_url}" in result.stdout def test_private_link_setup_error_exits_nonzero( @@ -109,13 +108,7 @@ def test_private_link_setup_error_exits_nonzero( monkeypatch.setattr( link_routes.spl_handoff, "run_spl_handoff", - lambda **_kwargs: operations.HandoffResult( - "error", - "try again", - True, - True, - None, - ), + lambda **_kwargs: operations.HandoffResult("error", "try again", True), ) result = _invoke("setup", "--wait-seconds", "1", "--poll-interval", "0.01") @@ -140,9 +133,7 @@ def test_private_link_setup_needs_subscription_exits_zero( "needs_subscription", "private link needs an active subscription before it can turn on.", False, - True, - None, - subscribe_url, + subscribe_url=subscribe_url, ), ) diff --git a/solstone/apps/link/tests/test_private_link_routes.py b/solstone/apps/link/tests/test_private_link_routes.py index f9aa8973e..6bc3c779c 100644 --- a/solstone/apps/link/tests/test_private_link_routes.py +++ b/solstone/apps/link/tests/test_private_link_routes.py @@ -6,6 +6,7 @@ from __future__ import annotations import json import threading import time +import urllib.parse import pytest @@ -108,7 +109,7 @@ def test_private_link_enable_busy_returns_service_busy( def slow_flow(**_kwargs): started.set() release.wait(2) - return operations.HandoffResult("enabled", None, False, True, None) + return operations.HandoffResult("enabled", None, False) monkeypatch.setattr(link_routes.spl_handoff, "run_spl_handoff", slow_flow) @@ -132,6 +133,30 @@ def test_private_link_enable_already_enabled_guard(link_env): assert response.get_json()["reason_code"] == "invalid_operation_for_state" +def test_private_link_enable_prepare_failure_returns_service_operation_failed( + link_env, + monkeypatch: pytest.MonkeyPatch, +): + env = link_env() + monkeypatch.setattr( + link_routes.spl_handoff, + "build_spl_handoff_url", + lambda: (_ for _ in ()).throw(OSError("locked")), + ) + monkeypatch.setattr( + link_routes.spl_handoff, + "run_spl_handoff", + lambda **_kwargs: pytest.fail("handoff should not run"), + ) + + response = env.client.post("/app/link/private-link/enable") + + assert response.status_code == 500 + data = response.get_json() + assert data["reason_code"] == "service_operation_failed" + assert data["detail"] == "couldn't prepare the consent link" + + def test_private_link_enable_success_operation_reaches_enabled( link_env, monkeypatch: pytest.MonkeyPatch, @@ -141,7 +166,7 @@ def test_private_link_enable_success_operation_reaches_enabled( monkeypatch.setattr( link_routes.spl_handoff, "run_spl_handoff", - lambda **_kwargs: operations.HandoffResult("enabled", None, False, True, None), + lambda **_kwargs: operations.HandoffResult("enabled", None, False), ) response = env.client.post("/app/link/private-link/enable") @@ -151,30 +176,34 @@ def test_private_link_enable_success_operation_reaches_enabled( assert payload["operation"]["phase"] == "enabled" -def test_private_link_enable_browser_open_failure_is_surfaced( +def test_private_link_enable_returns_consent_url( link_env, monkeypatch: pytest.MonkeyPatch, ): env = link_env() + consent_url = "https://services.test/enable/spl?nonce=NONCE&instance=00000000-0000" + monkeypatch.setattr( + link_routes.spl_handoff, + "build_spl_handoff_url", + lambda: (consent_url, "NONCE", "https://services.test"), + ) monkeypatch.setattr( link_routes.spl_handoff, "run_spl_handoff", - lambda **_kwargs: operations.HandoffResult( - "enabled", - None, - False, - False, - "http://portal/x", - ), + lambda **_kwargs: operations.HandoffResult("enabled", None, False), ) response = env.client.post("/app/link/private-link/enable") + started = response.get_json() payload = _wait_for_phase(env, "enabled") + parsed = urllib.parse.urlparse(started["operation"]["portal_url"]) assert response.status_code == 202 - assert payload["operation"]["browser_open_succeeded"] is False - assert payload["operation"]["portal_url"] == "http://portal/x" + assert started["operation"]["portal_url"] == consent_url + assert parsed.path == "/enable/spl" + assert urllib.parse.parse_qs(parsed.query)["nonce"] == ["NONCE"] + assert payload["operation"]["portal_url"] == consent_url def test_private_link_disable_success(link_env): diff --git a/solstone/apps/link/tests/test_reach_copy.py b/solstone/apps/link/tests/test_reach_copy.py index 7c5a99982..a0422e610 100644 --- a/solstone/apps/link/tests/test_reach_copy.py +++ b/solstone/apps/link/tests/test_reach_copy.py @@ -39,7 +39,7 @@ U2_COPY_VALUES = [ copy.CHECK_AGAIN_LABEL, copy.PRIVATE_LINK_DISABLE_CTA, copy.PRIVATE_LINK_SETTING_UP, - copy.PRIVATE_LINK_BROWSER_FALLBACK, + copy.PRIVATE_LINK_PORTAL_CTA, copy.PRIVATE_LINK_SETUP_SUCCESS, copy.PRIVATE_LINK_SETUP_FAILED, copy.PRIVATE_LINK_DISABLE_SUCCESS, @@ -120,10 +120,7 @@ def test_reach_shell_corrected_copy_is_locked() -> None: assert copy.CHECK_AGAIN_LABEL == "check again" assert copy.PRIVATE_LINK_DISABLE_CTA == "turn off solstone private link" assert copy.PRIVATE_LINK_SETTING_UP == "setting up solstone private link…" - assert ( - copy.PRIVATE_LINK_BROWSER_FALLBACK - == "couldn't open your browser. open this link to finish:" - ) + assert copy.PRIVATE_LINK_PORTAL_CTA == "continue to approve →" assert ( copy.PRIVATE_LINK_SETUP_SUCCESS == "solstone private link is on. your devices can reach home from anywhere." diff --git a/solstone/apps/link/tests/test_workspace_states.py b/solstone/apps/link/tests/test_workspace_states.py index e88760842..9262ff345 100644 --- a/solstone/apps/link/tests/test_workspace_states.py +++ b/solstone/apps/link/tests/test_workspace_states.py @@ -193,3 +193,24 @@ def test_pair_modal_error_state_present(link_env) -> None: assert copy.PAIR_ERROR_BODY in body_text assert "function showPairError" in body assert body.count("showPairError(") >= 2 + + +def test_private_link_consent_link_rendering(link_env) -> None: + env = link_env() + response = env.client.get("/app/link/") + + assert response.status_code == 200 + body = response.get_data(as_text=True) + body_text = _normalized_body(body) + + assert "browser_open_" + "succeeded" not in body + assert "PRIVATE_LINK_TERMINAL_PHASES.has(operation.phase)" in body + assert "privateLinkSetup.hidden = !!operation &&" in body + assert "typeof operation.portal_url === 'string'" in body + assert "PRIVATE_LINK_PORTAL_CTA" in body + assert copy.PRIVATE_LINK_PORTAL_CTA in body_text + + setup_start = body.index("async function startPrivateLinkSetup") + setup_end = body.index("async function disablePrivateLink", setup_start) + setup_body = body[setup_start:setup_end] + assert "window.open(consentUrl, '_blank', 'noopener')" in setup_body diff --git a/solstone/apps/link/workspace.html b/solstone/apps/link/workspace.html index 18020bf1d..c61382f99 100644 --- a/solstone/apps/link/workspace.html +++ b/solstone/apps/link/workspace.html @@ -258,7 +258,7 @@ SUCCESS_VERIFY_NOTE_ANYWHERE: {{ link_copy.SUCCESS_VERIFY_NOTE_ANYWHERE|tojson }}, STATUS_SENTENCES: {{ link_copy.STATUS_SENTENCES|tojson }}, PRIVATE_LINK_SETTING_UP: {{ link_copy.PRIVATE_LINK_SETTING_UP|tojson }}, - PRIVATE_LINK_BROWSER_FALLBACK: {{ link_copy.PRIVATE_LINK_BROWSER_FALLBACK|tojson }}, + PRIVATE_LINK_PORTAL_CTA: {{ link_copy.PRIVATE_LINK_PORTAL_CTA|tojson }}, PRIVATE_LINK_SETUP_SUCCESS: {{ link_copy.PRIVATE_LINK_SETUP_SUCCESS|tojson }}, PRIVATE_LINK_SETUP_FAILED: {{ link_copy.PRIVATE_LINK_SETUP_FAILED|tojson }}, PRIVATE_LINK_NEEDS_SUBSCRIPTION_HEADLINE: {{ link_copy.PRIVATE_LINK_NEEDS_SUBSCRIPTION_HEADLINE|tojson }}, @@ -874,7 +874,7 @@ privateLinkOperationLink.href = linkUrl; privateLinkOperationLink.textContent = subscribeUrl ? window.LinkCopy.PRIVATE_LINK_NEEDS_SUBSCRIPTION_CTA || 'set up a subscription' - : portalUrl; + : window.LinkCopy.PRIVATE_LINK_PORTAL_CTA || 'continue to approve →'; } if (privateLinkOperationRetry) { privateLinkOperationRetry.hidden = !privateLinkRetryAction; @@ -885,6 +885,9 @@ function renderPrivateLinkStatus(data) { latestPrivateLink = data || {}; const operation = latestPrivateLink.operation; + if (privateLinkSetup) { + privateLinkSetup.hidden = !!operation && !PRIVATE_LINK_TERMINAL_PHASES.has(operation.phase); + } if (!operation) { if (latestPrivateLink.state === 'inconsistent') { setPrivateLinkOperation( @@ -915,13 +918,9 @@ retryAction = startPrivateLinkSetup; } - const portalUrl = !subscribeUrl && operation.browser_open_succeeded === false && operation.portal_url + const portalUrl = !subscribeUrl && typeof operation.portal_url === 'string' ? operation.portal_url : ''; - if (portalUrl) { - const fallback = window.LinkCopy.PRIVATE_LINK_BROWSER_FALLBACK || "couldn't open your browser. open this link to finish:"; - detail = detail ? `${detail} ${fallback}` : fallback; - } setPrivateLinkOperation(headline, detail, { portalUrl, subscribeUrl, retryAction }); } @@ -963,6 +962,8 @@ setPrivateLinkOperation(window.LinkCopy.PRIVATE_LINK_SETTING_UP || 'setting up solstone private link...'); try { const start = await window.apiJson('/app/link/private-link/enable', { method: 'POST' }); + const consentUrl = start?.operation?.portal_url; + if (consentUrl) window.open(consentUrl, '_blank', 'noopener'); renderPrivateLinkStatus({ operation: start?.operation || null }); await pollPrivateLinkUntilTerminal(); await refreshStatus(); diff --git a/solstone/apps/thinking/call.py b/solstone/apps/thinking/call.py index a4c6c29c2..7e1de2600 100644 --- a/solstone/apps/thinking/call.py +++ b/solstone/apps/thinking/call.py @@ -69,6 +69,7 @@ _SCOUT_GUIDANCE = { _SCOUT_STATE_MANUAL_KEY_PRESENT: "Clear the BYO Gemini key before enabling Scout.", _SCOUT_STATE_REPAIR_NEEDED: "Scout needs repair; try again from Thinking.", } +_SCOUT_CONSENT_CTA = "continue to approve →" app = typer.Typer(help="Thinking providers, keys, and local model setup.") @@ -169,6 +170,14 @@ def _echo_scout_guidance(key: Any) -> None: typer.echo(guidance) +def _maybe_echo_scout_portal(operation: Any) -> None: + if not isinstance(operation, dict): + return + portal_url = operation.get("portal_url") + if portal_url: + typer.echo(f"{_SCOUT_CONSENT_CTA} {portal_url}") + + def _poll_scout_until_terminal( *, wait_seconds: float, @@ -245,7 +254,10 @@ def scout_enable( ) -> None: """Enable Scout hosted Gemini.""" - _post_scout_action("/app/thinking/api/scout/enable") + response = _post_scout_action("/app/thinking/api/scout/enable") + _maybe_echo_scout_portal( + response.get("operation") if isinstance(response, dict) else None + ) status, phase, operation_guidance = _poll_scout_until_terminal( wait_seconds=wait_seconds, poll_interval=poll_interval, @@ -267,7 +279,10 @@ def scout_refresh( ) -> None: """Refresh Scout hosted Gemini status.""" - _post_scout_action("/app/thinking/api/scout/refresh") + response = _post_scout_action("/app/thinking/api/scout/refresh") + _maybe_echo_scout_portal( + response.get("operation") if isinstance(response, dict) else None + ) status, phase, operation_guidance = _poll_scout_until_terminal( wait_seconds=wait_seconds, poll_interval=poll_interval, diff --git a/solstone/apps/thinking/copy.py b/solstone/apps/thinking/copy.py index 41aa0521f..48cc9928a 100644 --- a/solstone/apps/thinking/copy.py +++ b/solstone/apps/thinking/copy.py @@ -88,6 +88,7 @@ SCOUT_RESTING_GUIDANCE = { SCOUT_STATE_MANUAL_KEY_PRESENT: "A Gemini key you manage is already set.", } SCOUT_MANUAL_KEY_BLOCK_COPY = "a Gemini key you manage is already set — clear it in your own key first, then turn on scout." +SCOUT_CONSENT_CTA = "continue to approve →" def thinking_copy_payload() -> dict[str, Any]: @@ -105,6 +106,7 @@ def thinking_copy_payload() -> dict[str, Any]: "state_labels": dict(SCOUT_STATE_LABELS), "resting_guidance": dict(SCOUT_RESTING_GUIDANCE), "manual_key_block": SCOUT_MANUAL_KEY_BLOCK_COPY, + "consent_cta": SCOUT_CONSENT_CTA, }, } diff --git a/solstone/apps/thinking/routes.py b/solstone/apps/thinking/routes.py index 71c4ef46f..847fd5515 100644 --- a/solstone/apps/thinking/routes.py +++ b/solstone/apps/thinking/routes.py @@ -95,10 +95,11 @@ def _thinking_operation_failed(detail: str = GENERIC_THINKING_ERROR) -> Any: def _start_scout_operation( kind: str, - flow: Callable[[Callable[[str], bool]], operations.HandoffResult], + portal_url: str | None, + flow: Callable[[], operations.HandoffResult], ) -> Any: try: - payload = operations.start_operation("scout", kind, flow) + payload = operations.start_operation("scout", kind, portal_url, flow) except operations.OperationBusyError: return error_response(SERVICE_BUSY, detail="operation already running") return ( @@ -444,11 +445,14 @@ def scout_enable() -> Any: INVALID_OPERATION_FOR_STATE, detail=thinking_copy.SCOUT_MANUAL_KEY_BLOCK_COPY, ) + consent_url, nonce, base_url = scout_handoff.build_scout_handoff_url() return _start_scout_operation( "enable", - lambda opener: scout_handoff.run_scout_handoff( + consent_url, + lambda: scout_handoff.run_scout_handoff( refresh=False, - open_browser=opener, + nonce=nonce, + base_url=base_url, ), ) except Exception: @@ -468,11 +472,14 @@ def scout_refresh() -> Any: INVALID_OPERATION_FOR_STATE, detail="Scout refresh isn't available right now.", ) + consent_url, nonce, base_url = scout_handoff.build_scout_handoff_url() return _start_scout_operation( "refresh", - lambda opener: scout_handoff.run_scout_handoff( + consent_url, + lambda: scout_handoff.run_scout_handoff( refresh=True, - open_browser=opener, + nonce=nonce, + base_url=base_url, ), ) except Exception: diff --git a/solstone/apps/thinking/static/thinking.js b/solstone/apps/thinking/static/thinking.js index 9a3b44622..dd8626074 100644 --- a/solstone/apps/thinking/static/thinking.js +++ b/solstone/apps/thinking/static/thinking.js @@ -56,6 +56,14 @@ } } + function setLink(id, url, text) { + const el = $(id); + if (!el) return; + el.hidden = !url; + el.href = url || ''; + el.textContent = url ? text : ''; + } + function setHidden(id, hidden) { const el = $(id); if (!el) return; @@ -442,6 +450,7 @@ setText('scoutSetupSub', 'checking scout'); setText('scoutSetupMeta', ''); setMessage('scoutLaneOperation', ''); + setLink('scoutLaneOperationLink', '', ''); setHidden('scoutCheckstrip', true); setButtonState('scoutEnable', false, true); setButtonState('scoutRefresh', false, true); @@ -481,7 +490,7 @@ const operation = scout.operation; const operationActive = !!operation && !scoutTerminalPhases.has(operation.phase); const actions = scout.actions || {}; - setButtonState('scoutEnable', !!actions.enable, operationActive || !actions.enable); + setButtonState('scoutEnable', !!actions.enable && !operationActive, !actions.enable); setButtonState( 'scoutRefresh', !!actions.refresh && scoutState !== 'on', @@ -498,8 +507,14 @@ operationGuidance ? `${phaseLabel} — ${operationGuidance}` : phaseLabel, phase === 'repair_needed' ? 'error' : '', ); + setLink( + 'scoutLaneOperationLink', + operation.portal_url || '', + scoutCopy.consent_cta || 'continue to approve →', + ); } else { setMessage('scoutLaneOperation', ''); + setLink('scoutLaneOperationLink', '', ''); } } @@ -853,6 +868,11 @@ return state.scout?.operation || null; } + function openConsentTab(operation) { + const url = operation?.portal_url; + if (url) window.open(url, '_blank', 'noopener'); + } + async function refreshLocalModels() { state.localModels = await api('api/local/models'); renderLocalModels(); @@ -891,12 +911,14 @@ async function enableScout() { setMessage('scoutLaneOperation', ''); + let start; try { - await api('api/scout/enable', {method: 'POST'}); + start = await api('api/scout/enable', {method: 'POST'}); } catch (err) { setMessage('scoutLaneOperation', err.message, 'error'); return; } + openConsentTab(start?.operation); const operation = await pollScoutUntilTerminal(); const phase = operation?.phase; @@ -923,7 +945,8 @@ } async function refreshScoutOp() { - await api('api/scout/refresh', {method: 'POST'}); + const start = await api('api/scout/refresh', {method: 'POST'}); + openConsentTab(start?.operation); await pollScoutUntilTerminal(); if (state.scout?.state === 'on') { await Promise.all([refreshProviders(), refreshKeys()]); diff --git a/solstone/apps/thinking/tests/test_scout_lane.py b/solstone/apps/thinking/tests/test_scout_lane.py index 59cfb0dff..3b8be6368 100644 --- a/solstone/apps/thinking/tests/test_scout_lane.py +++ b/solstone/apps/thinking/tests/test_scout_lane.py @@ -89,8 +89,7 @@ def test_operation_phase_maps_to_product_phase( "phase": raw_phase, "guidance": "next", "retryable": False, - "browser_open_succeeded": True, - "portal_url": None, + "portal_url": "https://services.test/enable/scout?nonce=NONCE", "elapsed_ms": 12, } ) @@ -98,6 +97,7 @@ def test_operation_phase_maps_to_product_phase( assert payload is not None assert payload["phase"] == expected assert payload["kind"] == "enable" + assert payload["portal_url"] == "https://services.test/enable/scout?nonce=NONCE" assert payload["elapsed_ms"] == 12 diff --git a/solstone/apps/thinking/tests/test_scout_routes.py b/solstone/apps/thinking/tests/test_scout_routes.py index 83ef0024f..fd084578d 100644 --- a/solstone/apps/thinking/tests/test_scout_routes.py +++ b/solstone/apps/thinking/tests/test_scout_routes.py @@ -116,7 +116,7 @@ def test_enable_success_remaps_terminal_phase_and_reads_enabled_state( ) -> None: def runner(**_kwargs): scout.provision_scout_handoff(_approved_payload()) - return operations.HandoffResult("enabled", None, False, True, None) + return operations.HandoffResult("enabled", None, False) monkeypatch.setattr(scout_handoff, "run_scout_handoff", runner) @@ -167,7 +167,7 @@ def test_refresh_allowed_only_when_requested_or_on( monkeypatch.setattr( scout_handoff, "run_scout_handoff", - lambda **_kwargs: operations.HandoffResult("pending", None, False, True, None), + lambda **_kwargs: operations.HandoffResult("pending", None, False), ) off_response = thinking_client.post("/app/thinking/api/scout/refresh") @@ -188,7 +188,7 @@ def test_service_busy_for_second_scout_operation( def runner(**_kwargs): started.set() release.wait(2) - return operations.HandoffResult("enabled", None, False, True, None) + return operations.HandoffResult("enabled", None, False) monkeypatch.setattr(scout_handoff, "run_scout_handoff", runner) @@ -298,8 +298,6 @@ def test_terminal_phase_remap( raw_phase, "next step", raw_phase == "error", - True, - None, ), ) diff --git a/solstone/apps/thinking/tests/test_workspace_html.py b/solstone/apps/thinking/tests/test_workspace_html.py index ace56200e..0ba13bc4f 100644 --- a/solstone/apps/thinking/tests/test_workspace_html.py +++ b/solstone/apps/thinking/tests/test_workspace_html.py @@ -38,6 +38,7 @@ def test_workspace_renders_each_lane(settings_env): assert 'id="scoutRefresh"' in html assert 'id="scoutDisable"' in html assert 'id="scoutLaneOperation"' in html + assert 'id="scoutLaneOperationLink"' in html for view in ("main", "scout-setup", "byo-setup", "local-setup", "lane-switch"): assert f'data-view="{view}"' in html assert 'data-open-view="scout-setup"' in html @@ -67,6 +68,15 @@ def test_workspace_renders_each_lane(settings_env): assert "thinking/static/thinking.js" in html +def test_scout_consent_static_behavior_is_wired() -> None: + js = STATIC.read_text(encoding="utf-8") + + assert "window.open(url, '_blank', 'noopener')" in js + assert "scoutLaneOperationLink" in js + assert "operation.portal_url || ''" in js + assert "!!actions.enable && !operationActive" in js + + def test_copy_payload_round_trips_apostrophes() -> None: payload = thinking_copy.thinking_copy_payload() encoded = json.dumps(payload) diff --git a/solstone/apps/thinking/workspace.html b/solstone/apps/thinking/workspace.html index 98d908d24..69a2831fd 100644 --- a/solstone/apps/thinking/workspace.html +++ b/solstone/apps/thinking/workspace.html @@ -565,6 +565,7 @@
+
diff --git a/solstone/think/services/operations.py b/solstone/think/services/operations.py index 64a71b00f..ed27bd57c 100644 --- a/solstone/think/services/operations.py +++ b/solstone/think/services/operations.py @@ -8,7 +8,6 @@ from __future__ import annotations import logging import threading import time -import webbrowser from collections.abc import Callable from dataclasses import dataclass from typing import Any @@ -32,8 +31,6 @@ class HandoffResult: phase: str guidance: str | None retryable: bool - browser_open_succeeded: bool | None - portal_url: str | None subscribe_url: str | None = None @@ -44,7 +41,6 @@ class OperationEntry: phase: str guidance: str | None retryable: bool - browser_open_succeeded: bool | None portal_url: str | None started_monotonic: float ended_monotonic: float | None = None @@ -62,58 +58,28 @@ def clear_registry() -> None: _REGISTRY.clear() -def _open_browser(url: str) -> bool: - try: - return bool(webbrowser.open(url, new=2)) - except Exception as exc: - logger.warning("service browser open failed: %s", exc) - return False - - -def open_for_handoff( - browser_url: str, - open_browser: Callable[[str], bool], -) -> tuple[bool, str | None]: - try: - browser_open_succeeded = bool(open_browser(browser_url)) - except Exception as exc: - logger.warning("service browser open failed: %s", exc) - browser_open_succeeded = False - return browser_open_succeeded, browser_url if not browser_open_succeeded else None - - def _outcome_result( code: str, guidance: str | None, - browser_open_succeeded: bool | None, - portal_url: str | None, subscribe_url: str | None = None, ) -> HandoffResult: if code == outcomes.APPROVED: - return HandoffResult("enabled", None, False, browser_open_succeeded, portal_url) + return HandoffResult("enabled", None, False) if code == outcomes.PENDING: - return HandoffResult( - "pending", guidance, False, browser_open_succeeded, portal_url - ) + return HandoffResult("pending", guidance, False) if code == outcomes.REVOKED: - return HandoffResult( - "revoked", guidance, False, browser_open_succeeded, portal_url - ) + return HandoffResult("revoked", guidance, False) if code == outcomes.NEEDS_SUBSCRIPTION: return HandoffResult( "needs_subscription", guidance, False, - browser_open_succeeded, - portal_url, subscribe_url=subscribe_url, ) return HandoffResult( "error", guidance, code in RETRYABLE_CODES, - browser_open_succeeded, - portal_url, ) @@ -144,7 +110,6 @@ def _operation_payload( "phase": entry.phase, "guidance": entry.guidance, "retryable": entry.retryable, - "browser_open_succeeded": entry.browser_open_succeeded, "portal_url": entry.portal_url, "subscribe_url": entry.subscribe_url, "elapsed_ms": int(max(0.0, ts - entry.started_monotonic) * 1000), @@ -162,43 +127,26 @@ def _update_entry_from_result( entry.phase = result.phase entry.guidance = result.guidance entry.retryable = result.retryable - entry.browser_open_succeeded = result.browser_open_succeeded - entry.portal_url = result.portal_url entry.subscribe_url = result.subscribe_url entry.ended_monotonic = time.monotonic() -def _tracked_opener(entry: OperationEntry) -> Callable[[str], bool]: - def opener(url: str) -> bool: - browser_open_succeeded, manual_url = open_for_handoff(url, _open_browser) - with _REGISTRY_LOCK: - current = _REGISTRY.get(entry.service) - if current is entry: - entry.browser_open_succeeded = browser_open_succeeded - entry.portal_url = manual_url - return browser_open_succeeded - - return opener - - def _run_operation( entry: OperationEntry, - flow: Callable[[Callable[[str], bool]], HandoffResult], + flow: Callable[[], HandoffResult], ) -> None: with _REGISTRY_LOCK: current = _REGISTRY.get(entry.service) if current is entry: entry.phase = "waiting" try: - result = flow(_tracked_opener(entry)) + result = flow() except Exception: logger.exception("service operation failed") result = HandoffResult( phase="error", guidance=None, retryable=True, - browser_open_succeeded=entry.browser_open_succeeded, - portal_url=entry.portal_url, ) _update_entry_from_result(entry, result) @@ -206,7 +154,8 @@ def _run_operation( def start_operation( service: str, kind: str, - flow: Callable[[Callable[[str], bool]], HandoffResult], + portal_url: str | None, + flow: Callable[[], HandoffResult], ) -> dict[str, Any]: with _REGISTRY_LOCK: now = time.monotonic() @@ -219,8 +168,7 @@ def start_operation( phase="starting", guidance=None, retryable=False, - browser_open_succeeded=None, - portal_url=None, + portal_url=portal_url, started_monotonic=now, ) _REGISTRY[service] = entry diff --git a/solstone/think/services/outcomes.py b/solstone/think/services/outcomes.py index a66c4674c..4374938c6 100644 --- a/solstone/think/services/outcomes.py +++ b/solstone/think/services/outcomes.py @@ -34,7 +34,7 @@ CODES = frozenset( GUIDANCE: dict[str, str | None] = { APPROVED: None, - PENDING: "Keep the browser open while the request finishes.", + PENDING: "Keep the approval page open while the request finishes.", REVOKED: "Consent was not granted. Start a new enable flow when ready.", EXPIRED: "This enable link is no longer active. Start a new enable flow.", MALFORMED: ( diff --git a/solstone/think/services/portal_client.py b/solstone/think/services/portal_client.py index 86a6e7ea3..cf557a249 100644 --- a/solstone/think/services/portal_client.py +++ b/solstone/think/services/portal_client.py @@ -94,6 +94,23 @@ def browser_url( return url +def build_consent_url( + service: str, *, instance: str | None = None +) -> tuple[str, str, str]: + """Mint a nonce and build the portal consent URL for ``service``. + + Returns ``(consent_url, nonce, base_url)``. + """ + + base_url = portal_base_url() + nonce = mint_nonce() + return ( + browser_url(base_url, nonce, service=service, instance=instance), + nonce, + base_url, + ) + + def is_timeout_error(exc: BaseException) -> bool: if isinstance(exc, (socket.timeout, TimeoutError)): return True diff --git a/solstone/think/services/scout_handoff.py b/solstone/think/services/scout_handoff.py index 06d1c184d..2a85866ca 100644 --- a/solstone/think/services/scout_handoff.py +++ b/solstone/think/services/scout_handoff.py @@ -1,7 +1,7 @@ # SPDX-License-Identifier: AGPL-3.0-only # Copyright (c) 2026 sol pbc -"""Scout browser-consent handoff runner.""" +"""Scout consent handoff runner.""" from __future__ import annotations @@ -16,44 +16,41 @@ logger = logging.getLogger(__name__) SERVICE_SCOUT = "scout" +def build_scout_handoff_url() -> tuple[str, str, str]: + """Mint a nonce and build the scout consent URL. + + Returns ``(consent_url, nonce, base_url)``. + """ + + return portal_client.build_consent_url(SERVICE_SCOUT) + + def _handoff_error_result( token: str, *, detail: str | None = None, - browser_open_succeeded: bool | None, - portal_url: str | None, ) -> operations.HandoffResult: try: outcome = outcomes.outcome_from_token(token, detail=detail) except ValueError: outcome = outcomes.outcome_for_code(outcomes.LOCAL_ERROR, detail=detail) - return operations._outcome_result( - outcome.code, - outcome.guidance, - browser_open_succeeded, - portal_url, - ) + return operations._outcome_result(outcome.code, outcome.guidance) def run_scout_handoff( *, refresh: bool, + nonce: str, + base_url: str, poll_once: Callable[ ..., portal_client.PollOutcome ] = portal_client.poll_handoff_once, - open_browser: Callable[[str], bool] = operations._open_browser, clock: Callable[[], float] = time.monotonic, wait_seconds: int = portal_client.DEFAULT_WAIT_SECONDS, ) -> operations.HandoffResult: - """Run the scout browser-consent flow synchronously for route/thread callers.""" + """Run the scout consent flow synchronously for route/thread callers.""" _ = refresh - base_url = portal_client.portal_base_url() - nonce = portal_client.mint_nonce() - browser_url = portal_client.browser_url(base_url, nonce, service=SERVICE_SCOUT) - browser_open_succeeded, manual_url = operations.open_for_handoff( - browser_url, open_browser - ) deadline = clock() + wait_seconds while clock() < deadline: @@ -72,21 +69,13 @@ def run_scout_handoff( return _handoff_error_result( outcome.reason, detail=outcome.detail, - browser_open_succeeded=browser_open_succeeded, - portal_url=manual_url, ) return _handoff_error_result( "unexpected_payload", detail=outcome.detail, - browser_open_succeeded=browser_open_succeeded, - portal_url=manual_url, ) if outcome.kind != "success": - return _handoff_error_result( - "unexpected_payload", - browser_open_succeeded=browser_open_succeeded, - portal_url=manual_url, - ) + return _handoff_error_result("unexpected_payload") try: result = scout.apply_scout_state(outcome.payload or {}) @@ -94,22 +83,12 @@ def run_scout_handoff( return _handoff_error_result( exc.token, detail=exc.detail, - browser_open_succeeded=browser_open_succeeded, - portal_url=manual_url, ) except scout.JournalNotInitializedError: - return _handoff_error_result( - "journal_not_initialized", - browser_open_succeeded=browser_open_succeeded, - portal_url=manual_url, - ) + return _handoff_error_result("journal_not_initialized") except Exception: logger.exception("scout handoff write failed") - return _handoff_error_result( - "write_failed", - browser_open_succeeded=browser_open_succeeded, - portal_url=manual_url, - ) + return _handoff_error_result("write_failed") if result.kind == "approved": guidance = None @@ -124,12 +103,6 @@ def run_scout_handoff( phase=phase, guidance=guidance, retryable=False, - browser_open_succeeded=browser_open_succeeded, - portal_url=manual_url, ) - return _handoff_error_result( - "consent_timeout", - browser_open_succeeded=browser_open_succeeded, - portal_url=manual_url, - ) + return _handoff_error_result("consent_timeout") diff --git a/solstone/think/services/spl_handoff.py b/solstone/think/services/spl_handoff.py index ccf43dad4..53fb39557 100644 --- a/solstone/think/services/spl_handoff.py +++ b/solstone/think/services/spl_handoff.py @@ -1,13 +1,11 @@ # SPDX-License-Identifier: AGPL-3.0-only # Copyright (c) 2026 sol pbc -"""Presentation-neutral spl browser-consent handoff flow.""" +"""Presentation-neutral spl consent handoff flow.""" from __future__ import annotations -import logging import time -import webbrowser from collections.abc import Callable from typing import Any @@ -15,8 +13,6 @@ from solstone.think.link.paths import LinkState from solstone.think.services import operations, outcomes, portal_client, spl from solstone.think.services.constants import SERVICE_SPL -log = logging.getLogger(__name__) - _STATES = frozenset( { outcomes.APPROVED, @@ -34,12 +30,14 @@ class MalformedConsent(ValueError): """Raised when the spl consent payload violates the wire contract.""" -def _open_browser(url: str) -> bool: - try: - return bool(webbrowser.open(url, new=2)) - except Exception as exc: - log.warning("spl browser open failed: %s", exc) - return False +def build_spl_handoff_url() -> tuple[str, str, str]: + """Resolve the link instance, mint a nonce, and build the spl consent URL. + + Returns ``(consent_url, nonce, base_url)``. + """ + + instance_id = LinkState.load_or_create().instance_id + return portal_client.build_consent_url(SERVICE_SPL, instance=instance_id) def _is_approved_at(value: Any) -> bool: @@ -75,33 +73,12 @@ def _classify_spl_payload(payload: dict[str, Any]) -> str: def enable_spl_via_consent( *, - base_url: str | None = None, - instance_id: str | None = None, + base_url: str, + nonce: str, wait_seconds: int = portal_client.DEFAULT_WAIT_SECONDS, - open_browser: Callable[[str], bool] | None = None, poll_once: Callable[..., portal_client.PollOutcome] | None = None, clock: Callable[[], float] = time.monotonic, ) -> outcomes.HandoffOutcome: - resolved_base_url = base_url or portal_client.portal_base_url() - nonce = portal_client.mint_nonce() - if instance_id is None: - try: - instance_id = LinkState.load_or_create().instance_id - except OSError: - log.warning("spl instance id resolution failed", exc_info=True) - return outcomes.outcome_for_code(outcomes.LOCAL_ERROR) - browser_url = portal_client.browser_url( - resolved_base_url, - nonce, - service=SERVICE_SPL, - instance=instance_id, - ) - opener = open_browser or _open_browser - try: - opener(browser_url) - except Exception as exc: - log.warning("spl browser open failed: %s", exc) - poll = poll_once or portal_client.poll_handoff_once deadline = clock() + wait_seconds while clock() < deadline: @@ -110,7 +87,7 @@ def enable_spl_via_consent( max(0.1, deadline - clock()), ) outcome = poll( - resolved_base_url, + base_url, nonce, timeout=timeout, component="switchboard", @@ -162,27 +139,19 @@ def enable_spl_via_consent( def run_spl_handoff( *, + nonce: str, + base_url: str, poll_once: Callable[ ..., portal_client.PollOutcome ] = portal_client.poll_handoff_once, - open_browser: Callable[[str], bool] = operations._open_browser, clock: Callable[[], float] = time.monotonic, wait_seconds: int = portal_client.DEFAULT_WAIT_SECONDS, ) -> operations.HandoffResult: - """Run the spl browser-consent flow synchronously for route/thread callers.""" - - browser_open_succeeded: bool | None = None - manual_url: str | None = None - - def wrapped_open_browser(url: str) -> bool: - nonlocal browser_open_succeeded, manual_url - browser_open_succeeded, manual_url = operations.open_for_handoff( - url, open_browser - ) - return browser_open_succeeded + """Run the spl consent flow synchronously for route/thread callers.""" outcome = enable_spl_via_consent( - open_browser=wrapped_open_browser, + base_url=base_url, + nonce=nonce, poll_once=poll_once, clock=clock, wait_seconds=wait_seconds, @@ -190,7 +159,5 @@ def run_spl_handoff( return operations._outcome_result( outcome.code, outcome.guidance, - browser_open_succeeded, - manual_url, subscribe_url=outcome.detail, ) diff --git a/tests/services/test_operations.py b/tests/services/test_operations.py index 3dbd959f1..b6c19914d 100644 --- a/tests/services/test_operations.py +++ b/tests/services/test_operations.py @@ -31,16 +31,18 @@ def test_start_operation_raises_busy_for_same_service() -> None: started = threading.Event() release = threading.Event() - def flow(_open_browser): + def flow(): started.set() release.wait(2) - return operations.HandoffResult("enabled", None, False, True, None) + return operations.HandoffResult("enabled", None, False) - operations.start_operation("scout", "enable", flow) + operations.start_operation("scout", "enable", "https://portal.test/scout", flow) _wait_until(started.is_set) with pytest.raises(operations.OperationBusyError): - operations.start_operation("scout", "refresh", flow) + operations.start_operation( + "scout", "refresh", "https://portal.test/scout", flow + ) release.set() @@ -50,18 +52,20 @@ def test_different_services_can_run_concurrently() -> None: spl_started = threading.Event() release = threading.Event() - def scout_flow(_open_browser): + def scout_flow(): scout_started.set() release.wait(2) - return operations.HandoffResult("enabled", None, False, True, None) + return operations.HandoffResult("enabled", None, False) - def spl_flow(_open_browser): + def spl_flow(): spl_started.set() release.wait(2) - return operations.HandoffResult("enabled", None, False, True, None) + return operations.HandoffResult("enabled", None, False) - operations.start_operation("scout", "enable", scout_flow) - operations.start_operation("spl", "spl_enable", spl_flow) + operations.start_operation( + "scout", "enable", "https://portal.test/scout", scout_flow + ) + operations.start_operation("spl", "spl_enable", "https://portal.test/spl", spl_flow) _wait_until(scout_started.is_set) _wait_until(spl_started.is_set) @@ -74,9 +78,8 @@ def test_sweep_drops_completed_entry_after_grace( operations.start_operation( "scout", "enable", - lambda _open_browser: operations.HandoffResult( - "enabled", None, False, True, None - ), + "https://portal.test/scout", + lambda: operations.HandoffResult("enabled", None, False), ) _wait_until(lambda: operations.operation_for_service("scout")["phase"] == "enabled") @@ -86,10 +89,10 @@ def test_sweep_drops_completed_entry_after_grace( def test_flow_exception_becomes_retryable_error() -> None: - def fail(_open_browser): + def fail(): raise RuntimeError("boom") - operations.start_operation("scout", "enable", fail) + operations.start_operation("scout", "enable", "https://portal.test/scout", fail) def is_error() -> bool: operation = operations.operation_for_service("scout") @@ -98,17 +101,15 @@ def test_flow_exception_becomes_retryable_error() -> None: _wait_until(is_error) operation = operations.operation_for_service("scout") assert operation["retryable"] is True - assert operation["browser_open_succeeded"] is None - assert operation["portal_url"] is None + assert operation["portal_url"] == "https://portal.test/scout" def test_clear_registry_empties_entries() -> None: operations.start_operation( "scout", "enable", - lambda _open_browser: operations.HandoffResult( - "enabled", None, False, True, None - ), + "https://portal.test/scout", + lambda: operations.HandoffResult("enabled", None, False), ) _wait_until(lambda: operations.operation_for_service("scout") is not None) diff --git a/tests/services/test_scout_handoff.py b/tests/services/test_scout_handoff.py index 5261af145..d632cbd53 100644 --- a/tests/services/test_scout_handoff.py +++ b/tests/services/test_scout_handoff.py @@ -41,7 +41,8 @@ def _config(journal: Path) -> dict: def test_run_scout_handoff_maps_approved_to_enabled(journal_copy: Path) -> None: result = scout_handoff.run_scout_handoff( refresh=False, - open_browser=lambda _url: True, + nonce="NONCE", + base_url="http://portal.test", poll_once=lambda *_args, **_kwargs: PollOutcome( kind="success", payload=_approved_payload(), @@ -50,8 +51,6 @@ def test_run_scout_handoff_maps_approved_to_enabled(journal_copy: Path) -> None: assert result.phase == "enabled" assert result.retryable is False - assert result.browser_open_succeeded is True - assert result.portal_url is None saved = _config(journal_copy) assert saved["env"]["GOOGLE_API_KEY"] == "google-one" assert saved["services"]["scout"]["account_id"] == "acct-one" @@ -60,7 +59,8 @@ def test_run_scout_handoff_maps_approved_to_enabled(journal_copy: Path) -> None: def test_run_scout_handoff_maps_pending(journal_copy: Path) -> None: result = scout_handoff.run_scout_handoff( refresh=True, - open_browser=lambda _url: True, + nonce="NONCE", + base_url="http://portal.test", poll_once=lambda *_args, **_kwargs: PollOutcome( kind="success", payload={ @@ -83,7 +83,8 @@ def test_run_scout_handoff_maps_pending(journal_copy: Path) -> None: def test_run_scout_handoff_maps_revoked(journal_copy: Path) -> None: result = scout_handoff.run_scout_handoff( refresh=True, - open_browser=lambda _url: True, + nonce="NONCE", + base_url="http://portal.test", poll_once=lambda *_args, **_kwargs: PollOutcome( kind="success", payload={"state": "revoked"}, @@ -111,7 +112,8 @@ def test_run_scout_handoff_error_outcomes_do_not_write_journal( result = scout_handoff.run_scout_handoff( refresh=True, - open_browser=lambda _url: True, + nonce="NONCE", + base_url="http://portal.test", poll_once=lambda *_args, **_kwargs: PollOutcome(kind="failed", reason=reason), ) @@ -127,7 +129,8 @@ def test_run_scout_handoff_malformed_apply_does_not_write_journal( result = scout_handoff.run_scout_handoff( refresh=True, - open_browser=lambda _url: True, + nonce="NONCE", + base_url="http://portal.test", poll_once=lambda *_args, **_kwargs: PollOutcome( kind="success", payload={ diff --git a/tests/services/test_spl_handoff.py b/tests/services/test_spl_handoff.py index bad159095..744ce726b 100644 --- a/tests/services/test_spl_handoff.py +++ b/tests/services/test_spl_handoff.py @@ -27,6 +27,8 @@ from solstone.think.services import ( from solstone.think.spl import relay_client TEST_INSTANCE_ID = "00000000-0000-4000-8000-000000000000" +TEST_NONCE = "TESTNONCE" +TEST_BASE_URL = "https://services.test" TEST_SUBSCRIBE_URL = "https://services.test/account/subscription" @@ -109,13 +111,13 @@ def _set_posture(journal_copy: Path, posture: str) -> None: def _run( *, - instance_id: str | None = TEST_INSTANCE_ID, + base_url: str = TEST_BASE_URL, + nonce: str = TEST_NONCE, **kwargs, ) -> outcomes.HandoffOutcome: return spl_handoff.enable_spl_via_consent( - base_url="https://services.test", - instance_id=instance_id, - open_browser=lambda _url: True, + base_url=base_url, + nonce=nonce, **kwargs, ) @@ -310,7 +312,7 @@ def test_poll_failures_map_to_taxonomy(monkeypatch, item: Any, code: str) -> Non assert outcome.code == code -def test_instance_resolution_failure_returns_local_error(monkeypatch) -> None: +def test_build_spl_handoff_url_raises_when_link_state_unreadable(monkeypatch) -> None: def fail_load_or_create(*, default_label: str = "solstone") -> LinkState: _ = default_label raise OSError("locked") @@ -321,13 +323,8 @@ def test_instance_resolution_failure_returns_local_error(monkeypatch) -> None: staticmethod(fail_load_or_create), ) - outcome = spl_handoff.enable_spl_via_consent( - base_url="https://services.test", - open_browser=lambda _url: pytest.fail("browser should not open"), - poll_once=lambda *_args, **_kwargs: pytest.fail("poll should not run"), - ) - - assert outcome.code == outcomes.LOCAL_ERROR + with pytest.raises(OSError): + spl_handoff.build_spl_handoff_url() def test_relay_unreachable_after_approval_is_network_error( @@ -404,15 +401,15 @@ def test_journal_not_initialized_after_approval_is_local_error( assert outcome.detail is None -def test_browser_open_false_still_polls_and_can_succeed( +def test_poll_success_enables_spl( journal_copy: Path, monkeypatch, ) -> None: _install_spl_relay(monkeypatch) outcome = spl_handoff.enable_spl_via_consent( - base_url="https://services.test", - open_browser=lambda _url: False, + base_url=TEST_BASE_URL, + nonce=TEST_NONCE, poll_once=lambda *_args, **_kwargs: portal_client.PollOutcome( kind="success", payload=_approved_payload(), @@ -423,6 +420,34 @@ def test_browser_open_false_still_polls_and_can_succeed( assert read_posture() == "spl" +def test_spl_handoff_never_invokes_global_browser_open( + journal_copy: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + browser_module = __import__("webbrowser") + monkeypatch.setattr( + browser_module, + "open", + lambda *_args, **_kwargs: pytest.fail("browser open should not be called"), + ) + _install_spl_relay(monkeypatch) + monkeypatch.setenv("SERVICES_PORTAL_URL", TEST_BASE_URL) + + consent_url, nonce, base_url = spl_handoff.build_spl_handoff_url() + result = spl_handoff.run_spl_handoff( + nonce=nonce, + base_url=base_url, + poll_once=lambda *_args, **_kwargs: portal_client.PollOutcome( + kind="success", + payload=_approved_payload(), + ), + ) + + assert result.phase == "enabled" + assert consent_url.startswith(f"{TEST_BASE_URL}/enable/spl?nonce=") + assert read_posture() == "spl" + + def test_first_enable_uses_same_persisted_instance_for_portal_and_relay( journal_copy: Path, monkeypatch: pytest.MonkeyPatch, @@ -430,23 +455,20 @@ def test_first_enable_uses_same_persisted_instance_for_portal_and_relay( state_file = journal_copy / "link" / "state.json" assert not state_file.exists() - opened_urls: list[str] = [] enroll_instance_ids: list[str] = [] monkeypatch.setenv("SOL_LINK_RELAY_URL", "https://relay.test") + monkeypatch.setenv("SERVICES_PORTAL_URL", TEST_BASE_URL) def enroll_home(_relay_url: str, **kwargs: Any) -> str: enroll_instance_ids.append(kwargs["instance_id"]) return "tok.spl" - def open_browser(url: str) -> bool: - opened_urls.append(url) - return True - monkeypatch.setattr(spl_handoff.spl, "enroll_home", enroll_home) + consent_url, nonce, base_url = spl_handoff.build_spl_handoff_url() outcome = spl_handoff.enable_spl_via_consent( - base_url="https://services.test", - open_browser=open_browser, + base_url=base_url, + nonce=nonce, poll_once=lambda *_args, **_kwargs: portal_client.PollOutcome( kind="success", payload=_approved_payload(), @@ -456,9 +478,9 @@ def test_first_enable_uses_same_persisted_instance_for_portal_and_relay( assert outcome.code == outcomes.APPROVED persisted = LinkState.load() assert persisted is not None - parsed = urllib.parse.urlparse(opened_urls[0]) - opened_instance_id = urllib.parse.parse_qs(parsed.query)["instance"][0] - assert opened_instance_id == persisted.instance_id == enroll_instance_ids[0] + parsed = urllib.parse.urlparse(consent_url) + consent_instance_id = urllib.parse.parse_qs(parsed.query)["instance"][0] + assert consent_instance_id == persisted.instance_id == enroll_instance_ids[0] def test_run_spl_handoff_sets_posture_and_service_token( @@ -472,7 +494,8 @@ def test_run_spl_handoff_sets_posture_and_service_token( ) result = spl_handoff.run_spl_handoff( - open_browser=lambda _url: True, + nonce=TEST_NONCE, + base_url=TEST_BASE_URL, poll_once=lambda *_args, **_kwargs: portal_client.PollOutcome( kind="success", payload={"service": "spl", "state": outcomes.APPROVED, "approved_at": 1}, @@ -495,7 +518,8 @@ def test_run_spl_handoff_local_error_retryable_without_enabled_state( monkeypatch.setattr(spl_handoff.spl, "enroll_home", fail_enroll) result = spl_handoff.run_spl_handoff( - open_browser=lambda _url: True, + nonce=TEST_NONCE, + base_url=TEST_BASE_URL, poll_once=lambda *_args, **_kwargs: portal_client.PollOutcome( kind="success", payload={"service": "spl", "state": outcomes.APPROVED, "approved_at": 1}, @@ -512,7 +536,8 @@ def test_run_spl_handoff_maps_needs_subscription_to_operation( journal_copy: Path, ) -> None: result = spl_handoff.run_spl_handoff( - open_browser=lambda _url: True, + nonce=TEST_NONCE, + base_url=TEST_BASE_URL, poll_once=lambda *_args, **_kwargs: portal_client.PollOutcome( kind="success", payload={ @@ -529,7 +554,12 @@ def test_run_spl_handoff_maps_needs_subscription_to_operation( operations.clear_registry() try: - operations.start_operation("spl", "spl_enable", lambda _open: result) + operations.start_operation( + "spl", + "spl_enable", + "https://services.test/enable/spl?nonce=TESTNONCE", + lambda: result, + ) _wait_until( lambda: ( operations.operation_for_service("spl")["phase"] == "needs_subscription" @@ -544,21 +574,24 @@ def test_run_spl_handoff_maps_needs_subscription_to_operation( def test_run_spl_handoff_maps_terminal_outcomes(journal_copy: Path) -> None: revoked = spl_handoff.run_spl_handoff( - open_browser=lambda _url: True, + nonce=TEST_NONCE, + base_url=TEST_BASE_URL, poll_once=lambda *_args, **_kwargs: portal_client.PollOutcome( kind="success", payload={"service": "spl", "state": outcomes.REVOKED}, ), ) expired = spl_handoff.run_spl_handoff( - open_browser=lambda _url: True, + nonce=TEST_NONCE, + base_url=TEST_BASE_URL, poll_once=lambda *_args, **_kwargs: portal_client.PollOutcome( kind="failed", reason="consent_link_expired", ), ) malformed = spl_handoff.run_spl_handoff( - open_browser=lambda _url: True, + nonce=TEST_NONCE, + base_url=TEST_BASE_URL, poll_once=lambda *_args, **_kwargs: portal_client.PollOutcome( kind="success", payload={"service": "spl", "state": "bad"}, diff --git a/tests/test_thinking_call_parity.py b/tests/test_thinking_call_parity.py index 5a2d02f9c..eb0cf3d66 100644 --- a/tests/test_thinking_call_parity.py +++ b/tests/test_thinking_call_parity.py @@ -203,7 +203,7 @@ def test_scout_enable_polls_terminal_success( def runner_result(**_kwargs): scout.provision_scout_handoff(_approved_scout_payload()) - return operations.HandoffResult("enabled", None, False, True, None) + return operations.HandoffResult("enabled", None, False) monkeypatch.setattr(scout_handoff, "run_scout_handoff", runner_result) @@ -233,8 +233,6 @@ def test_scout_enable_exits_nonzero_on_repair_needed( "error", "Try again.", True, - False, - "http://portal.test/enable/scout", ), ) @@ -273,6 +271,7 @@ def test_scout_cli_copy_mirror_matches_thinking_copy() -> None: thinking_copy.SCOUT_STATE_ENDED, thinking_copy.SCOUT_STATE_REPAIR_NEEDED, } + assert thinking_call._SCOUT_CONSENT_CTA == thinking_copy.SCOUT_CONSENT_CTA def test_keys_set_clear_validate_and_invalid_env( -- 2.51.2