From 1cd3b59f7d3ccac21bd25db10e8a0ede6ca4712e Mon Sep 17 00:00:00 2001 From: Ken Simpson Date: Mon, 31 Aug 2026 16:05:48 -0700 Subject: [PATCH] Keep Subrosa authentication state consistent --- README.md | 2 +- docs/LEARNINGS.md | 8 +++++ modal_ui_events.py | 3 ++ routes_queue.py | 13 ++++--- subrosa_login.py | 4 +-- tests/test_routes_modal_containers.py | 2 ++ tests/test_routes_queue.py | 6 +++- tests/test_subrosa_login.py | 4 +-- tests/test_subrosa_provider.py | 4 +-- tests/test_web_modal_toggle.py | 15 ++++++++ tests/test_web_subrosa_login.py | 9 ++++- web/modal_toggle.js | 35 +++++++++++++++++++ web/subrosa_login.js | 49 ++++++++++++++++++++++++--- 13 files changed, 136 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 9b12f17..5aee559 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ The connected graph is authoritative. Connecting only Modal configurations produ Capacity limits are concurrency ceilings, not eager provisioning requests. Modal containers scale as work needs them, Vast instances are quoted during planning and rented only for selected slots, SSH worker containers are prepared lazily on the declared host, and Subrosa jobs wait for workers in the configured relay pool. Sequential components can reuse one slot; parallel components consume distinct slots or wait when that is cheaper than another environment. -Subrosa support currently targets the relay mock-worker milestone. A **Subrosa Configuration** declares a `ws://`/`wss://` relay base URL, pool, concurrency limit, and advanced `credential_id` widget. The keyring reference defaults to the stable, human-readable `subrosa-default`, so one saved token can serve every workflow; changing it intentionally selects a different local token. Click **Login to Subrosa** to open the trusted relay portal, authenticate with a passkey or recovery code, mint a new `srk_` extension token, validate it locally, and save it in the operating-system keyring. Login and token validation accept HTTPS `subrosa.red` services (plus loopback development servers), preventing a workflow-supplied relay from receiving a saved service token. The token is never stored in the workflow or queue payload. The button checks saved-token validity through `whoami`; queue interception repeats that check before prompt rewriting or ComfyUI node execution, and the relay client retains a final defensive check before opening a billable job. A missing or rejected token stops immediately, highlights the responsible **Subrosa Configuration** node in red, and shows a concise instruction to click Login again instead of attributing the failure to a downstream workflow node. The scheduler advertises pool-specific synthetic hardware: `mock-4090` has 24 GiB VRAM and `mock-B300` has 288 GiB VRAM, while unknown pools conservatively fall back to 24 GiB. Only the B300 VRAM figure is authoritative; its CPU count and host RAM are plausible planning values, and memory bandwidth and bus width are not represented by the scheduler capability model. The client waits for a claimed worker, transports the existing CRMTRPC1 request/input stream in 512 KiB lane-0 messages, streams progress back to the canvas, reports relay settlement and centicredits, and sends cancellation over lane 1 on the same socket. Model, LoRA, and custom-node synchronization is deliberately disabled for this milestone and must not be relied on until the relay-managed R2 backend lands. +Subrosa support currently targets the relay mock-worker milestone. A **Subrosa Configuration** declares a `ws://`/`wss://` relay base URL, pool, concurrency limit, and advanced `credential_id` widget. The keyring reference defaults to the stable, human-readable `subrosa-default`, so one saved token can serve every workflow; changing it intentionally selects a different local token. Use **Click to Authenticate** to open the trusted relay portal, authenticate with a passkey or recovery code, mint a new `srk_` extension token, validate it locally, and save it in the operating-system keyring. Login and token validation accept HTTPS `subrosa.red` services (plus loopback development servers), preventing a workflow-supplied relay from receiving a saved service token. The token is never stored in the workflow or queue payload. The frontend explicitly materializes the effective `credential_id` in the queued prompt even when ComfyUI omits a default-valued advanced widget, ensuring the node status and execution preflight validate the same keyring entry. The button checks saved-token validity through `whoami`; queue interception repeats that check before prompt rewriting or ComfyUI node execution, and the relay client retains a final defensive check before opening a billable job. A missing or rejected token durably changes the responsible node's button to **Click to Authenticate**, highlights that node in red, and shows the same concise authentication instruction instead of attributing the failure to a downstream workflow node. The scheduler advertises pool-specific synthetic hardware: `mock-4090` has 24 GiB VRAM and `mock-B300` has 288 GiB VRAM, while unknown pools conservatively fall back to 24 GiB. Only the B300 VRAM figure is authoritative; its CPU count and host RAM are plausible planning values, and memory bandwidth and bus width are not represented by the scheduler capability model. The client waits for a claimed worker, transports the existing CRMTRPC1 request/input stream in 512 KiB lane-0 messages, streams progress back to the canvas, reports relay settlement and centicredits, and sends cancellation over lane 1 on the same socket. Model, LoRA, and custom-node synchronization is deliberately disabled for this milestone and must not be relied on until the relay-managed R2 backend lands. Login writes the token from the local ComfyUI backend, so macOS must have an unlocked GUI login keychain. The following remains available as a manual diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index 6df5da0..662ad4c 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -785,6 +785,14 @@ request. Run this preflight before prompt rewriting and before handing the prompt to ComfyUI; a configuration node's ordinary execution order is not an adequate barrier for independent graph branches. +An advanced widget's visible default is not guaranteed to appear in ComfyUI's +serialized prompt. If browser status uses the live widget while backend +execution applies an omitted-field compatibility fallback, they can validate +different keyring entries and report contradictory states. Materialize the +effective non-secret reference before submission, and feed queue-time auth +failures back into the provider node's own state. A stale asynchronous status +response must not overwrite a newer authentication-required event. + ## Compatibility imports can hide an unfinished extraction Moving behavior and mutable state out of a stable entrypoint does not make the entrypoint thin if it still eagerly imports every extracted private helper. Import only the hooks and operations used by production glue, then resolve legacy read-only private access through the focused owner modules. This preserves a migration surface without creating hundreds of stale bindings or obscuring which module actually owns execution. diff --git a/modal_ui_events.py b/modal_ui_events.py index 22a6099..3905800 100644 --- a/modal_ui_events.py +++ b/modal_ui_events.py @@ -31,6 +31,7 @@ def _emit_modal_status( active_node_class_type: str | None = None, active_node_role: str | None = None, error_message: str | None = None, + error_code: str | None = None, status_message: str | None = None, status_current: int | None = None, status_total: int | None = None, @@ -73,6 +74,8 @@ def _emit_modal_status( payload["active_node_role"] = active_node_role if error_message is not None: payload["error_message"] = error_message + if error_code is not None: + payload["error_code"] = error_code if status_message is not None: payload["status_message"] = status_message if status_current is not None: diff --git a/routes_queue.py b/routes_queue.py index fcb6050..421012f 100644 --- a/routes_queue.py +++ b/routes_queue.py @@ -408,6 +408,7 @@ def _queue_error_response( configurator_node_id=state.configurator_node_id, modal_gpu=state.request_modal_gpu, failed_node_id=failed_node_id or None, + error_code=getattr(exc, "code", None), **{key: value for key, value in kwargs.items() if value is not None}, ) node_errors = _queue_node_errors(state, exc, failed_node_id) @@ -415,8 +416,8 @@ def _queue_error_response( if failed_node_id: error = { "type": getattr(exc, "code", None) or "provider_configuration_invalid", - "message": "Subrosa Configuration failed validation", - "details": str(exc), + "message": str(exc), + "details": "", "extra_info": {"node_id": failed_node_id}, } return web.json_response( @@ -441,7 +442,11 @@ def _queue_node_errors( class_type = str(prompt_node.get("class_type") or "SubrosaRemoteConfiguration") reason = { "type": getattr(exc, "code", None) or "provider_configuration_invalid", - "message": "Subrosa Configuration failed validation", + "message": ( + "Subrosa authentication required" + if getattr(exc, "code", None) == "subrosa_login_required" + else "Subrosa Configuration failed validation" + ), "details": str(exc), "extra_info": {}, } @@ -499,7 +504,7 @@ async def _handle_modal_queue_prompt( prompt_server, ctx, state, exc, phase="error", status=400 ) except SubrosaConfigurationValidationError as exc: - logger.info( + logger.debug( "Subrosa configuration credential preflight failed node_id=%s: %s", exc.configuration_id, exc, diff --git a/subrosa_login.py b/subrosa_login.py index dc76bff..8fd3eef 100644 --- a/subrosa_login.py +++ b/subrosa_login.py @@ -39,8 +39,8 @@ SUBROSA_CREDENTIAL_IMPORT_ROUTE = "/remote/subrosa/credentials" SUBROSA_CREDENTIAL_STATUS_ROUTE = "/remote/subrosa/status" SUBROSA_LOGIN_REQUIRED_CODE = "subrosa_login_required" SUBROSA_LOGIN_REQUIRED_MESSAGE = ( - 'Subrosa login is missing or no longer valid. Click "Login to Subrosa" on ' - "the Subrosa Configuration node, then queue the workflow again." + 'Subrosa authentication required. Click the Subrosa Configuration node\'s ' + '"Click to Authenticate" button, then queue the workflow again.' ) _USER_AGENT = ( "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) " diff --git a/tests/test_routes_modal_containers.py b/tests/test_routes_modal_containers.py index 4be3287..a7511c4 100644 --- a/tests/test_routes_modal_containers.py +++ b/tests/test_routes_modal_containers.py @@ -162,10 +162,12 @@ def test_modal_status_preserves_attributed_failure_node( prompt_id="prompt-1", node_ids=["7"], failed_node_id="42", + error_code="subrosa_login_required", error_message="Click Login again.", ) assert prompt_server.messages[0][1]["failed_node_id"] == "42" + assert prompt_server.messages[0][1]["error_code"] == "subrosa_login_required" def test_modal_telemetry_replay_coalesces_each_execution_source( diff --git a/tests/test_routes_queue.py b/tests/test_routes_queue.py index c881dcb..1d362b7 100644 --- a/tests/test_routes_queue.py +++ b/tests/test_routes_queue.py @@ -41,13 +41,16 @@ def test_attributed_queue_error_targets_subrosa_configuration_node( payload = json.loads(response.text) assert payload["error"]["type"] == "subrosa_login_required" + assert payload["error"]["message"] == ( + subrosa_login_module.SUBROSA_LOGIN_REQUIRED_MESSAGE + ) assert payload["error"]["extra_info"]["node_id"] == "42" assert payload["node_errors"] == { "42": { "errors": [ { "type": "subrosa_login_required", - "message": "Subrosa Configuration failed validation", + "message": "Subrosa authentication required", "details": subrosa_login_module.SUBROSA_LOGIN_REQUIRED_MESSAGE, "extra_info": {}, } @@ -57,6 +60,7 @@ def test_attributed_queue_error_targets_subrosa_configuration_node( } } assert emitted[0]["failed_node_id"] == "42" + assert emitted[0]["error_code"] == "subrosa_login_required" def test_invalid_subrosa_token_stops_before_rewrite_and_comfyui_execution( diff --git a/tests/test_subrosa_login.py b/tests/test_subrosa_login.py index 204efff..3c636a0 100644 --- a/tests/test_subrosa_login.py +++ b/tests/test_subrosa_login.py @@ -120,7 +120,7 @@ def test_rejected_token_uses_concise_login_again_error( with pytest.raises( subrosa_login_module.SubrosaLoginRequiredError, - match="Click.*Login to Subrosa", + match="Click.*Authenticate", ): asyncio.run( subrosa_login_module.validate_subrosa_token( @@ -171,7 +171,7 @@ def test_queue_preflight_attributes_rejected_token_to_subrosa_node( with pytest.raises( subrosa_login_module.SubrosaConfigurationValidationError, - match="Click.*Login to Subrosa", + match="Click.*Authenticate", ) as exc_info: asyncio.run( subrosa_login_module.preflight_subrosa_configurations( diff --git a/tests/test_subrosa_provider.py b/tests/test_subrosa_provider.py index a52dae3..30a1d2b 100644 --- a/tests/test_subrosa_provider.py +++ b/tests/test_subrosa_provider.py @@ -198,7 +198,7 @@ def test_missing_workflow_token_instructs_user_to_click_login( with pytest.raises( subrosa_login_module.SubrosaLoginRequiredError, - match="Click.*Login to Subrosa", + match="Click.*Authenticate", ): subrosa_login_module.require_saved_subrosa_token( store, @@ -742,7 +742,7 @@ def test_run_relay_rejects_invalid_token_before_opening_job_socket( with pytest.raises( subrosa_login_module.SubrosaLoginRequiredError, - match="Click.*Login to Subrosa", + match="Click.*Authenticate", ): asyncio.run( client._run_relay( diff --git a/tests/test_web_modal_toggle.py b/tests/test_web_modal_toggle.py index ba9d579..722500a 100644 --- a/tests/test_web_modal_toggle.py +++ b/tests/test_web_modal_toggle.py @@ -56,6 +56,21 @@ def test_queue_failure_highlights_attributed_configuration_node() -> None: assert "setNodesPhase([failedNodeId], STATE_ERROR" in source +def test_queue_materializes_subrosa_widget_credential_reference() -> None: + """Default-valued widgets must use the same credential ID as status checks.""" + source = _modal_toggle_source() + + function_start = source.index("function materializeSubrosaCredentialIds(prompt)") + queue_start = source.index("api.queuePrompt = async function modalQueuePrompt") + materialize_call = source.index("materializeSubrosaCredentialIds(prompt);", queue_start) + + assert function_start < queue_start < materialize_call + assert 'candidate?.name === "credential_id"' in source[function_start:queue_start] + assert "widgetValue || serializedValue || String(serializedNodeId)" in source + assert 'window.addEventListener("subrosa-authentication-status"' in source + assert "clearAttributedNodeFailure(event.detail.node_id)" in source + + def test_global_modal_status_badge_is_installed() -> None: """The frontend should expose a dedicated global Modal activity indicator.""" source = _modal_toggle_source() diff --git a/tests/test_web_subrosa_login.py b/tests/test_web_subrosa_login.py index 5ca65a0..8eb5f98 100644 --- a/tests/test_web_subrosa_login.py +++ b/tests/test_web_subrosa_login.py @@ -12,7 +12,7 @@ def test_subrosa_login_is_direct_nonce_bound_and_non_serialized() -> None: ) assert '"SubrosaRemoteConfiguration"' in source - assert '"Login to Subrosa"' in source + assert '"Click to Authenticate"' in source assert 'serialize: false' in source assert 'window.open(' in source assert '"about:blank"' in source @@ -20,6 +20,13 @@ def test_subrosa_login_is_direct_nonce_bound_and_non_serialized() -> None: assert "globalThis.crypto.randomUUID()" in source assert "event.origin !== pending.origin" in source assert "event.source !== pending.popup" in source + assert 'api.addEventListener("modal_status", handleModalStatus)' in source + assert 'detail.error_code !== LOGIN_REQUIRED_CODE' in source + assert "requireAuthentication(node)" in source + assert '"subrosa-authentication-status"' in source + assert "markAuthenticated(node)" in source + assert "statusRevisions.get(node) !== revision" in source + assert '|| String(node?.id ?? "")' in source assert 'hostname.endsWith(".subrosa.red")' in source assert "/remote/subrosa/credentials" in source assert "/remote/subrosa/status" in source diff --git a/web/modal_toggle.js b/web/modal_toggle.js index 13a6a74..92f7015 100644 --- a/web/modal_toggle.js +++ b/web/modal_toggle.js @@ -3437,6 +3437,22 @@ function serializedRemoteConfiguratorNodeId(prompt) { return matches.length === 1 ? String(matches[0][0]) : null; } +/** Materialize the effective keyring reference omitted for default-valued widgets. */ +function materializeSubrosaCredentialIds(prompt) { + for (const [serializedNodeId, promptNode] of Object.entries(prompt ?? {})) { + if (promptNode?.class_type !== "SubrosaRemoteConfiguration") continue; + const liveNode = allWorkflowNodes().find( + (candidate) => nodeId(candidate) === String(serializedNodeId), + ); + const widgetValue = String( + (liveNode?.widgets ?? []).find((candidate) => candidate?.name === "credential_id")?.value ?? "", + ).trim(); + const serializedValue = String(promptNode?.inputs?.credential_id ?? "").trim(); + promptNode.inputs ||= {}; + promptNode.inputs.credential_id = widgetValue || serializedValue || String(serializedNodeId); + } +} + /** * Resolve and lazily mount the configurator panel for one serialized node id. * @param {string | null | undefined} configuratorNodeId @@ -6991,6 +7007,19 @@ function markQueueFailure(remoteNodeIds, promptId, error) { setNodesPhase(remoteNodeIds, STATE_ERROR, promptId, queueErrorMessage(error)); } +/** Clear a stale provider-node failure after fresh authentication succeeds. */ +function clearAttributedNodeFailure(nodeIdValue) { + const safeNodeId = String(nodeIdValue ?? ""); + const state = modalNodeStates.get(safeNodeId); + if (!safeNodeId || state?.phase !== STATE_ERROR) return; + clearNodeTimer(safeNodeId); + clearNodeProgress(safeNodeId, state.promptId); + clearNodeCached(safeNodeId, state.promptId); + modalNodeStates.delete(safeNodeId); + stopAnimationLoopIfIdle(); + refreshNodeDecorations(); +} + /** * Preserve an actionable server queue failure when PromptExecutionError is generic. * @param {Error | any} error @@ -7171,6 +7200,11 @@ function registerExecutionListeners() { } if (typeof window !== "undefined") { window.addEventListener("focus", refreshModalUiAfterVisibilityChange); + window.addEventListener("subrosa-authentication-status", (event) => { + if (event?.detail?.connected === true) { + clearAttributedNodeFailure(event.detail.node_id); + } + }); } api.__modalExecutionListenersRegistered = true; } @@ -7185,6 +7219,7 @@ function patchQueuePrompt() { api.queuePrompt = async function modalQueuePrompt(number, data, options) { const { output: prompt, workflow } = data; + materializeSubrosaCredentialIds(prompt); stampModalGpuOnWorkflow(workflow); const modalGpu = selectedModalGpu(workflow); const configuratorNodeId = serializedRemoteConfiguratorNodeId(prompt); diff --git a/web/subrosa_login.js b/web/subrosa_login.js index 952d436..bace94b 100644 --- a/web/subrosa_login.js +++ b/web/subrosa_login.js @@ -5,7 +5,11 @@ const SUBROSA_NODE_ID = "SubrosaRemoteConfiguration"; const CREDENTIAL_IMPORT_ROUTE = "/remote/subrosa/credentials"; const CREDENTIAL_STATUS_ROUTE = "/remote/subrosa/status"; const LOGIN_MESSAGE_TYPE = "subrosa-comfyui-login"; +const LOGIN_REQUIRED_CODE = "subrosa_login_required"; +const AUTHENTICATION_STATUS_EVENT = "subrosa-authentication-status"; +const AUTHENTICATE_LABEL = "Click to Authenticate"; const pendingLogins = new Map(); +const statusRevisions = new WeakMap(); /** Return JSON or raise one credential-safe local diagnostic. */ async function requestJson(route, options = {}) { @@ -24,7 +28,7 @@ function widget(node, name) { /** Return the workflow's non-secret keyring reference. */ function credentialId(node) { - return String(widget(node, "credential_id")?.value ?? "").trim(); + return String(widget(node, "credential_id")?.value ?? "").trim() || String(node?.id ?? ""); } /** Return the configured relay URL. */ @@ -54,6 +58,28 @@ function setLoginState(node, label) { app.graph?.setDirtyCanvas?.(true, true); } +/** Invalidate older status requests and return the new node-local revision. */ +function nextStatusRevision(node) { + const revision = (statusRevisions.get(node) ?? 0) + 1; + statusRevisions.set(node, revision); + return revision; +} + +/** Keep authentication-required state visible until a newer whoami succeeds. */ +function requireAuthentication(node) { + nextStatusRevision(node); + setLoginState(node, AUTHENTICATE_LABEL); +} + +/** Publish a verified connection and clear any older queue-failure decoration. */ +function markAuthenticated(node) { + nextStatusRevision(node); + setLoginState(node, "Subrosa: Connected"); + window.dispatchEvent(new CustomEvent(AUTHENTICATION_STATUS_EVENT, { + detail: { node_id: String(node.id), connected: true }, + })); +} + /** Resolve one live Subrosa node by graph ID. */ function subrosaNodeById(nodeId) { return (app.graph?._nodes ?? []).find( @@ -63,6 +89,7 @@ function subrosaNodeById(nodeId) { /** Ask the local backend whether the saved token still authenticates. */ async function refreshLoginStatus(node) { + const revision = nextStatusRevision(node); const id = credentialId(node); if (!id) { setLoginState(node, "Subrosa: credential_id required"); @@ -72,8 +99,11 @@ async function refreshLoginStatus(node) { try { const route = `${CREDENTIAL_STATUS_ROUTE}?credential_id=${encodeURIComponent(id)}&relay_url=${encodeURIComponent(relayUrl(node))}`; const status = await requestJson(route); - setLoginState(node, status.connected ? "Subrosa: Connected" : "Login to Subrosa"); + if (statusRevisions.get(node) !== revision) return; + if (status.connected) markAuthenticated(node); + else requireAuthentication(node); } catch (error) { + if (statusRevisions.get(node) !== revision) return; setLoginState(node, `Subrosa: ${String(error?.message ?? error)}`); } } @@ -131,9 +161,9 @@ async function handleLoginResult(event) { try { setLoginState(node, "Subrosa: Validating…"); await savePortalToken(node, token); - setLoginState(node, "Subrosa: Connected"); + markAuthenticated(node); } catch (error) { - setLoginState(node, `Subrosa: ${String(error?.message ?? error)}`); + requireAuthentication(node); } finally { token = ""; } @@ -144,7 +174,7 @@ function decorateSubrosaNode(node) { if (node?.comfyClass !== SUBROSA_NODE_ID || node.__subrosaLoginWidget) return; const loginWidget = node.addWidget( "button", - "Login to Subrosa", + AUTHENTICATE_LABEL, null, () => startSubrosaLogin(node), { serialize: false }, @@ -154,11 +184,20 @@ function decorateSubrosaNode(node) { void refreshLoginStatus(node); } +/** Apply a queue-time authentication failure to the exact Subrosa node. */ +function handleModalStatus(event) { + const detail = event?.detail ?? event ?? {}; + if (detail.phase !== "error" || detail.error_code !== LOGIN_REQUIRED_CODE) return; + const node = subrosaNodeById(detail.failed_node_id); + if (node) requireAuthentication(node); +} + app.registerExtension({ name: "Comfy.RemoteExecution.SubrosaLogin", async init() { window.addEventListener("message", (event) => void handleLoginResult(event)); + api.addEventListener("modal_status", handleModalStatus); }, async nodeCreated(node) {