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(