diff --git a/.changepacks/changepack_log_bridge_before_login.json b/.changepacks/changepack_log_bridge_before_login.json new file mode 100644 index 0000000..77bff58 --- /dev/null +++ b/.changepacks/changepack_log_bridge_before_login.json @@ -0,0 +1,8 @@ +{ + "changes": { + "crates/devup-mcp-figma/Cargo.toml": "Minor", + "crates/devup-mcp/Cargo.toml": "Minor" + }, + "note": "The bridge is now tried before a login is demanded, and `doctor` reports it as a path. A collection asked the OAuth backend for a token before it asked anything whether a token was needed, and refused outright when there wasn't one. That inverted the two paths. The bridge spends no Figma allowance and needs no credential of any kind, so it is the path to reach for; direct is metered and a single screen costs several reads. Whoever had attached the plugin - precisely to stay off the metered path - was told to go and authorize the metered path first, and the export never started, on a file every read of which the plugin was sitting there ready to serve. After the previous change moved node-scoped metadata onto the bridge, nothing in a tsx export needs the remote path at all, so that refusal stood between an attached plugin and a finished screen for no remaining reason. `FigmaUpstream::serves_without_credentials` answers per file key rather than per process, because a plugin holding another file open says nothing about this one, and `FallbackUpstream` delegates it to the bridge. The login is now required only when no plugin is holding this file. A read inside the collection that the bridge genuinely cannot serve - a file-scope metadata read, or referencePng's get_screenshot - still refuses on its own, where the reason belongs to that read instead of condemning the whole export before it starts. Separately, `devup_figma_auth doctor` knew only about `direct`, which made the only instrument for diagnosing the connection one that could give exactly one answer - run login - to every question, including the ones whose real answer was 'run the plugin' or 'the listener never bound'. It now reports `preferredPath: bridge` and a `paths.bridge` block separating the three states that need three different repairs: not listening (the port was taken or switched off), listening with nothing attached (run the plugin, and the port is named), and attached (with the file keys, because a plugin open on the wrong file looks identical from the outside). Measured against the release binary with no token stored at all: a plugin attached over the real socket, doctor reporting available, and an export that reached the read instead of DEVUP_AUTH_REQUIRED.", + "date": "2026-09-14T16:00:00+09:00" +} diff --git a/README.md b/README.md index c5418b4..2e10051 100644 --- a/README.md +++ b/README.md @@ -147,7 +147,17 @@ stdio MCP를 지원하는 클라이언트에 다음과 같이 등록합니다. ```json { "status": "disconnected", + "preferredPath": "bridge", + "preferredPathNote": "Two paths reach Figma and they are not equals. ...", "paths": { + "bridge": { + "available": false, + "listening": true, + "port": 1993, + "attachedFiles": [], + "attachedFilesNote": "File keys the attached plugins have open. ...", + "reason": "The bridge is listening on 127.0.0.1:1993 but no plugin is attached. ..." + }, "direct": { "available": false, "credentialSource": "none", @@ -162,7 +172,15 @@ stdio MCP를 지원하는 클라이언트에 다음과 같이 등록합니다. } ``` -`doctor`는 네트워크 호출을 전혀 하지 않습니다. 세 필드는 **서로 다른 것**을 말하므로 함께 읽어야 합니다. +`doctor`는 네트워크 호출을 전혀 하지 않습니다. + +**경로는 둘이고 대등하지 않습니다.** `preferredPath`가 언제나 `bridge`인 이유입니다 — 브리지는 로그인도 필요 없고 Figma 한도도 쓰지 않습니다. `paths.bridge`의 세 상태는 고치는 방법이 서로 다르니 구분해서 읽어야 합니다. + +- `listening: false` — 이 프로세스가 브리지 포트를 아예 잡지 못했습니다. `DEVUP_FIGMA_BRIDGE_PORT`가 `off`이거나, 다른 devup-mcp가 이미 그 포트를 쥐고 있는 경우입니다(MCP 클라이언트를 여러 개 띄우면 정상입니다). 플러그인을 아무리 실행해도 이 프로세스로는 오지 않습니다. +- `listening: true`, `attachedFiles: []` — 문은 열려 있는데 아무도 들어오지 않았습니다. 대상 파일에서 `Devup Bridge` 플러그인을 실행하세요. +- `attachedFiles`에 파일 키가 있음 — 정상 동작입니다. **이 상태면 로그인 없이 그 파일의 수집이 그대로 됩니다.** + +`paths.direct`의 세 필드는 **서로 다른 것**을 말하므로 함께 읽어야 합니다. - `credentialSource` — **client 등록 자격증명**(`client_id`/`client_secret`)이 어디서 왔는지. `cli-arg`, `env`, `credential-store`, `none` 중 하나입니다. - `tokenState` — **사용자의 access token** 상태. `valid`, `expired`, `absent` 중 하나입니다. @@ -184,12 +202,22 @@ Figma MCP Catalog에 승인된 client(예: 직접 waitlist로 등록해 발급 ## Figma 연결 설정 -devup-mcp가 Figma에 붙는 경로는 하나입니다 — **원격 OAuth (`direct`)**. `devup_figma_auth { action: "login" }`으로 브라우저 인증. Figma MCP Catalog에 승인된 client만 등록할 수 있습니다. -현재 사용 가능한지는 `devup_figma_auth { action: "doctor" }`로 확인하세요. +devup-mcp가 Figma에 붙는 경로는 **둘**이고, 대등하지 않습니다. -### 브리지 플러그인 — 한도를 쓰지 않고 읽기 +| 경로 | 로그인 | Figma 한도 | 언제 쓰나 | +|---|---|---|---| +| **브리지** (`bridge`) | 필요 없음 | **쓰지 않음** | **기본.** 데스크톱 앱에서 플러그인을 띄워 두면 그쪽으로 읽습니다 | +| 직접 (`direct`) | `devup_figma_auth { action: "login" }` | 씁니다 | 브리지가 못 하는 읽기와, 플러그인을 띄울 수 없는 환경(CI 등) | -원격 OAuth 경로는 Figma가 **사용량을 셉니다.** devup-mcp의 수집은 화면 하나에 snapshot을 여러 번 부르므로 한도가 금방 바닥납니다. +**브리지를 먼저 쓰십시오.** direct는 Figma가 사용량을 세는 경로이고, 화면 하나가 여러 번의 읽기를 쓰므로 한도가 금방 바닥납니다. + +**플러그인이 이 파일을 맡고 있으면 로그인을 요구하지 않습니다.** 예전에는 수집을 시작하기 전에 토큰부터 확인해서, 한도를 아끼려고 플러그인을 띄운 사람에게 "먼저 한도 쓰는 경로를 여세요"라고 거절했습니다. 지금은 브리지를 먼저 보고, 이 파일을 맡은 플러그인이 없을 때만 로그인을 요구합니다. 수집 도중 브리지가 못 하는 읽기가 있으면 **그 읽기가** 자기 이유로 거절하므로, 무엇이 왜 막혔는지가 그대로 드러납니다. + +지금 어느 경로가 살아 있는지는 `devup_figma_auth { action: "doctor" }`의 `paths.bridge`/`paths.direct`로 확인하세요. + +현재 브리지가 **못** 하는 읽기는 둘입니다 — `scope: "file"`의 노드 없는 metadata 읽기(최상위 페이지 목록이라 계약이 다릅니다)와 `referencePng`의 `get_screenshot`. 그 밖의 tsx export 경로는 전부 브리지로 갑니다. + +### 브리지 플러그인 — 한도를 쓰지 않고 읽기 그래서 **우리가 직접 만든 Figma 플러그인**을 통해 같은 읽기를 할 수 있습니다. 이 경로는 한도를 쓰지 않습니다. 플러그인이 붙어 있으면 스크립트 읽기가 그쪽으로 가고, **안 붙어 있으면 아무 일도 일어나지 않고 그대로 원격 경로로** 갑니다. 설치하지 않은 사람의 동작은 바뀌지 않습니다. @@ -548,7 +576,7 @@ snapshot에 없는 목적지(legacy 경로, 다중 루트 요청)는 조용히 탐색과 검색은 변수 catalog를 수집하지 않습니다. 정확한 UI 변환 단계에서 선택 subtree의 모든 보존 필드에 있는 `VARIABLE_ALIAS`와 paint/text/effect/grid style ID를 재귀적으로 스캔하고, 실제 사용된 ID만 공식 Figma API로 조회합니다. `outputs: ["devupJson"]`에 `scope: "file"`을 함께 준 경우에만 file 전체 로컬 catalog를 수집합니다. -Figma 연결은 direct 하나뿐입니다. `sourcePolicy` 파라미터는 `auto`와 `direct` 둘 다 같은 동작이었으므로 제거했습니다 — 분기하지 않는 선택지는 호출자에게 틀릴 기회만 주었습니다. direct 경로는 연결과 read-only capability catalog 조회를 각각 30초, 개별 tool 호출을 5분으로 제한합니다. deadline을 넘기면 해당 remote session을 폐기하고 디자인 원문 없이 `retryable` timeout 단계만 반환합니다. +`sourcePolicy` 파라미터는 `auto`와 `direct` 둘 다 같은 동작이었으므로 제거했습니다 — 분기하지 않는 선택지는 호출자에게 틀릴 기회만 주었습니다. 경로 선택은 파라미터가 아니라 **플러그인이 붙어 있는지**가 정합니다: 붙어 있으면 브리지, 아니면 direct입니다. direct 경로는 연결과 read-only capability catalog 조회를 각각 30초, 개별 tool 호출을 5분으로 제한합니다. deadline을 넘기면 해당 remote session을 폐기하고 디자인 원문 없이 `retryable` timeout 단계만 반환합니다. 정확한 node 링크의 UI 변환은 하나 이상의 공식 `use_figma` 호출 안에서 subtree와 실제 사용 리소스를 수집합니다. 수집 스크립트는 checked-in manifest(devup-ui 변환기가 실제로 읽는 필드만)만 확인하고 — 프로토타입 체인 전체를 훑거나 미분류 필드를 `extra`에 담지 않습니다 — `null`/빈 배열/미바인딩 style ID 같은 기본값은 봉투에서 생략합니다. 결과는 항상 텍스트(`devupFastSnapshotEnvelope`)이며 PNG 같은 바이너리 transport는 없습니다. 한 subtree가 15KB 텍스트 한도를 넘으면 같은 스크립트를 `offset`을 옮겨 다시 호출하는 방식으로 텍스트 페이지네이션합니다 — 각 라운드는 그 라운드가 보낸 node에서만 리소스를 스캔해 자기 완결적이며, Rust가 여러 라운드의 node와 리소스를 병합합니다. Rust는 schema·대상 ID·node graph·리소스 참조·(페이지 중이 아닐 때의) 자식 완전성을 모두 검증한 뒤에만 결과를 채택합니다. 한 항목이라도 불일치하면 fast 결과 전체를 버리고 기존 cursor 수집을 0부터 재시작합니다. Section multi-root에서는 성공한 root와 resource는 그대로 보존하고 실패하거나 상한을 넘은 root만 legacy로 다시 수집한 뒤 원래 시각 순서로 합칩니다. direct upstream은 연결과 read-only tool catalog를 한 session에서 재사용하고 30초 TTL, 연결 종료 또는 transport 오류 때만 재연결·재검증합니다. 결과의 `stats`에는 `figmaToolCalls`, `transport`(`text` | `text-paginated` | `legacy-cursor`), `fallbackUsed`, node/variable/style 수와 byte 수만 포함되며 원본 디자인이나 인증 정보는 포함되지 않습니다. diff --git a/crates/devup-mcp-figma/src/bridge.rs b/crates/devup-mcp-figma/src/bridge.rs index 79af744..f1d63c8 100644 --- a/crates/devup-mcp-figma/src/bridge.rs +++ b/crates/devup-mcp-figma/src/bridge.rs @@ -418,15 +418,39 @@ pub trait PreferredUpstream: FigmaUpstream { fn is_live(&self) -> bool; } +/// 브리지 경로의 실측 상태. `devup_figma_auth doctor` 의 `paths.bridge` 가 된다. +/// +/// 이 값이 있다는 것 자체가 "이 프로세스가 브리지를 열었다"는 뜻이다. 포트를 +/// 잡지 못했거나 `DEVUP_FIGMA_BRIDGE_PORT=off` 면 브리지 상류가 아예 만들어지지 +/// 않으므로 `None` 이 된다. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BridgePathSnapshot { + /// 실제로 잡은 포트. 플러그인 manifest 의 `allowedDomains` 와 같아야 붙는다. + pub port: Option, + /// 지금 붙어 있는 플러그인이 열어 둔 파일 키. 빈 문자열은 자기 파일 키를 + /// 보고하지 못한 플러그인이며, 혼자 붙어 있을 때만 읽기를 받는다. + pub attached_files: Vec, +} + /// 플러그인을 통해 Figma 를 읽는 `FigmaUpstream`. #[derive(Clone)] pub struct BridgeFigmaClient { state: BridgeState, + /// 진단에만 쓴다. 읽기 경로는 포트를 알 필요가 없다. + port: Option, } impl BridgeFigmaClient { pub fn new(state: BridgeState) -> Self { - Self { state } + Self { state, port: None } + } + + /// 잡은 포트를 함께 들고 있게 한다. "문이 어디에 열려 있는가"는 붙지 않는 + /// 플러그인을 진단할 때 가장 먼저 확인할 값이다. + #[must_use] + pub fn with_port(mut self, port: u16) -> Self { + self.port = Some(port); + self } } @@ -456,6 +480,17 @@ impl FigmaUpstream for BridgeFigmaClient { fn batch_budget(&self) -> BatchBudget { bridge_batch_budget() } + + async fn serves_without_credentials(&self, file_key: &str) -> bool { + self.state.has_plugin(file_key).await + } + + async fn bridge_path_snapshot(&self) -> Option { + Some(BridgePathSnapshot { + port: self.port, + attached_files: self.state.connected_files().await, + }) + } } #[async_trait] @@ -547,4 +582,14 @@ where self.secondary.batch_budget() } } + + /// 이 파일을 맡은 플러그인이 있으면 원격 자격증명 없이도 수집이 성립한다. + /// 뒤엣것은 원격이므로 물을 것이 없다. + async fn serves_without_credentials(&self, file_key: &str) -> bool { + self.preferred.serves_without_credentials(file_key).await + } + + async fn bridge_path_snapshot(&self) -> Option { + self.preferred.bridge_path_snapshot().await + } } diff --git a/crates/devup-mcp-figma/src/lib.rs b/crates/devup-mcp-figma/src/lib.rs index 2c8b321..e4f128d 100644 --- a/crates/devup-mcp-figma/src/lib.rs +++ b/crates/devup-mcp-figma/src/lib.rs @@ -23,8 +23,8 @@ pub use collector::{ }; pub use bridge::{ - BridgeFigmaClient, BridgeJob, BridgeServer, BridgeState, DEFAULT_BRIDGE_PORT, FallbackUpstream, - PreferredUpstream, + BridgeFigmaClient, BridgeJob, BridgePathSnapshot, BridgeServer, BridgeState, + DEFAULT_BRIDGE_PORT, FallbackUpstream, PreferredUpstream, }; pub use credentials::{ ClientCredentialStore, ClientCredentials, CredentialStore, KeyringClientCredentialStore, diff --git a/crates/devup-mcp-figma/src/upstream.rs b/crates/devup-mcp-figma/src/upstream.rs index 6255220..a5b66b6 100644 --- a/crates/devup-mcp-figma/src/upstream.rs +++ b/crates/devup-mcp-figma/src/upstream.rs @@ -829,6 +829,24 @@ pub trait FigmaUpstream: Send + Sync { fn batch_budget(&self) -> BatchBudget { BatchBudget::default() } + + /// 이 파일을 Figma 자격증명 없이 읽을 수 있는지 — 즉 이 파일을 열어 둔 브리지 + /// 플러그인이 붙어 있는지. + /// + /// 로그인을 요구하기 전에 물어야 하는 값이다. 한도를 쓰지 않으려고 플러그인을 + /// 띄운 사람에게 한도를 쓰는 경로부터 열라고 시키는 것은 순서가 거꾸로다. + /// 원격만 아는 상류는 기본값 `false` 를 그대로 쓴다. + async fn serves_without_credentials(&self, _file_key: &str) -> bool { + false + } + + /// 브리지 경로의 실측 상태. 브리지를 열지 않은 상류는 `None` 이다. + /// + /// 진단 전용이다 — `devup_figma_auth doctor` 가 direct 말고도 경로가 있다는 + /// 것을 말할 수 있어야 한다. + async fn bridge_path_snapshot(&self) -> Option { + None + } } #[derive(Clone)] diff --git a/crates/devup-mcp-figma/tests/bridge_transport.rs b/crates/devup-mcp-figma/tests/bridge_transport.rs index 2045efe..1c2cc41 100644 --- a/crates/devup-mcp-figma/tests/bridge_transport.rs +++ b/crates/devup-mcp-figma/tests/bridge_transport.rs @@ -215,6 +215,23 @@ async fn routing_is_decided_before_the_call() { ); } +/// 로그인을 요구할지 말지는 이 값이 정한다. +/// +/// 브리지는 Figma 한도도 자격증명도 쓰지 않으므로, 이 파일을 맡은 플러그인이 +/// 있으면 수집은 토큰 없이 성립한다. 판정은 파일 단위여야 한다 — 다른 파일을 +/// 열어 둔 플러그인이 붙어 있다고 해서 이 파일을 읽을 수 있는 것은 아니다. +#[tokio::test] +async fn a_plugin_holding_the_file_makes_it_readable_without_credentials() { + let server = BridgeServer::start(0).expect("an ephemeral port is free"); + let client = BridgeFigmaClient::new(server.state()); + + assert!(!client.serves_without_credentials(FILE_KEY).await); + + let _plugin = connect_plugin(&server).await; + assert!(client.serves_without_credentials(FILE_KEY).await); + assert!(!client.serves_without_credentials("OtherFile").await); +} + /// 키 없이 붙은 플러그인도 혼자면 맡는다. /// /// `figma.fileKey` 는 늘 오는 값이 아니다. 실기기에서 비어 온 적이 있고, 그때 등록을 diff --git a/crates/devup-mcp/src/server/diagnostics.rs b/crates/devup-mcp/src/server/diagnostics.rs index 2e5f9e2..1de4454 100644 --- a/crates/devup-mcp/src/server/diagnostics.rs +++ b/crates/devup-mcp/src/server/diagnostics.rs @@ -1,15 +1,22 @@ -//! Self-diagnosis for the "the direct connection will not authenticate" failure mode. +//! Self-diagnosis for the "devup-mcp will not reach Figma" failure mode. //! -//! `devup-mcp` talks to Figma over the direct connection, which needs stored -//! credentials (see `oauth.rs`). Without them `devup_figma_auth status` used to -//! answer a one-line `{"status":"disconnected"}` and no next step. This module -//! turns that into structured, factual guidance: +//! Two paths reach Figma and they are not equals. The **bridge** reads through +//! a plugin in the Figma desktop app: no login, and none of the allowance +//! Figma meters. The **direct** path is remote OAuth (see `oauth.rs`) and is +//! metered, which a single screen's several reads exhaust quickly. So the +//! bridge is the path to reach for and direct is the fallback. +//! +//! This module reported only `direct`, which made it an instrument that could +//! give exactly one answer — run `devup_figma_auth login` — to every question +//! about the connection, including the ones whose real answer was "run the +//! plugin" or "the listener never bound". Naming one path made it the only +//! path, and the metered one at that. [`bridge_path`] is the other half. //! //! - [`doctor_report`] backs the `devup_figma_auth {"action":"doctor"}` -//! action and reports whether the direct connection is usable right now, plus -//! client-specific setup data for the constraints that were verified by -//! hand (client_name allowlist, redirect_uri shape, the silent callback -//! port collision, PAT rejection). +//! action and reports whether each path is usable right now, which one to +//! prefer, plus client-specific setup data for the constraints that were +//! verified by hand (client_name allowlist, redirect_uri shape, the silent +//! callback port collision, PAT rejection). //! //! All facts embedded here (allowlist behavior, redirect_uri constraints, //! the callback-port trap) were measured against the real Figma Remote MCP @@ -26,7 +33,8 @@ //! Naming it as a path sent agents to a dead end, so it is named nowhere. use devup_mcp_figma::{ - AuthStatus, ClientCredentialSource, DEFAULT_CLIENT_NAME, DirectPathSnapshot, TokenState, + AuthStatus, BridgePathSnapshot, ClientCredentialSource, DEFAULT_CLIENT_NAME, + DirectPathSnapshot, TokenState, }; use serde_json::{Value, json}; @@ -63,11 +71,18 @@ const CREDENTIAL_SOURCE_NOTE: &str = "Where the OAuth *client registration* cred /// which credential source is in play (never the secret itself), whether /// the stored token is fresh, and — when a fixed callback port is /// configured — whether it is actually free right now. -pub async fn doctor_report(status: AuthStatus, direct: DirectPathSnapshot) -> Value { +pub async fn doctor_report( + status: AuthStatus, + direct: DirectPathSnapshot, + bridge: Option, +) -> Value { let direct_available = status == AuthStatus::Connected; json!({ "status": status, + "preferredPath": "bridge", + "preferredPathNote": "Two paths reach Figma and they are not equals. The bridge plugin reads through the Figma desktop app: no login, no OAuth, and it spends none of the Figma allowance the direct path is metered against — a single screen costs several reads, so the allowance goes quickly. Reach for the bridge first and keep direct as the fallback for what the bridge cannot serve (currently a file-scope metadata read and referencePng's get_screenshot).", "paths": { + "bridge": bridge_path(bridge.as_ref()), "direct": { "available": direct_available, "credentialSource": direct.credential_source, @@ -89,6 +104,52 @@ pub async fn doctor_report(status: AuthStatus, direct: DirectPathSnapshot) -> Va }) } +/// Reports the path that costs nothing, so `doctor` stops answering a +/// connection question with "log in" and nothing else. +/// +/// This module knew only about `direct`, so every reason it could give ended +/// at `devup_figma_auth login` — including for someone whose plugin was +/// attached and serving, and including when the real problem was that the +/// listener never bound. Naming only the metered path made the metered path +/// the only answer. +/// +/// Three states, and they are genuinely different repairs. `listening: false` +/// means this process opened no bridge at all — the port was taken or it was +/// switched off — and no amount of running the plugin will help until that is +/// fixed. `listening: true` with nothing attached means the door is open and +/// nobody walked through: run the plugin on the file. Files attached is the +/// working state, and it names them, because a plugin open on the wrong file +/// looks identical from the outside. +fn bridge_path(bridge: Option<&BridgePathSnapshot>) -> Value { + let Some(bridge) = bridge else { + return json!({ + "available": false, + "listening": false, + "port": null, + "attachedFiles": [], + "reason": "This process is not listening for the bridge plugin. Either DEVUP_FIGMA_BRIDGE_PORT is off/0, or the port was already taken — another devup-mcp on this machine holds it, which is normal when several MCP clients run at once. Only that process can serve the bridge; this one can use the metered direct path only.", + }); + }; + let attached = !bridge.attached_files.is_empty(); + json!({ + // Attached is the whole test. The bridge needs no credential of any + // kind, so there is nothing else for it to be waiting on. + "available": attached, + "listening": true, + "port": bridge.port, + "attachedFiles": bridge.attached_files, + "attachedFilesNote": "File keys the attached plugins have open. An empty string is a plugin that could not report its own file key (seen in Dev Mode); it serves reads only while it is the only one attached, because with two there is no way to tell which file is meant.", + "reason": if attached { + "A plugin is attached. Reads for the files listed in attachedFiles are served through it, spending no Figma allowance and needing no login.".to_owned() + } else { + format!( + "The bridge is listening on 127.0.0.1:{} but no plugin is attached, so every read falls through to the metered direct path. Open the target file in the Figma desktop app and run the Devup Bridge plugin (Plugins -> Development -> Import plugin from manifest... once, using plugin/manifest.json). The bridge works only while that plugin window stays open. If the indicator stays grey, the port in the plugin's manifest allowedDomains and the port here must match.", + bridge.port.map_or_else(|| "".to_owned(), |port| port.to_string()), + ) + }, + }) +} + /// Says which of the two credentials is present, and never lets one of them /// stand in for the other. /// @@ -223,13 +284,71 @@ mod tests { } } + /// `doctor` used to answer every connection question with "log in", + /// because `direct` was the only path it knew. It now reports the cheaper + /// one first, and distinguishes the three states that need three different + /// repairs: no listener, a listener nobody attached to, and a working + /// plugin. Only the middle one is fixed by running the plugin, and none of + /// them is fixed by logging in. + #[tokio::test] + async fn doctor_prefers_the_bridge_and_separates_its_three_states() { + let absent = doctor_report(AuthStatus::Disconnected, absent_direct_snapshot(), None).await; + assert_eq!(absent["preferredPath"], "bridge"); + assert_eq!(absent["paths"]["bridge"]["listening"], false); + assert_eq!(absent["paths"]["bridge"]["available"], false); + let reason = absent["paths"]["bridge"]["reason"].as_str().unwrap(); + assert!(reason.contains("DEVUP_FIGMA_BRIDGE_PORT"), "{reason}"); + assert!( + !reason.contains("devup_figma_auth"), + "a bridge problem is not repaired by logging in: {reason}" + ); + + let idle = doctor_report( + AuthStatus::Disconnected, + absent_direct_snapshot(), + Some(BridgePathSnapshot { + port: Some(1993), + attached_files: vec![], + }), + ) + .await; + assert_eq!(idle["paths"]["bridge"]["listening"], true); + assert_eq!(idle["paths"]["bridge"]["available"], false); + assert_eq!(idle["paths"]["bridge"]["port"], 1993); + assert!( + idle["paths"]["bridge"]["reason"] + .as_str() + .unwrap() + .contains("1993") + ); + + // Attached is the whole test: the bridge needs no credential, so a + // disconnected direct path takes nothing away from it. + let attached = doctor_report( + AuthStatus::Disconnected, + absent_direct_snapshot(), + Some(BridgePathSnapshot { + port: Some(1993), + attached_files: vec!["FileKey123".to_owned()], + }), + ) + .await; + assert_eq!(attached["paths"]["bridge"]["available"], true); + assert_eq!( + attached["paths"]["bridge"]["attachedFiles"][0], + "FileKey123" + ); + assert_eq!(attached["status"], "disconnected"); + } + #[tokio::test] async fn doctor_report_reflects_measured_auth_status_without_changing_status_shape() { - let connected = doctor_report(AuthStatus::Connected, absent_direct_snapshot()).await; + let connected = doctor_report(AuthStatus::Connected, absent_direct_snapshot(), None).await; assert_eq!(connected["status"], "connected"); assert_eq!(connected["paths"]["direct"]["available"], true); - let disconnected = doctor_report(AuthStatus::Disconnected, absent_direct_snapshot()).await; + let disconnected = + doctor_report(AuthStatus::Disconnected, absent_direct_snapshot(), None).await; assert_eq!(disconnected["status"], "disconnected"); assert_eq!(disconnected["paths"]["direct"]["available"], false); assert!(disconnected["clientSetup"]["constraints"]["clientNameAllowlist"].is_string()); @@ -242,7 +361,7 @@ mod tests { /// the primary route. #[tokio::test] async fn client_setup_leads_with_codex_and_demotes_the_other_hosts() { - let report = doctor_report(AuthStatus::Disconnected, absent_direct_snapshot()).await; + let report = doctor_report(AuthStatus::Disconnected, absent_direct_snapshot(), None).await; let setup = &report["clientSetup"]; assert_eq!(setup["codex"]["primary"], true); @@ -266,7 +385,7 @@ mod tests { #[tokio::test] async fn doctor_report_surfaces_the_registration_client_name_and_whether_it_is_default() { let default_report = - doctor_report(AuthStatus::Disconnected, absent_direct_snapshot()).await; + doctor_report(AuthStatus::Disconnected, absent_direct_snapshot(), None).await; let default_name = &default_report["paths"]["direct"]["registrationClientName"]; assert_eq!(default_name["value"], DEFAULT_CLIENT_NAME); assert_eq!(default_name["isDefault"], true); @@ -277,6 +396,7 @@ mod tests { client_name: "Acme Registered Client".to_owned(), ..absent_direct_snapshot() }, + None, ) .await; let overridden_name = &overridden["paths"]["direct"]["registrationClientName"]; @@ -293,7 +413,7 @@ mod tests { callback_port_free: Some(false), client_name: DEFAULT_CLIENT_NAME.to_owned(), }; - let report = doctor_report(AuthStatus::Disconnected, snapshot).await; + let report = doctor_report(AuthStatus::Disconnected, snapshot, None).await; assert_eq!(report["paths"]["direct"]["credentialSource"], "cli-arg"); assert_eq!(report["paths"]["direct"]["tokenState"], "expired"); assert_eq!(report["paths"]["direct"]["callbackPort"]["port"], 19876); @@ -332,6 +452,7 @@ mod tests { token_state: devup_mcp_figma::TokenState::Valid, ..absent_direct_snapshot() }, + None, ) .await; let direct = &report["paths"]["direct"]; @@ -374,6 +495,7 @@ mod tests { token_state: devup_mcp_figma::TokenState::Absent, ..absent_direct_snapshot() }, + None, ) .await; let direct = &report["paths"]["direct"]; @@ -407,6 +529,7 @@ mod tests { token_state, ..absent_direct_snapshot() }, + None, ) .await; let reason = report["paths"]["direct"]["reason"] @@ -424,7 +547,7 @@ mod tests { /// measurement against the real registration endpoint is still published. #[tokio::test] async fn the_measured_client_setup_constraints_survive_the_reason_rewrite() { - let report = doctor_report(AuthStatus::Connected, absent_direct_snapshot()).await; + let report = doctor_report(AuthStatus::Connected, absent_direct_snapshot(), None).await; let constraints = &report["clientSetup"]["constraints"]; for key in [ "registerEndpoint", @@ -470,7 +593,7 @@ mod tests { callback_port_free: Some(true), client_name: DEFAULT_CLIENT_NAME.to_owned(), }; - let report = doctor_report(AuthStatus::Connected, snapshot).await; + let report = doctor_report(AuthStatus::Connected, snapshot, None).await; assert!(report["paths"]["direct"].get("clientSecret").is_none()); assert!(report["paths"]["direct"].get("secret").is_none()); let serialized = report.to_string(); diff --git a/crates/devup-mcp/src/server/mod.rs b/crates/devup-mcp/src/server/mod.rs index fb00cd4..48b9774 100644 --- a/crates/devup-mcp/src/server/mod.rs +++ b/crates/devup-mcp/src/server/mod.rs @@ -278,7 +278,7 @@ impl Services { // path, so opening the door costs nothing when nobody walks through. let upstream: Arc = match BridgeServer::from_env() { Some(bridge) => Arc::new(FallbackUpstream::new( - BridgeFigmaClient::new(bridge.state()), + BridgeFigmaClient::new(bridge.state()).with_port(bridge.port()), remote, )), None => Arc::new(remote), @@ -413,14 +413,40 @@ impl DevupServer { scope, )); } - let auth_status = self.services.auth.status().await?; - if auth_status == AuthStatus::Disconnected { - return Err(DevupError::with_details( - ErrorCode::DevupAuthRequired, - "Using the Figma direct connection requires devup_figma_auth login.", - false, - json!({"source": "direct"}), - )); + // The bridge is asked first, because it is the path that spends no + // allowance and needs no login at all. Demanding the token before + // looking at it inverted that: someone who had attached the plugin + // precisely to stay off the metered path was told to go and authorize + // the metered path first, and the collection never started — on a file + // every read of which the plugin was sitting there ready to serve. + // + // A login is required only when no plugin is holding *this* file open. + // A read inside the collection that the bridge cannot serve still + // refuses on its own, where the reason is specific to that read, + // rather than here where it would condemn the whole export. + if !self + .services + .upstream + .serves_without_credentials(&request.target.file_key) + .await + { + let auth_status = self.services.auth.status().await?; + if auth_status == AuthStatus::Disconnected { + return Err(DevupError::with_details( + ErrorCode::DevupAuthRequired, + "No Figma path is open for this file. Preferred: run the Devup Bridge plugin on \ + this file in the Figma desktop app — it needs no login and spends no Figma \ + allowance. Otherwise authorize the metered direct connection with \ + devup_figma_auth login.", + false, + json!({ + "source": "direct", + "bridgeServesFile": false, + "preferredPath": "bridge", + "fileKey": request.target.file_key, + }), + )); + } } // Each tracked job owns its collector. Artifact single-flight would @@ -898,8 +924,9 @@ impl DevupServer { .direct_path_snapshot() .await .map_err(to_mcp_error)?; + let bridge = self.services.upstream.bridge_path_snapshot().await; return Ok(tool_result( - diagnostics::doctor_report(status, direct).await, + diagnostics::doctor_report(status, direct, bridge).await, )); } if input.action == "configure" { diff --git a/crates/devup-mcp/tests/source_orchestration.rs b/crates/devup-mcp/tests/source_orchestration.rs index 3e00efd..f4c4074 100644 --- a/crates/devup-mcp/tests/source_orchestration.rs +++ b/crates/devup-mcp/tests/source_orchestration.rs @@ -116,6 +116,29 @@ impl FigmaUpstream for FixtureUpstream { } } +/// A bridge plugin holding this file open, answering the same collection. +/// The one thing that separates it from `FixtureUpstream` is what it says +/// before the first read: this file needs no Figma credential. +#[derive(Default)] +struct BridgedFixture { + inner: FixtureUpstream, +} + +#[async_trait] +impl FigmaUpstream for BridgedFixture { + async fn list_tools(&self) -> Result, DevupError> { + self.inner.list_tools().await + } + + async fn call_read_tool(&self, call: ReadToolCall) -> Result { + self.inner.call_read_tool(call).await + } + + async fn serves_without_credentials(&self, _file_key: &str) -> bool { + true + } +} + async fn call_tool( auth: Arc, upstream: Arc, @@ -444,11 +467,41 @@ async fn auto_asks_to_be_logged_in_rather_than_starting_oauth() -> anyhow::Resul error.to_string().contains("devup_figma_auth login"), "the error should name the action that fixes it: {error}" ); + // Logging in is the fallback, not the first thing to reach for: it is the + // path Figma meters. The refusal has to say so, or the reader fixes it the + // expensive way every time. + assert!( + error.to_string().contains("Devup Bridge"), + "the cheaper path has to be offered first: {error}" + ); assert_eq!(auth.logins.load(Ordering::SeqCst), 0); assert_eq!(upstream.calls.load(Ordering::SeqCst), 0); Ok(()) } +/// The bridge spends no Figma allowance and needs no login, so it is the path +/// to try first. This was decided the other way round: the token was demanded +/// before anything looked at the plugin, so whoever had attached one — exactly +/// to stay off the metered path — was told to go and open the metered path, +/// and the export never started on a file whose every read the plugin was +/// sitting there ready to serve. A plugin holding the file is now enough. +#[tokio::test] +async fn an_attached_bridge_collects_without_demanding_a_direct_login() -> anyhow::Result<()> { + let auth = Arc::new(AuthProbe { + status: AuthStatus::Disconnected, + logins: AtomicUsize::new(0), + }); + let upstream = Arc::new(BridgedFixture::default()); + let result = call_tool(auth.clone(), upstream.clone(), input()).await?; + let output = result.structured_content.unwrap(); + + assert!(output["tsx"].as_str().unwrap().contains("SyntheticFrame")); + assert_eq!(upstream.inner.calls.load(Ordering::SeqCst), 3); + // No browser and no token: the collection never needed either. + assert_eq!(auth.logins.load(Ordering::SeqCst), 0); + Ok(()) +} + /// Every refusal now surfaces as itself. A capability that is missing says so /// at once; a spent allowance is waited out three times first, because a /// collection can cross a per-minute line partway through its own burst.