diff --git a/.changepacks/changepack_log_narrow_the_surface.json b/.changepacks/changepack_log_narrow_the_surface.json new file mode 100644 index 0000000..b65e95c --- /dev/null +++ b/.changepacks/changepack_log_narrow_the_surface.json @@ -0,0 +1,10 @@ +{ + "changes": { + "crates/devup-mcp/Cargo.toml": "Minor", + "crates/devup-mcp-figma/Cargo.toml": "Minor", + "crates/devup-mcp-devup-ui/Cargo.toml": "Minor", + "crates/devup-mcp-visual/Cargo.toml": "Patch" + }, + "note": "Narrow what devup-mcp asks for and what it sends back, and make a refusal say which of the two kinds it is. Nine tools become seven: devup_figma_to_ui and devup_figma_to_json were devup_figma_export with a single entry in outputs, so every client carried three schemas in its context and had to decide between them on every call, and the export tool's own description had to spend a sentence saying which to prefer. Use outputs: [tsx] and outputs: [devupJson] instead. sourcePolicy is gone from all of them: auto and direct both meant the direct connection and the parameter never branched, so it only ever offered a caller something to get wrong, and it was also part of the artifact cache key it could not affect. Every remaining closed-set input now publishes its accepted values in the JSON schema - action, scope, rootLayout, delivery, match, project context scope and stack diff layers - from one shared constant the parser reads too, so the schema cannot drift from what is accepted and a caller stops discovering the set one rejection at a time; outputs and the asset format were the only two that already did this. Errors are no longer all INTERNAL_ERROR. A mistake in the call itself - an unknown scope, a node that is not in the file, an expired artifactId - is now JSON-RPC INVALID_PARAMS, and everything behind the call stays INTERNAL_ERROR, so an agent can tell 'fix the arguments and retry' from 'stop and report' without parsing the message; the exact code and retryable are unchanged in data. Pure argument validation that had been reported as DEVUP_THEME_CONFLICT or DEVUP_SNAPSHOT_UNSUPPORTED is DEVUP_INVALID_INPUT, so a real theme conflict is no longer confused with a typo. The response is lighter for the same content. Measured on one export, a tsx-only call went from 2,411 to 1,405 bytes and the part every response carries regardless from 1,911 to 994. fidelity and completenessReport are the drill-down beneath quality and on a clean result restate it - 100% across six axes, six empty arrays - so they are sent when the result is not exact or complete, or when includeDiagnostics asks for them; on a Section export that was per screen. imports, usedTokens, componentImports, responsiveImports and responsiveComponents restated the tsx's own import line and its $tokens and are gone. deliverable stays: it was removed on the reasoning that the needs_figma handoff it guarded against is gone, and a consumer reported relying on it to know which value is the answer, which settles it. The server instructions were the larger cost and are corrected: they told every agent to take tsx, rawSnapshot and sourceMap together, which on the same measurement is about eight times the bytes of the code, and they now say to ask for an output only when it will be read. completeness and themeCompleteness stay - they grade how far token resolution reached, which quality does not say. Releases stop shipping devup-mcp-visual: the render harness builds it from source with cargo and nothing downloads it, so three of six assets were binaries no consumer used, built on every platform of every release. Cargo.lock is committed at the released version, which stops every release binary reporting its build id as -dirty and losing the ability to tell a release from a developer's working tree. Also removes two ErrorCode variants no production code ever constructed, whose only reference was a test pinning their wire strings - a test that would have passed forever whether or not they were reachable. A binding the resource catalog never named is now reported rather than quietly resolved. A fill bound to a variable, or a text carrying a style, is the design saying this is a token; the generator writes the token when the catalog carried that resource and the resolved value when it did not, and it has to write something because the module still has to compile. It said nothing when it did, so a hardcoded #7d7f83 could sit where the design says - identical today, no longer following the theme tomorrow - inside a response graded exact. Each such binding now raises DEVUP_CODEGEN_TOKEN_NAME_UNRESOLVED carrying the node, the property and the resource id, which also stops quality.projection reading exact. The check runs as a pass over the collected subtree rather than inside rendering, so it can name the node without threading an argument through the render functions, and the 268 plugin-parity goldens are unaffected because the generated bytes do not change.", + "date": "2026-09-08T15:40:00+09:00" +} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 94ee19d..9c8d289 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -194,26 +194,27 @@ jobs: set -euo pipefail for target in $TARGETS; do rustup target add "$target" - cargo build --release --target "$target" -p devup-mcp -p devup-mcp-visual + cargo build --release --target "$target" -p devup-mcp done + # Only the MCP server ships. devup-mcp-visual is a PNG comparator + # the render harness builds from source with cargo; nothing + # downloads it, so three of the six release assets were binaries no + # consumer had a use for while still costing a build on every + # platform of every release. mkdir -p dist - for bin in devup-mcp devup-mcp-visual; do - out="dist/${bin}-${SUFFIX}${EXT}" - if [ "$OS" = "macos-latest" ]; then - lipo -create -output "$out" \ - "target/aarch64-apple-darwin/release/${bin}" \ - "target/x86_64-apple-darwin/release/${bin}" - file "$out" - else - set -- $TARGETS - cp "target/$1/release/${bin}${EXT}" "$out" - fi - done + out="dist/devup-mcp-${SUFFIX}${EXT}" + if [ "$OS" = "macos-latest" ]; then + lipo -create -output "$out" \ + "target/aarch64-apple-darwin/release/devup-mcp" \ + "target/x86_64-apple-darwin/release/devup-mcp" + file "$out" + else + set -- $TARGETS + cp "target/$1/release/devup-mcp${EXT}" "$out" + fi ls -l dist # `bundle` needs all three platforms' server binaries in one place, and - # each is produced on a different runner. Only the MCP server itself is - # carried: devup-mcp-visual is a comparison CLI a consumer repository - # runs in its own CI, not something an MCP host ever launches. + # each is produced on a different runner. - name: Hand the server binary to the bundle job uses: actions/upload-artifact@v7 with: diff --git a/Cargo.lock b/Cargo.lock index 6abc7e8..418bf70 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -684,7 +684,7 @@ dependencies = [ [[package]] name = "devup-mcp" -version = "0.1.0" +version = "0.2.1" dependencies = [ "anyhow", "async-trait", @@ -707,7 +707,7 @@ dependencies = [ [[package]] name = "devup-mcp-devup-ui" -version = "0.1.0" +version = "0.2.1" dependencies = [ "devup-mcp-figma", "insta", @@ -724,7 +724,7 @@ dependencies = [ [[package]] name = "devup-mcp-figma" -version = "0.1.0" +version = "0.2.1" dependencies = [ "anyhow", "async-trait", @@ -749,7 +749,7 @@ dependencies = [ [[package]] name = "devup-mcp-visual" -version = "0.1.0" +version = "0.2.1" dependencies = [ "anyhow", "image", diff --git a/README.md b/README.md index a59a6ac..082638b 100644 --- a/README.md +++ b/README.md @@ -4,15 +4,37 @@ Rust-native MCP server that reads Figma designs and generates DevupUI artifacts. 저장소는 Cargo workspace이며 `devup-mcp` 실행 crate, OAuth·upstream·snapshot을 담당하는 `devup-mcp-figma`, TSX·theme projection을 담당하는 `devup-mcp-devup-ui`, PNG 비교 library/CLI인 `devup-mcp-visual`로 구성됩니다. 별도 IR/auth/server crate 없이 MCP 제품 설치 단위는 `devup-mcp` 하나입니다. -## 현재 제공 기능 +## 도구 -- `devup_figma_auth`: Figma 연결 상태 확인, 브라우저 OAuth 로그인, 로그아웃, 그리고 연결 실패 원인을 실측해 보고하는 `doctor` 진단 -- `devup_figma_to_ui`: Figma node 링크를 `@devup-ui/react` TSX로 변환 -- `devup_figma_to_json`: Figma 변수와 로컬 스타일을 `devup.json`으로 변환 -- `devup_figma_export`: Figma를 한 번 수집해 TSX, `devup.json`, raw snapshot, source map, asset manifest와 선택적 reference PNG를 함께 생성하거나 같은 artifact를 재사용 +Figma 쪽 4개, 프로젝트 쪽 3개, 모두 7개입니다. + +- `devup_figma_export`: Figma를 한 번 수집해 요청한 `outputs`만 투영합니다. TSX가 산출물이고, `componentTsx`·`responsiveTsx`·`devup.json`·source map·raw snapshot·asset manifest·reference PNG를 같은 수집에서 함께 얻거나, `cache.artifactId`로 재수집 없이 추가 투영할 수 있습니다. - `devup_figma_search`: 파일 전체의 page, section, frame, component를 이름으로 탐색 - `devup_figma_explore`: 링크된 요구사항/라벨 주변의 실제 화면 후보를 공간 순서로 탐색 -- Figma Plugin API의 readable data property를 raw JSON으로 보존하고, 알려지지 않은 runtime field는 `extra`, 실패한 getter는 `fieldErrors`로 유지 +- `devup_figma_auth`: 연결 상태 확인, 브라우저 OAuth 로그인, 로그아웃, 사전 등록 자격증명 주입(`configure`), 연결 실패 원인을 실측해 보고하는 `doctor` +- `devup_project_context`: 프로젝트의 실제 `devup.json` 토큰, `openapi.json` 엔드포인트, Vespertide 모델을 읽음 +- `devup_ui_validate`: 생성한 TSX를 프로젝트의 실제 `devup.json`에 대조해 검증 +- `devup_stack_diff`: DB 모델부터 생성된 API 클라이언트까지의 층간 드리프트 탐지 + +`devup_figma_to_ui`와 `devup_figma_to_json`은 각각 `devup_figma_export`에 `outputs: ["tsx"]`, `outputs: ["devupJson"]`을 넘긴 것과 같아서 제거했습니다. 도구가 셋이면 모든 클라이언트가 세 개의 스키마를 컨텍스트에 싣고도 어느 것을 부를지 매번 판단해야 했습니다. + +devup-mcp는 Figma Plugin API의 readable data property를 raw JSON으로 보존하고, 알려지지 않은 runtime field는 `extra`, 실패한 getter는 `fieldErrors`로 유지합니다. + +### 응답에 무엇이 들어오는가 + +`devup_figma_export`는 **요청한 `outputs`가 만드는 키만** 추가합니다. + +| output | 추가되는 키 | +|---|---| +| `tsx` | `tsx` | +| `componentTsx` | `componentTsx` | +| `responsiveTsx` | `responsiveTsx`, `responsiveSlots`, (표현 불가한 값이 있으면) `responsiveUnrepresented` | +| `devupJson` | `devupJson`, `themeCounts`, `themeCompleteness`, `conflicts`, `unresolvedVariables` | +| `sourceMap` / `rawSnapshot` / `rawPayload` / `assetManifest` / `referencePng` | 같은 이름의 키 | + +그 밖에 항상 붙는 것은 `status`, `quality`, `completeness`, `cache`, `collection`, `source`, `targetKind`, `failures`, `outputPaths`뿐입니다. `fidelity`와 `completenessReport`는 결과가 exact/complete가 **아닐 때**, 또는 `includeDiagnostics: true`일 때만 나옵니다 — 깨끗한 결과에서는 `quality`가 이미 한 말을 되풀이할 뿐이라 빼두었고, 그만큼(측정값 797 B) 매 응답이 가벼워집니다. + +에러는 호출 자체가 잘못된 경우(`DEVUP_INVALID_INPUT`, 없는 node/파일, 만료·부적합한 `artifactId` 등) JSON-RPC `-32602 INVALID_PARAMS`로, 그 밖의 실패는 `-32603 INTERNAL_ERROR`로 옵니다. 인자를 고쳐 다시 부를 일인지 멈추고 보고할 일인지를 메시지를 파싱하지 않고 구분할 수 있습니다. 정확한 `code`와 `retryable`은 예전처럼 `data`에 그대로 실립니다. devup-mcp는 Figma Remote MCP에 직접 붙습니다 — OAuth discovery, Dynamic Client Registration, PKCE S256, 일시적인 `127.0.0.1` callback을 구현합니다. Figma는 MCP Catalog에 승인된 client의 registration만 허용하므로 등록은 allowlist에 있는 `client_name`으로 이루어집니다(기본값 `Codex`). Figma PAT나 사용자가 만든 OAuth app은 필요하지 않습니다. @@ -76,7 +98,7 @@ stdio MCP를 지원하는 클라이언트에 다음과 같이 등록합니다. } ``` -시스템 브라우저는 `devup_figma_auth`의 `login`을 명시적으로 호출할 때만 열립니다. 일반 변환의 기본 `auto` 정책은 direct credential이 없거나 Catalog/capability가 허용되지 않으면 브라우저를 열지 않고 공식 Figma MCP handoff를 반환합니다. 인증 정보는 운영체제 credential store에만 저장되며 `logout`은 해당 정보만 삭제합니다. +시스템 브라우저는 `devup_figma_auth`의 `login`을 명시적으로 호출할 때만 열립니다. 변환 도구는 자격증명이 없으면 브라우저를 열지 않고 로그인이 필요하다는 오류를 반환합니다. 인증 정보는 운영체제 credential store에만 저장되며 `logout`은 해당 정보만 삭제합니다. ### 인증 @@ -84,7 +106,7 @@ stdio MCP를 지원하는 클라이언트에 다음과 같이 등록합니다. { "action": "status" } ``` -`action`은 `status`, `login`, `logout`, `doctor` 중 하나입니다. `status`/`login`/`logout`의 응답 형태는 항상 `{ "status": "connected" | "disconnected" }`입니다. Figma에 붙지 못하는 이유를 알고 싶으면 `doctor`를 호출하세요. +`action`은 `status`, `login`, `logout`, `configure`, `doctor` 중 하나이며, 스키마가 이 목록을 그대로 게시합니다. `status`/`login`/`logout`의 응답 형태는 항상 `{ "status": "connected" | "disconnected" }`입니다. Figma에 붙지 못하는 이유를 알고 싶으면 `doctor`를 호출하세요. ```json { "action": "doctor" } @@ -106,7 +128,7 @@ stdio MCP를 지원하는 클라이언트에 다음과 같이 등록합니다. } ``` -`doctor`는 네트워크 호출을 전혀 하지 않습니다. `paths.direct.credentialSource`는 `cli-arg`, `env`, `credential-store`, `none` 중 하나이고, `tokenState`는 `valid`, `expired`, `absent` 중 하나이며, `callbackPort`는 `--figma-callback-port`를 지정했을 때만 실측한 `port`/`free`를 담습니다. 자세한 제약과 두 연결 경로는 아래 "Figma 연결 설정" 절을 참고하세요. +`doctor`는 네트워크 호출을 전혀 하지 않습니다. `paths.direct.credentialSource`는 `cli-arg`, `env`, `credential-store`, `none` 중 하나이고, `tokenState`는 `valid`, `expired`, `absent` 중 하나이며, `callbackPort`는 `--figma-callback-port`를 지정했을 때만 실측한 `port`/`free`를 담습니다. 자세한 제약은 아래 "Figma 연결 설정" 절을 참고하세요. ### direct 경로에 사전 등록된 client 자격증명 주입하기 @@ -197,26 +219,24 @@ codex mcp add figma --url https://mcp.figma.com/mcp ```json { "url": "https://www.figma.com/design//?node-id=1-2", + "outputs": ["tsx"], "componentName": "OptionalComponentName", - "includeDiagnostics": true, "rootLayout": "standalone", - "sourcePolicy": "auto", "scope": "node", - "outputPath": "optional/path/Component.tsx" + "outputPaths": { "tsx": "optional/path/Component.tsx" } } ``` -결과에는 `tsx`, import 목록, 사용된 token, source 식별자, 보존한 node 수와 fallback diagnostics가 포함됩니다. Auto Layout은 `Flex`, 일반 container는 `Box`, text는 `Text`로 변환하고 theme binding이 있으면 JSX prop에서 `$token`을 우선 사용합니다. 변수 token은 비어 있지 않은 Figma `codeSyntax.WEB`을 우선하고, 없으면 변수 경로의 마지막 이름을 정규화합니다. 따라서 TSX의 `$token`, `usedTokens`, `devup.json` key와 source map이 같은 이름을 사용합니다. `rootLayout` 기본값인 `standalone`은 선택한 root의 크기·위치 제약까지 포함하고 Figma instance의 실제 자식 상태를 펼쳐 정의되지 않은 component 참조를 만들지 않습니다. 이미 레이아웃을 소유한 React 부모 안에 삽입할 때는 `rootLayout: "embedded"`로 root의 외부 크기·위치 제약만 생략합니다. +결과에는 `tsx`와 함께 `status`, `quality`, `cache`, `collection`, `source`가 포함됩니다. import 목록과 사용 token은 별도 키로 보내지 않습니다 — 각각 TSX의 첫 줄과 본문의 `$token`이 이미 같은 내용을 담고 있어, 응답에 두 번 싣는 만큼이 그대로 낭비였습니다. Auto Layout은 `Flex`, 일반 container는 `Box`, text는 `Text`로 변환하고 theme binding이 있으면 JSX prop에서 `$token`을 우선 사용합니다. 변수 token은 비어 있지 않은 Figma `codeSyntax.WEB`을 우선하고, 없으면 변수 경로의 마지막 이름을 정규화합니다. 따라서 TSX의 `$token`, `devup.json` key와 source map이 같은 이름을 사용합니다. `rootLayout` 기본값인 `standalone`은 선택한 root의 크기·위치 제약까지 포함하고 Figma instance의 실제 자식 상태를 펼쳐 정의되지 않은 component 참조를 만들지 않습니다. 이미 레이아웃을 소유한 React 부모 안에 삽입할 때는 `rootLayout: "embedded"`로 root의 외부 크기·위치 제약만 생략합니다. ### Figma → devup.json ```json { "url": "https://www.figma.com/design//?node-id=1-2", + "outputs": ["devupJson"], "scope": "file", - "includeDiagnostics": true, - "sourcePolicy": "auto", - "outputPath": "optional/path/devup.json" + "outputPaths": { "devupJson": "optional/path/devup.json" } } ``` @@ -235,7 +255,6 @@ codex mcp add figma --url https://mcp.figma.com/mcp "scope": "node", "strict": true, "refresh": false, - "sourcePolicy": "auto", "delivery": "auto" } ``` @@ -331,11 +350,10 @@ snapshot에 없는 목적지(legacy 경로, 다중 루트 요청)는 조용히 "nodeTypes": ["PAGE", "SECTION", "FRAME", "COMPONENT_SET"], "match": "normalized", "limit": 20, - "sourcePolicy": "auto" } ``` -검색은 먼저 read-only Plugin API로 실제 `figma.root.children` page catalog를 얻고, page마다 한 번씩 전환하는 작은 query projection을 병렬 실행합니다. 전체 page snapshot을 응답하지 않으므로 큰 파일에서도 공식 MCP text 상한을 피합니다. 결과는 원문 exact, Unicode NFC·공백·대소문자를 정규화한 exact, prefix, contains 순으로 정렬하고 `match: "fuzzy"`일 때만 오타 허용 검색을 추가하며, node ID, type, page, 전체 breadcrumb와 후속 `devup_figma_to_ui`에 그대로 전달할 canonical URL을 포함합니다. +검색은 먼저 read-only Plugin API로 실제 `figma.root.children` page catalog를 얻고, page마다 한 번씩 전환하는 작은 query projection을 병렬 실행합니다. 전체 page snapshot을 응답하지 않으므로 큰 파일에서도 공식 MCP text 상한을 피합니다. 결과는 원문 exact, Unicode NFC·공백·대소문자를 정규화한 exact, prefix, contains 순으로 정렬하고 `match: "fuzzy"`일 때만 오타 허용 검색을 추가하며, node ID, type, page, 전체 breadcrumb와 후속 `devup_figma_export`에 그대로 전달할 canonical URL을 포함합니다. ### 링크 주변 화면 탐색 @@ -345,15 +363,14 @@ snapshot에 없는 목적지(legacy 경로, 다중 루트 요청)는 조용히 "limit": 50, "includeTextPreview": true, "refresh": false, - "sourcePolicy": "auto" } ``` -요구사항 제목이나 설명 node 링크가 실제 구현 화면이 아닐 때 `devup_figma_explore`를 먼저 호출합니다. anchor와 같은 공간 묶음의 frame/component 후보를 시각 순서와 canonical URL로 반환하며, 다음 요구사항 제목에서 탐색 범위를 끝냅니다. 같은 파일·옵션에서 이미 수집한 더 큰 탐색 결과는 exact·related-node·superset 범위로 재사용되고, 동시에 들어온 호환 요청도 공식 Figma 호출 하나를 공유합니다. `refresh: true`는 모든 재사용을 건너뜁니다. 원하는 후보의 canonical URL을 `devup_figma_to_ui`에 넘겨 정확한 화면만 변환합니다. +요구사항 제목이나 설명 node 링크가 실제 구현 화면이 아닐 때 `devup_figma_explore`를 먼저 호출합니다. anchor와 같은 공간 묶음의 frame/component 후보를 시각 순서와 canonical URL로 반환하며, 다음 요구사항 제목에서 탐색 범위를 끝냅니다. 같은 파일·옵션에서 이미 수집한 더 큰 탐색 결과는 exact·related-node·superset 범위로 재사용되고, 동시에 들어온 호환 요청도 공식 Figma 호출 하나를 공유합니다. `refresh: true`는 모든 재사용을 건너뜁니다. 원하는 후보의 canonical URL을 `devup_figma_export`에 넘겨 정확한 화면만 변환합니다. -탐색과 검색은 변수 catalog를 수집하지 않습니다. 정확한 UI 변환 단계에서 선택 subtree의 모든 보존 필드에 있는 `VARIABLE_ALIAS`와 paint/text/effect/grid style ID를 재귀적으로 스캔하고, 실제 사용된 ID만 공식 Figma API로 조회합니다. `devup_figma_to_json`만 file 전체 로컬 catalog를 수집합니다. +탐색과 검색은 변수 catalog를 수집하지 않습니다. 정확한 UI 변환 단계에서 선택 subtree의 모든 보존 필드에 있는 `VARIABLE_ALIAS`와 paint/text/effect/grid style ID를 재귀적으로 스캔하고, 실제 사용된 ID만 공식 Figma API로 조회합니다. `outputs: ["devupJson"]`에 `scope: "file"`을 함께 준 경우에만 file 전체 로컬 catalog를 수집합니다. -`sourcePolicy`는 `auto` 또는 `direct`입니다 — 둘 다 direct 연결을 쓰며, 남겨둔 이유는 하위호환뿐입니다. direct 경로는 연결과 read-only capability catalog 조회를 각각 30초, 개별 tool 호출을 5분으로 제한합니다. deadline을 넘기면 해당 remote session을 폐기하고 디자인 원문 없이 `retryable` timeout 단계만 반환합니다. +Figma 연결은 direct 하나뿐입니다. `sourcePolicy` 파라미터는 `auto`와 `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-devup-ui/src/codegen/component.rs b/crates/devup-mcp-devup-ui/src/codegen/component.rs index dc9381b..9faa04c 100644 --- a/crates/devup-mcp-devup-ui/src/codegen/component.rs +++ b/crates/devup-mcp-devup-ui/src/codegen/component.rs @@ -1,7 +1,8 @@ use std::collections::{BTreeMap, BTreeSet, HashSet}; use devup_mcp_figma::{ - CollectedPayload, DevupError, Diagnostic, ErrorCode, RawNode, Snapshot, UpstreamResult, + CollectedPayload, DevupError, Diagnostic, DiagnosticSeverity, ErrorCode, RawNode, Snapshot, + UpstreamResult, }; use serde::{Deserialize, Serialize}; @@ -1044,12 +1045,129 @@ fn finalize_codegen_output( output.tsx = tsx; output.source_map = source_map; validate_tsx(&output.tsx)?; + output + .diagnostics + .extend(unresolved_token_bindings(snapshot, root_id, options)); output.projection_trace = build_projection_trace(snapshot, root_id, &output.tsx, &output.source_map); output.fidelity_report = validate_fidelity(snapshot, root_id, &output)?; Ok(output) } +/// Reports every binding the generated code had to write out as a value. +/// +/// A fill bound to a variable, or a text carrying a style, is the design +/// saying "this is a token". The generator writes the token when the resource +/// catalog carried that variable or style, and the resolved value when it did +/// not - and it has to write something, because the module still has to +/// compile and render. What it must not do is stay quiet about it: a +/// hardcoded `#7d7f83` where the design says `$caption` renders identically +/// today and stops following the theme tomorrow, and a caller reading a +/// response graded `exact` has no way to know one is in there. +/// +/// Run as a pass over the collected subtree rather than inside rendering, so +/// it sees the node a binding belongs to and needs no argument threaded +/// through the render functions to reach it. +fn unresolved_token_bindings( + snapshot: &Snapshot, + root_id: &str, + options: &CodegenOptions, +) -> Vec { + fn paint_variable_ids(fills: Option<&serde_json::Value>) -> Vec { + fills + .and_then(serde_json::Value::as_array) + .map(|paints| { + paints + .iter() + .filter_map(|paint| { + Some( + paint + .get("boundVariables")? + .get("color")? + .get("id")? + .as_str()? + .to_owned(), + ) + }) + .collect() + }) + .unwrap_or_default() + } + + let mut reported = BTreeSet::new(); + let mut diagnostics = Vec::new(); + let mut pending = vec![root_id.to_owned()]; + let mut seen = BTreeSet::new(); + while let Some(id) = pending.pop() { + if !seen.insert(id.clone()) { + continue; + } + let Some(node) = snapshot.nodes.get(&id) else { + continue; + }; + let view = node.typed_view(); + pending.extend(view.child_ids().map(str::to_owned)); + + let mut lost = Vec::new(); + for variable_id in paint_variable_ids(view.value("fills")) { + if !options.variable_tokens.contains_key(&variable_id) { + lost.push(("variable", "fills", variable_id)); + } + } + if let Some(segments) = view + .value("styledTextSegments") + .and_then(serde_json::Value::as_array) + { + for segment in segments { + for variable_id in paint_variable_ids(segment.get("fills")) { + if !options.variable_tokens.contains_key(&variable_id) { + lost.push(("variable", "fills", variable_id)); + } + } + if let Some(style_id) = segment + .get("textStyleId") + .and_then(serde_json::Value::as_str) + && !style_id.is_empty() + && !options.text_style_tokens.contains_key(style_id) + { + lost.push(("textStyle", "textStyleId", style_id.to_owned())); + } + } + } + if let Some(style_id) = view.string("textStyleId") + && !style_id.is_empty() + && !options.text_style_tokens.contains_key(style_id) + { + lost.push(("textStyle", "textStyleId", style_id.to_owned())); + } + + for (kind, property, resource_id) in lost { + if !reported.insert((id.clone(), property, resource_id.clone())) { + continue; + } + diagnostics.push(Diagnostic { + code: "DEVUP_CODEGEN_TOKEN_NAME_UNRESOLVED".to_owned(), + message: format!( + "{id} binds {property} to a {kind} the resource catalog did not name, \ + so the resolved value was written instead of its token. Collect the \ + resource, or treat the value in the generated code as a token that \ + still needs a name." + ), + severity: Some(DiagnosticSeverity::Warning), + resource_kind: Some(kind.to_owned()), + details: Some(serde_json::json!({ + "nodeId": id, + "property": property, + "resourceId": resource_id + })), + ..Diagnostic::default() + }); + } + } + diagnostics.sort_by(|left, right| left.message.cmp(&right.message)); + diagnostics +} + #[derive(Default)] struct Context { asset_names_per_node: bool, diff --git a/crates/devup-mcp-devup-ui/tests/unresolved_token_binding.rs b/crates/devup-mcp-devup-ui/tests/unresolved_token_binding.rs new file mode 100644 index 0000000..c329046 --- /dev/null +++ b/crates/devup-mcp-devup-ui/tests/unresolved_token_binding.rs @@ -0,0 +1,169 @@ +//! What the generated code does when a value is bound to a variable or a +//! style whose name never arrived. +//! +//! The binding is in the snapshot either way - the node says which variable +//! paints it - so the design's own answer is "this is a token". If the +//! resource catalog did not carry that variable, the token name is unknown +//! and the only thing left to write is the resolved value. Writing it is +//! right: the module still has to compile and render. Writing it *silently* +//! is not, because the response then grades the conversion `exact` while the +//! code has a hardcoded `#7d7f83` where the design has `$caption`, and +//! nothing in the answer says so. + +use devup_mcp_devup_ui::codegen::{CodegenOptions, generate_component}; +use devup_mcp_figma::{SnapshotChunk, merge_chunks}; +use serde_json::json; + +fn snapshot() -> devup_mcp_figma::Snapshot { + let chunk: SnapshotChunk = serde_json::from_value(json!({ + "fileKey": "file-key", + "version": "1", + "rootIds": ["1:1"], + "nodes": [ + { + "id": "1:1", "type": "FRAME", + "fields": { + "name": "Card", "childrenIds": ["1:2"], + "layoutMode": "VERTICAL", + "layoutSizingHorizontal": "FIXED", "layoutSizingVertical": "FIXED", + "width": 320, "height": 80, + // Bound to a variable the catalog never carried. + "fills": [{ + "type": "SOLID", + "color": {"r": 0.49, "g": 0.498, "b": 0.514, "a": 1}, + "boundVariables": {"color": {"type": "VARIABLE_ALIAS", "id": "VariableID:1:1009"}} + }] + }, + "extra": {}, "fieldErrors": {} + }, + { + "id": "1:2", "type": "TEXT", + "fields": { + "name": "Caption", "childrenIds": [], "characters": "caption", + "textTruncation": "DISABLED", + "styledTextSegments": [{ + "characters": "caption", + // Bound to a text style the catalog never carried. + "textStyleId": "S:ddba35e8000000000000000000000000,", + "fontName": {"family": "Pretendard", "style": "Regular"}, + "fontSize": 14, + "fontWeight": 400, + "lineHeight": {"unit": "PERCENT", "value": 160}, + "fills": [{ + "type": "SOLID", + "color": {"r": 0.49, "g": 0.498, "b": 0.514, "a": 1}, + "boundVariables": {"color": {"type": "VARIABLE_ALIAS", "id": "VariableID:1:1009"}} + }] + }] + }, + "extra": {}, "fieldErrors": {} + } + ], + "diagnostics": [] + })) + .expect("synthetic snapshot"); + merge_chunks(vec![chunk]).expect("snapshot") +} + +#[test] +fn a_binding_whose_name_never_arrived_is_reported_rather_than_quietly_resolved() { + let output = generate_component( + &snapshot(), + "1:1", + &CodegenOptions { + include_diagnostics: true, + ..CodegenOptions::default() + }, + ) + .expect("codegen"); + + // The module still has to be usable, so the resolved value is written. + assert!( + output.tsx.contains("#7d7f83") || output.tsx.contains("#7D7F83"), + "a value with no reachable token name still has to render:\n{}", + output.tsx + ); + + // But the answer has to admit it. Each binding that could not be named is + // reported once, with the node and the resource it came from, so a caller + // reading only the response knows a token was lost. + let unresolved = output + .diagnostics + .iter() + .filter(|diagnostic| diagnostic.code == "DEVUP_CODEGEN_TOKEN_NAME_UNRESOLVED") + .collect::>(); + assert!( + !unresolved.is_empty(), + "a hardcoded value standing in for a token must be reported, got: {:?}", + output + .diagnostics + .iter() + .map(|diagnostic| &diagnostic.code) + .collect::>() + ); + let reported = serde_json::to_string(&unresolved).expect("diagnostics serialize"); + assert!( + reported.contains("VariableID:1:1009"), + "the variable that could not be named must be identified: {reported}" + ); + assert!( + reported.contains("S:ddba35e8000000000000000000000000,"), + "the text style that could not be named must be identified: {reported}" + ); + + // And it must not be graded as an exact reproduction of the design. + assert!( + !output.fidelity_report.strict_compatible(), + "a lost token is a fidelity shortfall" + ); +} + +#[test] +fn a_binding_the_catalog_carried_is_written_as_its_token() { + let output = generate_component( + &snapshot(), + "1:1", + &CodegenOptions { + include_diagnostics: true, + variable_tokens: [("VariableID:1:1009".to_owned(), "caption".to_owned())] + .into_iter() + .collect(), + text_style_tokens: [( + "S:ddba35e8000000000000000000000000,".to_owned(), + "captionSm".to_owned(), + )] + .into_iter() + .collect(), + ..CodegenOptions::default() + }, + ) + .expect("codegen"); + + assert!( + output.tsx.contains("$caption"), + "a named binding belongs in the code as its token:\n{}", + output.tsx + ); + assert!( + output.tsx.contains("typography=\"captionSm\""), + "a named text style belongs in the code as its typography:\n{}", + output.tsx + ); + assert!( + !output.tsx.contains("#7d7f83") && !output.tsx.contains("#7D7F83"), + "no resolved value should survive beside the token that names it:\n{}", + output.tsx + ); + assert!( + !output.tsx.contains("fontSize=\"14px\""), + "a typography token replaces the font properties it stands for:\n{}", + output.tsx + ); + assert!( + output + .diagnostics + .iter() + .all(|diagnostic| diagnostic.code != "DEVUP_CODEGEN_TOKEN_NAME_UNRESOLVED"), + "nothing was lost, so nothing should be reported" + ); +} diff --git a/crates/devup-mcp-figma/src/errors.rs b/crates/devup-mcp-figma/src/errors.rs index 6280705..b08d914 100644 --- a/crates/devup-mcp-figma/src/errors.rs +++ b/crates/devup-mcp-figma/src/errors.rs @@ -11,7 +11,6 @@ pub enum ErrorCode { DevupFigmaRateLimited, DevupFigmaDirectUnavailable, DevupFigmaCatalogRejected, - DevupFigmaHostRequired, DevupFigmaHandoffExpired, DevupFigmaHandoffInvalid, DevupFigmaNodeNotFound, @@ -21,11 +20,34 @@ pub enum ErrorCode { DevupSnapshotUnsupported, DevupCodegenFailed, DevupThemeConflict, - DevupCompatCorpusDrift, DevupInvalidInput, DevupProjectRootNotFound, } +impl ErrorCode { + /// Whether this names a mistake in the call itself rather than a failure + /// behind it. + /// + /// Every error used to reach the caller as JSON-RPC `INTERNAL_ERROR`, so + /// "you passed a scope this tool does not have" and "Figma stopped + /// answering" arrived indistinguishable at the protocol level. That + /// matters more here than in a human-facing API, because the caller is + /// usually an agent deciding between two different next moves: fix the + /// arguments and call again, or stop and report. The codes below are the + /// ones the caller can act on by changing what it sent. + pub const fn is_caller_mistake(self) -> bool { + matches!( + self, + Self::DevupInvalidInput + | Self::DevupFigmaNodeNotFound + | Self::DevupFigmaUnsupportedFile + | Self::DevupProjectRootNotFound + | Self::DevupFigmaHandoffInvalid + | Self::DevupFigmaHandoffExpired + ) + } +} + #[derive(Clone, Serialize, Deserialize)] pub struct DevupError { pub code: ErrorCode, diff --git a/crates/devup-mcp-figma/src/lib.rs b/crates/devup-mcp-figma/src/lib.rs index d736795..85714de 100644 --- a/crates/devup-mcp-figma/src/lib.rs +++ b/crates/devup-mcp-figma/src/lib.rs @@ -64,8 +64,7 @@ pub use snapshot::{ read_snapshot_cursor, snapshot_chunk_from_result, }; pub use source::{ - SourcePolicy, UpstreamFailureContext, UpstreamFailureKind, classify_upstream_failure, - upstream_failure_error, + UpstreamFailureContext, UpstreamFailureKind, classify_upstream_failure, upstream_failure_error, }; pub use upstream::{ BuiltinScript, ExploreReadOptions, FigmaUpstream, ReadToolCall, RemoteFigmaClient, diff --git a/crates/devup-mcp-figma/src/source.rs b/crates/devup-mcp-figma/src/source.rs index d4e3dc1..40f81b4 100644 --- a/crates/devup-mcp-figma/src/source.rs +++ b/crates/devup-mcp-figma/src/source.rs @@ -1,16 +1,7 @@ -use serde::{Deserialize, Serialize}; use serde_json::json; use crate::{DevupError, ErrorCode}; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub enum SourcePolicy { - #[default] - Auto, - Direct, -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum UpstreamFailureContext { RegisterClient, @@ -132,10 +123,12 @@ impl UpstreamFailureKind { }; let mut details = json!({ "source": "direct", "status": status }); if self == Self::CatalogRejected { + // There is no host-handoff path any more, so the options are the + // two that actually exist: get a credential of your own, or get + // this client admitted. details["options"] = json!([ - "Register devup-mcp on the Figma MCP Catalog waitlist: https://www.figma.com/mcp-catalog/", "Inject client credentials you obtained yourself via devup_figma_auth { action: \"configure\", clientId, clientSecret }", - "Hand off to the official Figma MCP registered on the host (sourcePolicy: auto or host, the current default fallback)" + "Register your client on the Figma MCP Catalog waitlist: https://www.figma.com/mcp-catalog/" ]); } DevupError::with_details(code, message, retryable, details) diff --git a/crates/devup-mcp-figma/tests/oauth_flow.rs b/crates/devup-mcp-figma/tests/oauth_flow.rs index 017dcdf..78e5545 100644 --- a/crates/devup-mcp-figma/tests/oauth_flow.rs +++ b/crates/devup-mcp-figma/tests/oauth_flow.rs @@ -347,10 +347,13 @@ async fn dcr_403_is_classified_as_catalog_rejected_with_actionable_options() -> let options = error.details["options"] .as_array() .expect("catalog-rejected errors carry actionable options"); - // Three, not four: the local Dev Mode MCP was offered here and cannot - // serve devup-mcp at all, since it has no use_figma to run a collection - // with. An option that cannot work costs a turn to discover. - assert_eq!(options.len(), 3); + // Exactly the two routes that exist: bring a credential of your own, or + // get this client admitted. The local Dev Mode MCP was offered here once + // and cannot serve devup-mcp at all, having no use_figma to run a + // collection with; a handoff to the host's own Figma MCP was offered + // after that, and there is no handoff path any more. An option that + // cannot work costs a turn to discover. + assert_eq!(options.len(), 2); assert!( options .iter() @@ -361,6 +364,11 @@ async fn dcr_403_is_classified_as_catalog_rejected_with_actionable_options() -> .iter() .any(|option| option.as_str().unwrap_or_default().contains("mcp-catalog")) ); + assert!( + !options + .iter() + .any(|option| option.as_str().unwrap_or_default().contains("sourcePolicy")) + ); let serialized = serde_json::to_string(&error)?; assert!(!serialized.contains("Forbidden")); diff --git a/crates/devup-mcp-figma/tests/source_policy.rs b/crates/devup-mcp-figma/tests/upstream_failures.rs similarity index 56% rename from crates/devup-mcp-figma/tests/source_policy.rs rename to crates/devup-mcp-figma/tests/upstream_failures.rs index 90db82b..cde2892 100644 --- a/crates/devup-mcp-figma/tests/source_policy.rs +++ b/crates/devup-mcp-figma/tests/upstream_failures.rs @@ -1,5 +1,5 @@ use devup_mcp_figma::{ - ErrorCode, SourcePolicy, UpstreamFailureContext, UpstreamFailureKind, classify_upstream_failure, + ErrorCode, UpstreamFailureContext, UpstreamFailureKind, classify_upstream_failure, }; #[test] @@ -65,12 +65,7 @@ fn classifies_upstream_failures_from_boundary_metadata() { } #[test] -fn public_policy_and_error_codes_have_stable_json_values() { - assert_eq!(serde_json::to_value(SourcePolicy::Auto).unwrap(), "auto"); - assert_eq!( - serde_json::to_value(SourcePolicy::Direct).unwrap(), - "direct" - ); +fn error_codes_have_stable_json_values() { let codes = [ ( ErrorCode::DevupFigmaDirectUnavailable, @@ -80,10 +75,6 @@ fn public_policy_and_error_codes_have_stable_json_values() { ErrorCode::DevupFigmaCatalogRejected, "DEVUP_FIGMA_CATALOG_REJECTED", ), - ( - ErrorCode::DevupFigmaHostRequired, - "DEVUP_FIGMA_HOST_REQUIRED", - ), ( ErrorCode::DevupFigmaHandoffExpired, "DEVUP_FIGMA_HANDOFF_EXPIRED", @@ -92,16 +83,65 @@ fn public_policy_and_error_codes_have_stable_json_values() { ErrorCode::DevupFigmaHandoffInvalid, "DEVUP_FIGMA_HANDOFF_INVALID", ), - ( - ErrorCode::DevupCompatCorpusDrift, - "DEVUP_COMPAT_CORPUS_DRIFT", - ), + (ErrorCode::DevupInvalidInput, "DEVUP_INVALID_INPUT"), ]; for (code, expected) in codes { assert_eq!(serde_json::to_value(code).unwrap(), expected); } } +/// This split is what decides whether the caller sees INVALID_PARAMS or +/// INTERNAL_ERROR, which for an agent is the difference between fixing its +/// arguments and giving up. Getting a code onto the wrong side is silent — +/// the response still looks well formed — so both sides are pinned here. +/// +/// A variant added later and left unclassified falls to the `false` side and +/// is reported as INTERNAL_ERROR, which is the safe direction: the caller +/// stops instead of retrying a call that will never succeed. +#[test] +fn caller_mistakes_are_separated_from_failures_behind_the_call() { + for code in [ + ErrorCode::DevupInvalidInput, + ErrorCode::DevupFigmaNodeNotFound, + ErrorCode::DevupFigmaUnsupportedFile, + ErrorCode::DevupProjectRootNotFound, + ErrorCode::DevupFigmaHandoffInvalid, + ErrorCode::DevupFigmaHandoffExpired, + ] { + assert!( + code.is_caller_mistake(), + "{code:?} is fixable by the caller" + ); + } + + for code in [ + // Authentication, the network, and Figma itself: nothing the caller + // can repair by changing an argument. + ErrorCode::DevupAuthRequired, + ErrorCode::DevupAuthCallbackTimeout, + ErrorCode::DevupAuthStateMismatch, + ErrorCode::DevupFigmaCallbackPortInUse, + ErrorCode::DevupFigmaPermissionDenied, + ErrorCode::DevupFigmaRateLimited, + ErrorCode::DevupFigmaDirectUnavailable, + ErrorCode::DevupFigmaCatalogRejected, + ErrorCode::DevupFigmaResponseTooLarge, + ErrorCode::DevupFigmaVersionChanged, + // Conditions found in the design or the generated code, not in the + // arguments: a theme whose collections disagree is a real conflict, + // and reporting it as a bad parameter would send the caller looking + // for a typo it will not find. + ErrorCode::DevupSnapshotUnsupported, + ErrorCode::DevupCodegenFailed, + ErrorCode::DevupThemeConflict, + ] { + assert!( + !code.is_caller_mistake(), + "{code:?} is not fixable by changing the call" + ); + } +} + #[test] fn classified_errors_never_copy_the_raw_upstream_message() { let raw = "catalog rejected Authorization: Bearer figma-secret-token"; diff --git a/crates/devup-mcp/src/server/artifacts.rs b/crates/devup-mcp/src/server/artifacts.rs index e159108..0493532 100644 --- a/crates/devup-mcp/src/server/artifacts.rs +++ b/crates/devup-mcp/src/server/artifacts.rs @@ -9,7 +9,7 @@ use std::{ use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; use devup_mcp_figma::{ AssetSelection, CollectedPayload, CollectionRequest, CollectionScope, DevupError, ErrorCode, - ExploreReadOptions, ResourceScope, SearchReadOptions, SectionReadOptions, SourcePolicy, + ExploreReadOptions, ResourceScope, SearchReadOptions, SectionReadOptions, }; use rand::Rng; use serde::{Deserialize, Serialize}; @@ -35,11 +35,10 @@ pub struct ArtifactRequestKey { section: Option, asset_selections: Vec, reference_png: bool, - source_policy: SourcePolicy, } impl ArtifactRequestKey { - pub fn from_collection(request: &CollectionRequest, source_policy: SourcePolicy) -> Self { + pub fn from_collection(request: &CollectionRequest) -> Self { let mut section = request.section.clone(); if let Some(section) = &mut section { section.frame_ids.sort(); @@ -68,7 +67,6 @@ impl ArtifactRequestKey { section, asset_selections, reference_png: request.reference_png, - source_policy, } } diff --git a/crates/devup-mcp/src/server/mod.rs b/crates/devup-mcp/src/server/mod.rs index 734cb27..c4fcb2a 100644 --- a/crates/devup-mcp/src/server/mod.rs +++ b/crates/devup-mcp/src/server/mod.rs @@ -37,7 +37,7 @@ use devup_mcp_figma::{ ExploreNode, ExploreReadOptions, FigmaTarget, FigmaUpstream, KeyringClientCredentialStore, KeyringCredentialStore, OAuthManager, ReadToolCall, RemoteFigmaClient, ResourceScope, SearchReadOptions, SecretString, SectionCandidate, SectionIndex, SectionReadOptions, - SourcePolicy, SystemBrowser, TokenState, UpstreamResult, + SystemBrowser, TokenState, UpstreamResult, }; use artifacts::{ArtifactKind, ArtifactRequestKey, ArtifactStore}; @@ -48,13 +48,13 @@ use output::OutputPolicy; use pacing::CallPacer; use projection::complete_operation; use validation::{ - parse_asset_requests, parse_collection_scope, parse_root_layout, parse_source_policy, - validate_artifact_projection, validate_outputs, + parse_asset_requests, parse_collection_scope, parse_root_layout, validate_artifact_projection, + validate_outputs, }; pub use tools::{ AuthInput, FigmaAssetRequestInput, FigmaExploreInput, FigmaExportInput, FigmaSearchInput, - FigmaToJsonInput, FigmaToUiInput, ProjectContextInput, StackDiffInput, UiValidateInput, + ProjectContextInput, StackDiffInput, UiValidateInput, }; const FIGMA_ENDPOINT: &str = "https://mcp.figma.com/mcp"; @@ -284,10 +284,9 @@ impl DevupServer { &self, operation: PendingOperation, request: CollectionRequest, - policy: SourcePolicy, refresh: bool, ) -> Result { - let artifact_key = ArtifactRequestKey::from_collection(&request, policy); + let artifact_key = ArtifactRequestKey::from_collection(&request); if !refresh && let Some(artifact) = self.artifacts.lookup(&artifact_key).await { return complete_operation( operation, @@ -535,89 +534,6 @@ impl DevupServer { Ok(tool_result(json!({ "status": status }))) } - #[tool( - description = "Convert a Figma design link to deterministic DevupUI TypeScript only; use devup_figma_export when tokens or a source map are also needed, and never hand-interpret a handoff node tree", - output_schema = permissive_object_output_schema() - )] - async fn devup_figma_to_ui( - &self, - Parameters(input): Parameters, - ) -> Result { - let target = FigmaTarget::parse(&input.url).map_err(to_mcp_error)?; - target.node_id.as_ref().ok_or_else(|| { - to_mcp_error(DevupError::new( - ErrorCode::DevupFigmaNodeNotFound, - "A UI conversion link requires a node-id.", - false, - )) - })?; - let policy = parse_source_policy(&input.source_policy).map_err(to_mcp_error)?; - let scope = parse_collection_scope(&input.scope).map_err(to_mcp_error)?; - let root_layout = parse_root_layout(&input.root_layout).map_err(to_mcp_error)?; - let delivery = input - .delivery - .parse::() - .map_err(to_mcp_error)?; - let mut request = CollectionRequest::new(target, scope); - request.resource_scope = ResourceScope::Used; - let result = self - .start_operation( - PendingOperation::ToUi { - component_name: input.component_name, - include_diagnostics: input.include_diagnostics, - root_layout, - output_path: input.output_path, - delivery, - }, - request, - policy, - false, - ) - .await - .map_err(to_mcp_error)?; - Ok(tool_result(result)) - } - - #[tool( - description = "Convert Figma variables and styles to deterministic devup.json", - output_schema = permissive_object_output_schema() - )] - async fn devup_figma_to_json( - &self, - Parameters(input): Parameters, - ) -> Result { - let target = FigmaTarget::parse(&input.url).map_err(to_mcp_error)?; - parse_scope(&input.scope).map_err(to_mcp_error)?; - let policy = parse_source_policy(&input.source_policy).map_err(to_mcp_error)?; - let collection_scope = parse_collection_scope(&input.scope).map_err(to_mcp_error)?; - let delivery = input - .delivery - .parse::() - .map_err(to_mcp_error)?; - let mut request = CollectionRequest::new(target, collection_scope); - if collection_scope == CollectionScope::File { - request.resource_scope = ResourceScope::File; - request.variables_only = true; - } else { - request.resource_scope = ResourceScope::Used; - } - let result = self - .start_operation( - PendingOperation::ToJson { - scope: input.scope, - include_diagnostics: input.include_diagnostics, - output_path: input.output_path, - delivery, - }, - request, - policy, - false, - ) - .await - .map_err(to_mcp_error)?; - Ok(tool_result(result)) - } - #[tool( description = "Search Figma pages, sections, frames, and components by name to locate the target before devup_figma_export", output_schema = permissive_object_output_schema() @@ -627,7 +543,6 @@ impl DevupServer { Parameters(input): Parameters, ) -> Result { let target = FigmaTarget::parse(&input.url).map_err(to_mcp_error)?; - let policy = parse_source_policy(&input.source_policy).map_err(to_mcp_error)?; let mut request = CollectionRequest::new(target, CollectionScope::File); request.search = Some(SearchReadOptions { query: input.query.clone(), @@ -644,7 +559,6 @@ impl DevupServer { limit: input.limit, }, request, - policy, false, ) .await @@ -675,7 +589,6 @@ impl DevupServer { false, ))); } - let policy = parse_source_policy(&input.source_policy).map_err(to_mcp_error)?; let requested_target = target.clone(); let mut request = CollectionRequest::new(target, CollectionScope::Node); request.resource_scope = ResourceScope::None; @@ -690,7 +603,6 @@ impl DevupServer { target: requested_target, }, request, - policy, input.refresh, ) .await @@ -699,7 +611,12 @@ impl DevupServer { } #[tool( - description = "Acquire a Figma design once and project tsx/componentTsx/devupJson/sourceMap/rawSnapshot together in one collection; the primary Figma-to-code entry point, preferred over devup_figma_to_ui for implementation. Request tsx and componentTsx together to get the same screen twice: tsx expands every instance into primitives, componentTsx keeps them as references with their imports, so the difference between them is each component's body. responsiveTsx is the whole screen at every width it is drawn at, merged into one module whose differing values are devup-ui responsive arrays; it is produced whenever the capture carries more than one width, and names anything the widths asked for that one tree cannot say", + description = "Acquire a Figma design once and project any combination of outputs from that one collection; the Figma-to-code entry point. \ + Ask only for what you will read: `tsx` is the deliverable, and the response always carries `status`, `quality`, `cache.artifactId`, `collection` and `source` beside it. \ + Each output adds its own keys and nothing else - tsx adds `tsx`; componentTsx adds `componentTsx`; responsiveTsx adds `responsiveTsx`, `responsiveSlots` and, where a width asked for something one tree cannot say, `responsiveUnrepresented`; devupJson adds `devupJson`, `themeCounts`, `themeCompleteness`, `conflicts` and `unresolvedVariables`; sourceMap, rawSnapshot, rawPayload, assetManifest and referencePng each add the key they name. \ + `fidelity` and `completenessReport` appear only when the result is not exact or complete, or when includeDiagnostics is set. \ + tsx expands every instance into primitives while componentTsx keeps them as references, so requesting both gives the same screen twice and the difference between them is each component's body. responsiveTsx merges every width the capture carries into one module whose differing values are devup-ui responsive arrays, and is produced whenever there is more than one width. \ + Reuse a previous acquisition with `artifactId` from `cache` to project further outputs without calling Figma again.", output_schema = permissive_object_output_schema() )] async fn devup_figma_export( @@ -758,7 +675,6 @@ impl DevupServer { false, )) })?; - let policy = parse_source_policy(&input.source_policy).map_err(to_mcp_error)?; let collection_scope = parse_collection_scope(&input.scope).map_err(to_mcp_error)?; if collection_scope != CollectionScope::Node { @@ -796,7 +712,6 @@ impl DevupServer { delivery, }, request, - policy, false, ) .await @@ -852,7 +767,6 @@ impl DevupServer { false, ))); } - let policy = parse_source_policy(&input.source_policy).map_err(to_mcp_error)?; let collection_scope = parse_collection_scope(&input.scope).map_err(to_mcp_error)?; let mut request = CollectionRequest::new(target, collection_scope); let (asset_selections, asset_output_paths) = @@ -891,7 +805,6 @@ impl DevupServer { delivery, }, request, - policy, input.refresh, ) .await @@ -1020,9 +933,22 @@ fn parse_scope(scope: &str) -> Result { } } +/// Maps a [`DevupError`] onto the JSON-RPC error the caller actually sees. +/// +/// The protocol code is not decoration here: the caller is usually an agent +/// choosing between "fix the arguments and call again" and "stop and report", +/// and every error used to arrive as INTERNAL_ERROR, which says the second +/// thing about both. A mistake in the call itself is INVALID_PARAMS, so that +/// decision can be made without parsing the message. `data` keeps carrying +/// the exact `code` and `retryable`, unchanged. fn to_mcp_error(error: DevupError) -> ErrorData { + let mcp_code = if error.code.is_caller_mistake() { + McpErrorCode::INVALID_PARAMS + } else { + McpErrorCode::INTERNAL_ERROR + }; ErrorData::new( - McpErrorCode::INTERNAL_ERROR, + mcp_code, error.message, Some(json!({ "code": error.code, "retryable": error.retryable, "details": error.details })), ) @@ -1040,7 +966,8 @@ impl ServerHandler for DevupServer { .with_server_info(Implementation::new("devup-mcp", env!("CARGO_PKG_VERSION"))) .with_instructions( "1. devup-mcp is the primary source for turning a Figma design into code. Do not replace it with another source.\n\ - 2. When the goal is implementation, call devup_figma_export first and take tsx, rawSnapshot, and sourceMap together.\n\ + 2. When the goal is implementation, call devup_figma_export first and take tsx. That is the deliverable; a complete response marks it with deliverable.isFinal.\n\ + 2a. Ask for an output only when you will read it. Measured against the same screen, sourceMap is about 5x the size of the tsx it annotates, rawPayload about 7x, and rawSnapshot about 2x, so requesting them by default spends most of the response on bytes nothing reads. sourceMap is for tracing a generated line back to its Figma node; rawSnapshot and rawPayload are for banking a capture as an offline fixture. componentTsx is the same screen with instances left as references, and responsiveTsx appears on its own whenever the capture carries more than one width.\n\ 3. get_design_context, screenshots, and visual reasoning are verification aids only. Do not overwrite devup-mcp output.\n\ 4. Do not hand-interpret a node tree to write devup-ui code. Do not infer layout from coordinates.\n\ 5. If a devup-mcp call fails, record it explicitly. Do not silently route around it.\n\ diff --git a/crates/devup-mcp/src/server/operation.rs b/crates/devup-mcp/src/server/operation.rs index fcfc84f..c17fe4b 100644 --- a/crates/devup-mcp/src/server/operation.rs +++ b/crates/devup-mcp/src/server/operation.rs @@ -24,19 +24,6 @@ pub enum PendingOperation { operation: Box, artifact_key: ArtifactRequestKey, }, - ToUi { - component_name: Option, - include_diagnostics: bool, - root_layout: RootLayout, - output_path: Option, - delivery: DeliveryMode, - }, - ToJson { - scope: String, - include_diagnostics: bool, - output_path: Option, - delivery: DeliveryMode, - }, Export { outputs: Vec, component_name: Option, diff --git a/crates/devup-mcp/src/server/projection.rs b/crates/devup-mcp/src/server/projection.rs index 2c71066..6196486 100644 --- a/crates/devup-mcp/src/server/projection.rs +++ b/crates/devup-mcp/src/server/projection.rs @@ -24,7 +24,8 @@ use super::{ output::{OutputPolicy, OutputTransaction}, parse_scope, quality::{ - OutputQuality, acquisition_quality, assets_quality, projection_quality, theme_quality, + AcquisitionQuality, OutputQuality, ProjectionQuality, acquisition_quality, assets_quality, + projection_quality, theme_quality, }, section_candidate_as_explore, section_index_from_payload, }; @@ -403,18 +404,51 @@ pub(super) fn artifact_metadata(artifact: &ArtifactLookup) -> Value { }) } -pub(super) fn commit_single_output( - policy: &OutputPolicy, - path: Option<&str>, - name: &str, - contents: &[u8], -) -> Result, DevupError> { - let Some(path) = path else { - return Ok(None); - }; - let mut transaction = OutputTransaction::new(); - transaction.stage(name, policy.resolve(path)?, contents)?; - Ok(transaction.commit()?.remove(name)) +/// Adds `completenessReport` only when it has something to say. +/// +/// `quality.acquisition` already grades the capture, and on a clean one the +/// report underneath it is six empty arrays and a row of counters that agree +/// with the grade. Measured on one export it was 360 of the 1,911 bytes every +/// response carried before this, none of it actionable. It is attached +/// whenever the capture is not clean, and whenever the caller asked for +/// diagnostics and therefore wants the detail regardless. +/// Adds `fidelity` only when the conversion was not exact. +/// +/// `quality.projection` is the signal a caller acts on; `fidelity` is the +/// drill-down beneath it, at 437 measured bytes, repeated once per screen on +/// a Section export. Tying it to that same grade keeps the two from +/// disagreeing: an exact conversion sends the grade alone, and anything less +/// sends the axes that explain it. +/// +/// Deliberately not keyed on `strict_compatible`, which also fails on a +/// coverage shortfall that changes nothing about the output - that would put +/// the report back on almost every response while `quality` still read +/// `exact`. `strict: true` keeps using `strict_compatible` to refuse, and +/// returns the same report in the error. +fn attach_fidelity( + response: &mut Value, + report: &devup_mcp_devup_ui::provenance::FidelityReport, + projection: ProjectionQuality, + include_diagnostics: bool, +) { + if include_diagnostics || projection != ProjectionQuality::Exact { + response["fidelity"] = json!(report); + } +} + +fn attach_completeness_report( + response: &mut Value, + quality: OutputQuality, + report: &devup_mcp_figma::PayloadCompletenessReport, + include_diagnostics: bool, +) { + let clean = matches!( + quality.acquisition, + AcquisitionQuality::Complete | AcquisitionQuality::ExpectedProjection + ); + if include_diagnostics || !clean { + response["completenessReport"] = json!(report); + } } pub(super) async fn complete_operation( @@ -432,195 +466,6 @@ pub(super) async fn complete_operation( }; let completeness_report = payload.completeness_report(); match operation { - PendingOperation::ToUi { - component_name, - include_diagnostics, - root_layout, - output_path, - delivery, - } => { - let node_id = payload.target.node_id.as_deref().ok_or_else(|| { - DevupError::new( - ErrorCode::DevupFigmaNodeNotFound, - "A UI conversion payload requires a node ID.", - false, - ) - })?; - let output = generate_component( - &payload.snapshot, - node_id, - &CodegenOptions { - component_name, - include_diagnostics, - inline_instances: true, - root_layout, - // `devup_figma_to_ui` hands back one module and no files, - // so there is nothing for a per-node name to keep apart. - asset_names_per_node: false, - ..CodegenOptions::default() - } - .with_payload_tokens(payload), - )?; - let quality = OutputQuality { - acquisition: acquisition_quality(&completeness_report, false), - projection: projection_quality(true, &output.diagnostics), - theme: theme_quality(false, 0, 0), - assets: assets_quality(false, &[], &[]), - }; - let status = quality.status(); - let diagnostics = if include_diagnostics { - output.diagnostics.clone() - } else { - Vec::new() - }; - let projected_outputs = vec![ProjectedOutput::text( - "tsx", - "text/typescript", - output.tsx.as_bytes().to_vec(), - )]; - let mut result = json!({ - "status": status, - "quality": quality, - "tsx": output.tsx, - "imports": output.imports, - "usedTokens": output.used_tokens, - "fidelity": output.fidelity_report, - "diagnostics": diagnostics, - "outputPath": null, - "completeness": payload.completeness, - "completenessReport": &completeness_report, - "rootLayout": root_layout, - "collection": collection, - "cache": artifact_metadata(artifact), - "source": { - "kind": source_kind, - "fileKey": payload.target.file_key, - "nodeId": node_id, - "version": payload.snapshot.version - }, - "snapshot": { - "preservedNodeCount": payload.snapshot.nodes.len(), - "fieldErrorCount": payload.snapshot.nodes.values() - .map(|node| node.field_errors.len()).sum::() - } - }); - let attachment = apply_delivery( - &mut result, - delivery, - artifact_store, - artifact, - projected_outputs, - ) - .await?; - let written_path = match commit_single_output( - output_policy, - output_path.as_deref(), - "tsx", - output.tsx.as_bytes(), - ) { - Ok(path) => path, - Err(error) => { - rollback_delivery(artifact_store, artifact, attachment).await; - return Err(error); - } - }; - commit_delivery(attachment); - result["outputPath"] = json!(written_path); - if status == "complete" { - // Unambiguous "this is the real, final answer" marker. - // Without it, an agent repeatedly seeing `needs_figma` - // intermediate steps has, in an observed real failure, - // concluded the conversion was "probably done" and moved - // on to hand-interpreting the raw node tree instead of - // waiting for this response. - result["deliverable"] = json!({ - "kind": "devup-ui-tsx", - "isFinal": true, - "note": "This tsx is the final deliverable. Implement from this value." - }); - } - Ok(result) - } - PendingOperation::ToJson { - scope, - include_diagnostics, - output_path, - delivery, - } => { - let result = payload.variables.as_ref().ok_or_else(|| { - DevupError::new( - ErrorCode::DevupSnapshotUnsupported, - "There is no Figma variable/style collection result.", - false, - ) - })?; - let variables = variable_snapshot_from_result(result)?; - let output = generate_devup_json(&variables, parse_scope(&scope)?)?; - let quality = OutputQuality { - acquisition: acquisition_quality(&completeness_report, false), - projection: projection_quality(false, &[]), - theme: theme_quality( - true, - output.conflicts.len(), - output.unresolved_variables.len(), - ), - assets: assets_quality(false, &[], &[]), - }; - let status = quality.status(); - let diagnostics = if include_diagnostics { - output.diagnostics.clone() - } else { - Vec::new() - }; - let projected_outputs = vec![ProjectedOutput::text( - "devupJson", - "application/json", - output.json.as_bytes().to_vec(), - )]; - let mut result = json!({ - "status": status, - "quality": quality, - "devupJson": output.json, - "counts": output.counts, - "completeness": output.completeness, - "completenessReport": &completeness_report, - "conflicts": output.conflicts, - "unresolvedVariables": output.unresolved_variables, - "diagnostics": diagnostics, - "outputPath": null, - "collection": collection, - "cache": artifact_metadata(artifact), - "source": { - "kind": source_kind, - "fileKey": payload.target.file_key, - "nodeId": payload.target.node_id, - "version": payload.snapshot.version - } - }); - let attachment = apply_delivery( - &mut result, - delivery, - artifact_store, - artifact, - projected_outputs, - ) - .await?; - let written_path = match commit_single_output( - output_policy, - output_path.as_deref(), - "devupJson", - output.json.as_bytes(), - ) { - Ok(path) => path, - Err(error) => { - rollback_delivery(artifact_store, artifact, attachment).await; - return Err(error); - } - }; - commit_delivery(attachment); - result["outputPath"] = json!(written_path); - Ok(result) - } PendingOperation::Search { query, node_types, @@ -643,14 +488,13 @@ pub(super) async fn complete_operation( theme: theme_quality(false, 0, 0), assets: assets_quality(false, &[], &[]), }; - Ok(json!({ + let mut response = json!({ "status": quality.status(), "quality": quality, "query": query, "count": matches.len(), "matches": matches, "completeness": payload.completeness, - "completenessReport": &completeness_report, "collection": collection, "cache": artifact_metadata(artifact), "source": { @@ -658,7 +502,9 @@ pub(super) async fn complete_operation( "fileKey": payload.target.file_key, "version": payload.snapshot.version } - })) + }); + attach_completeness_report(&mut response, quality, &completeness_report, false); + Ok(response) } PendingOperation::Explore { limit, target } => { let result = explore_snapshot(&payload.snapshot, &target, &ExploreOptions { limit })?; @@ -669,7 +515,7 @@ pub(super) async fn complete_operation( theme: theme_quality(false, 0, 0), assets: assets_quality(false, &[], &[]), }; - Ok(json!({ + let mut response = json!({ "status": quality.status(), "quality": quality, "targetKind": result.target_kind, @@ -680,7 +526,6 @@ pub(super) async fn complete_operation( "truncated": result.truncated, "diagnostics": payload.snapshot.diagnostics, "completeness": payload.completeness, - "completenessReport": &completeness_report, "collection": collection, "cache": artifact_metadata(artifact), "source": { @@ -689,7 +534,9 @@ pub(super) async fn complete_operation( "nodeId": target.node_id, "version": payload.snapshot.version } - })) + }); + attach_completeness_report(&mut response, quality, &completeness_report, false); + Ok(response) } PendingOperation::Export { outputs, @@ -707,8 +554,10 @@ pub(super) async fn complete_operation( delivery, } => { let mut result = Map::new(); + // Not restated by quality: this grades how far token resolution + // reached - whether external library variables were covered - where + // quality.theme only says whether what was resolved conflicts. result.insert("completeness".to_owned(), json!(payload.completeness)); - result.insert("completenessReport".to_owned(), json!(&completeness_report)); result.insert("collection".to_owned(), json!(collection)); result.insert("cache".to_owned(), artifact_metadata(artifact)); result.insert("failures".to_owned(), json!(&payload.failures)); @@ -893,18 +742,30 @@ pub(super) async fn complete_operation( "sourceVersion": payload.source_version } }); + // Every key here is paid for once per screen, so on + // allScreens the redundant ones multiply. `imports` and + // `usedTokens` both restate what the tsx beside them + // already spells out - its import line and its `$token`s. let mut frame = json!({ "nodeId": candidate.node.node_id, "name": candidate.node.name, "canonicalUrl": candidate.canonical_url, "status": frame_quality.status(), "quality": frame_quality, - "tsx": output.tsx, - "imports": output.imports, - "usedTokens": output.used_tokens, - "fidelity": output.fidelity_report, - "completenessReport": &completeness_report + "tsx": output.tsx }); + attach_fidelity( + &mut frame, + &output.fidelity_report, + frame_quality.projection, + include_diagnostics, + ); + attach_completeness_report( + &mut frame, + frame_quality, + &completeness_report, + include_diagnostics, + ); if outputs.iter().any(|output| output == "sourceMap") { frame["sourceMap"] = source_map; } @@ -965,11 +826,6 @@ pub(super) async fn complete_operation( pending_text_outputs.insert("responsiveTsx".to_owned(), module.clone()); } result.insert("responsiveTsx".to_owned(), json!(module)); - result.insert("responsiveImports".to_owned(), json!(merged.primitives())); - result.insert( - "responsiveComponents".to_owned(), - json!(merged.referenced_components()), - ); result.insert("responsiveSlots".to_owned(), json!(merged.slots)); if !merged.unrepresented.is_empty() { result.insert( @@ -1016,9 +872,6 @@ pub(super) async fn complete_operation( pending_text_outputs.insert("tsx".to_owned(), output.tsx.clone()); } result.insert("tsx".to_owned(), json!(output.tsx)); - result.insert("imports".to_owned(), json!(output.imports)); - result.insert("usedTokens".to_owned(), json!(output.used_tokens)); - result.insert("fidelity".to_owned(), json!(output.fidelity_report)); if include_diagnostics { result.insert("diagnostics".to_owned(), json!(&output.diagnostics)); } @@ -1049,7 +902,6 @@ pub(super) async fn complete_operation( pending_text_outputs.insert("componentTsx".to_owned(), output.tsx.clone()); } result.insert("componentTsx".to_owned(), json!(output.tsx)); - result.insert("componentImports".to_owned(), json!(output.imports)); } if outputs.iter().any(|output| output == "devupJson") { @@ -1308,15 +1160,19 @@ pub(super) async fn complete_operation( let final_status = quality.status(); result.insert("status".to_owned(), json!(final_status)); result.insert("quality".to_owned(), json!(quality)); + // An unambiguous "this is the answer, implement from it" marker. + // It was removed once on the reasoning that the `needs_figma` + // handoff it guarded against is gone, so it only restated + // `status`. A consumer reported relying on it, which settles it: + // `status: "complete"` says the run went well, and this says + // which value is the deliverable. 109 bytes for that is cheap. + // + // Checked before `apply_delivery` may move `tsx` into + // `resources`, so it reflects whether a devup-ui TSX was + // produced rather than how it was routed for delivery. let tsx_produced = section_tsx_projected || outputs.iter().any(|output| output == "tsx"); if final_status == "complete" && tsx_produced { - // Same unambiguous final-answer marker as devup_figma_to_ui - // — see that branch's comment for why this exists. Checked - // here (before `apply_delivery` may move `tsx`/each frame's - // `tsx` into `resources`) so the marker reflects whether a - // devup-ui TSX was actually produced, independent of how - // large output routed it for delivery. result.insert( "deliverable".to_owned(), json!({ @@ -1326,6 +1182,30 @@ pub(super) async fn complete_operation( }), ); } + // Only the whole-node tsx is missing its fidelity report at this + // point; each Section frame already carries its own. + if !section_tsx_projected && let Some(report) = fidelity_reports.first() { + let mut carrier = Value::Object(Map::new()); + attach_fidelity( + &mut carrier, + report, + quality.projection, + include_diagnostics, + ); + if let Some(fidelity) = carrier.get("fidelity") { + result.insert("fidelity".to_owned(), fidelity.clone()); + } + } + let mut carrier = Value::Object(Map::new()); + attach_completeness_report( + &mut carrier, + quality, + &completeness_report, + include_diagnostics, + ); + if let Some(report) = carrier.get("completenessReport") { + result.insert("completenessReport".to_owned(), report.clone()); + } // The code and the bytes have to name an asset alike. Renaming // here, once both are assembled, keeps the two in step while the // code generator itself stays byte-for-byte the plugin's. diff --git a/crates/devup-mcp/src/server/tools.rs b/crates/devup-mcp/src/server/tools.rs index 4e3acf7..9b7c3eb 100644 --- a/crates/devup-mcp/src/server/tools.rs +++ b/crates/devup-mcp/src/server/tools.rs @@ -2,17 +2,16 @@ use rmcp::schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; -/// `action` is `status`, `login`, `logout`, `configure`, or `doctor`. -/// `doctor` never touches OAuth state; it measures which connection paths -/// (direct OAuth, host handoff) are currently usable -/// and returns client-specific setup guidance. `configure` persists a -/// pre-registered client credential (`clientId`, optional `clientSecret`) -/// so later `login` calls skip Dynamic Client Registration entirely; the -/// secret is stored in the OS credential store and never echoed back. See -/// `server::diagnostics`. +/// `doctor` never touches OAuth state; it measures whether the direct +/// connection is usable right now and returns client-specific setup +/// guidance. `configure` persists a pre-registered client credential +/// (`clientId`, optional `clientSecret`) so later `login` calls skip Dynamic +/// Client Registration entirely; the secret is stored in the OS credential +/// store and never echoed back. See `server::diagnostics`. #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct AuthInput { + #[schemars(extend("enum" = super::validation::AUTH_ACTIONS))] pub action: String, #[serde(default)] pub client_id: Option, @@ -20,42 +19,6 @@ pub struct AuthInput { pub client_secret: Option, } -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -#[serde(rename_all = "camelCase")] -pub struct FigmaToUiInput { - pub url: String, - #[serde(default)] - pub component_name: Option, - #[serde(default)] - pub include_diagnostics: bool, - #[serde(default = "default_source_policy")] - pub source_policy: String, - #[serde(default = "default_scope")] - pub scope: String, - #[serde(default = "default_root_layout")] - pub root_layout: String, - #[serde(default)] - pub output_path: Option, - #[serde(default = "default_delivery")] - pub delivery: String, -} - -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -#[serde(rename_all = "camelCase")] -pub struct FigmaToJsonInput { - pub url: String, - #[serde(default = "default_scope")] - pub scope: String, - #[serde(default)] - pub include_diagnostics: bool, - #[serde(default = "default_source_policy")] - pub source_policy: String, - #[serde(default)] - pub output_path: Option, - #[serde(default = "default_delivery")] - pub delivery: String, -} - #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct FigmaExportInput { @@ -73,11 +36,11 @@ pub struct FigmaExportInput { pub component_name: Option, #[serde(default)] pub include_diagnostics: bool, - #[serde(default = "default_source_policy")] - pub source_policy: String, #[serde(default = "default_scope")] + #[schemars(extend("enum" = super::validation::COLLECTION_SCOPES))] pub scope: String, #[serde(default = "default_root_layout")] + #[schemars(extend("enum" = super::validation::ROOT_LAYOUTS))] pub root_layout: String, /// Name every asset after the node it came from rather than after its /// layer, so two drawings a designer named alike get a file each, and a @@ -106,6 +69,7 @@ pub struct FigmaExportInput { #[serde(default)] pub asset_requests: Vec, #[serde(default = "default_delivery")] + #[schemars(extend("enum" = super::validation::DELIVERY_MODES))] pub delivery: String, } @@ -114,7 +78,7 @@ pub struct FigmaExportInput { pub struct FigmaAssetRequestInput { pub asset_id: String, #[serde(default = "default_asset_format")] - #[schemars(extend("enum" = ["png", "jpg", "svg", "pdf"]))] + #[schemars(extend("enum" = super::validation::ASSET_FORMATS))] pub format: String, #[serde(default = "default_asset_scale")] pub scale: u8, @@ -130,11 +94,10 @@ pub struct FigmaSearchInput { #[serde(default)] pub node_types: Vec, #[serde(default = "default_match", rename = "match")] + #[schemars(extend("enum" = super::validation::SEARCH_MATCH_KINDS))] pub match_kind: String, #[serde(default = "default_limit")] pub limit: usize, - #[serde(default = "default_source_policy")] - pub source_policy: String, } #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] @@ -145,8 +108,6 @@ pub struct FigmaExploreInput { pub limit: usize, #[serde(default = "default_true")] pub include_text_preview: bool, - #[serde(default = "default_source_policy")] - pub source_policy: String, #[serde(default)] pub refresh: bool, } @@ -159,6 +120,7 @@ pub struct FigmaExploreInput { #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct ProjectContextInput { + #[schemars(extend("enum" = super::validation::PROJECT_CONTEXT_SCOPES))] pub scope: String, #[serde(default)] pub project_root: Option, @@ -191,6 +153,10 @@ pub struct StackDiffInput { #[serde(default)] pub project_root: Option, #[serde(default)] + #[schemars(extend("items" = serde_json::json!({ + "type": "string", + "enum": super::validation::STACK_DIFF_LAYERS, + })))] pub layers: Vec, } @@ -202,10 +168,6 @@ fn default_root_layout() -> String { "standalone".to_owned() } -fn default_source_policy() -> String { - "auto".to_owned() -} - fn default_delivery() -> String { "auto".to_owned() } diff --git a/crates/devup-mcp/src/server/validation.rs b/crates/devup-mcp/src/server/validation.rs index 3b1b52a..dc5bf0f 100644 --- a/crates/devup-mcp/src/server/validation.rs +++ b/crates/devup-mcp/src/server/validation.rs @@ -1,7 +1,6 @@ use devup_mcp_devup_ui::codegen::RootLayout; use devup_mcp_figma::{ AssetFormat, AssetSelection, CollectionScope, DevupError, ErrorCode, ResourceScope, - SourcePolicy, }; use serde_json::json; @@ -10,11 +9,35 @@ use super::{ tools::FigmaAssetRequestInput, }; -/// The export outputs this server understands. +/// The values each closed-set string input accepts. /// -/// The JSON schema for `outputs` advertises this same constant, so a caller -/// can discover the set instead of learning it one rejection at a time, and -/// the published schema cannot drift from what is actually accepted. +/// Every one of these is advertised by the tool's JSON schema and consumed by +/// the parser below, so a caller discovers the set instead of learning it one +/// rejection at a time, and the published schema cannot drift from what is +/// actually accepted. `outputs` and the asset `format` were the only two +/// inputs that did this; the rest arrived as bare strings, which left an +/// agent to guess `scope` and `delivery` and find out by being refused. +pub(crate) const AUTH_ACTIONS: [&str; 5] = ["status", "login", "logout", "configure", "doctor"]; + +pub(crate) const COLLECTION_SCOPES: [&str; 3] = ["node", "page", "file"]; + +pub(crate) const ROOT_LAYOUTS: [&str; 2] = ["standalone", "embedded"]; + +pub(crate) const DELIVERY_MODES: [&str; 3] = ["auto", "inline", "resource"]; + +pub(crate) const SEARCH_MATCH_KINDS: [&str; 3] = ["exact", "normalized", "fuzzy"]; + +pub(crate) const ASSET_FORMATS: [&str; 4] = ["png", "jpg", "svg", "pdf"]; + +pub(crate) const PROJECT_CONTEXT_SCOPES: [&str; 4] = ["theme", "api", "db", "all"]; + +pub(crate) const STACK_DIFF_LAYERS: [&str; 4] = [ + "db-entity", + "entity-route", + "route-openapi", + "openapi-client", +]; + pub(crate) const EXPORT_OUTPUTS: [&str; 9] = [ "tsx", "componentTsx", @@ -103,7 +126,7 @@ fn collection_scope_rank(scope: CollectionScope) -> u8 { pub(super) fn validate_outputs(outputs: &[String]) -> Result<(), DevupError> { if outputs.is_empty() { return Err(DevupError::new( - ErrorCode::DevupSnapshotUnsupported, + ErrorCode::DevupInvalidInput, "outputs must contain at least one entry.", false, )); @@ -111,7 +134,7 @@ pub(super) fn validate_outputs(outputs: &[String]) -> Result<(), DevupError> { for output in outputs { if !EXPORT_OUTPUTS.contains(&output.as_str()) { return Err(DevupError::new( - ErrorCode::DevupSnapshotUnsupported, + ErrorCode::DevupInvalidInput, format!( "Unsupported export output: {output}. Supported: {}.", EXPORT_OUTPUTS.join(", ") @@ -123,19 +146,6 @@ pub(super) fn validate_outputs(outputs: &[String]) -> Result<(), DevupError> { Ok(()) } -pub(super) fn parse_source_policy(policy: &str) -> Result { - match policy { - "auto" => Ok(SourcePolicy::Auto), - "direct" => Ok(SourcePolicy::Direct), - - _ => Err(DevupError::new( - ErrorCode::DevupInvalidInput, - "sourcePolicy must be auto or direct.", - false, - )), - } -} - pub(super) fn parse_asset_requests( requests: &[FigmaAssetRequestInput], ) -> Result< @@ -147,7 +157,7 @@ pub(super) fn parse_asset_requests( > { if requests.len() > 16 { return Err(DevupError::new( - ErrorCode::DevupSnapshotUnsupported, + ErrorCode::DevupInvalidInput, "At most 16 assets can be exported at once.", false, )); @@ -162,7 +172,7 @@ pub(super) fn parse_asset_requests( || !seen.insert(request.asset_id.as_str()) { return Err(DevupError::new( - ErrorCode::DevupSnapshotUnsupported, + ErrorCode::DevupInvalidInput, "An assetRequests ID, scale, or duplicate entry is invalid.", false, )); @@ -174,8 +184,8 @@ pub(super) fn parse_asset_requests( "pdf" => AssetFormat::Pdf, _ => { return Err(DevupError::new( - ErrorCode::DevupSnapshotUnsupported, - "asset format must be png, jpg, svg, or pdf.", + ErrorCode::DevupInvalidInput, + format!("asset format must be one of: {}.", ASSET_FORMATS.join(", ")), false, )); } @@ -192,14 +202,19 @@ pub(super) fn parse_asset_requests( Ok((selections, output_paths)) } +// A bad `scope` or `rootLayout` used to be reported as DEVUP_THEME_CONFLICT, +// which names a real condition - two collections defining one token - and has +// nothing to do with a misspelled argument. Both are DEVUP_INVALID_INPUT now, +// so `is_caller_mistake` can route them to INVALID_PARAMS without dragging +// genuine theme conflicts along with them. pub(super) fn parse_collection_scope(scope: &str) -> Result { match scope { "node" => Ok(CollectionScope::Node), "page" => Ok(CollectionScope::Page), "file" => Ok(CollectionScope::File), _ => Err(DevupError::new( - ErrorCode::DevupThemeConflict, - "scope must be node, page, or file.", + ErrorCode::DevupInvalidInput, + format!("scope must be one of: {}.", COLLECTION_SCOPES.join(", ")), false, )), } @@ -210,8 +225,8 @@ pub(super) fn parse_root_layout(root_layout: &str) -> Result Ok(RootLayout::Standalone), "embedded" => Ok(RootLayout::Embedded), _ => Err(DevupError::new( - ErrorCode::DevupThemeConflict, - "rootLayout must be standalone or embedded.", + ErrorCode::DevupInvalidInput, + format!("rootLayout must be one of: {}.", ROOT_LAYOUTS.join(", ")), false, )), } diff --git a/crates/devup-mcp/tests/artifact_cache.rs b/crates/devup-mcp/tests/artifact_cache.rs index 1f9b9e6..c32d837 100644 --- a/crates/devup-mcp/tests/artifact_cache.rs +++ b/crates/devup-mcp/tests/artifact_cache.rs @@ -13,7 +13,7 @@ use devup_mcp::server::artifacts::{ use devup_mcp_figma::{ AssetFormat, AssetSelection, CollectedPayload, CollectionRequest, CollectionScope, CollectionStats, ExploreReadOptions, FigmaTarget, PayloadCompleteness, RawNode, ResourceScope, - SearchReadOptions, SectionReadOptions, Snapshot, SourcePolicy, + SearchReadOptions, SectionReadOptions, Snapshot, }; use serde_json::json; @@ -123,7 +123,7 @@ async fn artifact_lookup_preserves_capture_capabilities() -> anyhow::Result<()> let design_request = request("design-file", "1:1"); let design = store .insert( - ArtifactRequestKey::from_collection(&design_request, SourcePolicy::Direct), + ArtifactRequestKey::from_collection(&design_request), payload("design-file", "1:1", "design"), ) .await?; @@ -137,7 +137,7 @@ async fn artifact_lookup_preserves_capture_capabilities() -> anyhow::Result<()> theme_request.variables_only = true; let theme = store .insert( - ArtifactRequestKey::from_collection(&theme_request, SourcePolicy::Direct), + ArtifactRequestKey::from_collection(&theme_request), payload("theme-file", "2:2", "theme"), ) .await?; @@ -155,7 +155,7 @@ async fn artifact_lookup_preserves_capture_capabilities() -> anyhow::Result<()> }); let search = store .insert( - ArtifactRequestKey::from_collection(&search_request, SourcePolicy::Direct), + ArtifactRequestKey::from_collection(&search_request), payload("search-file", "3:3", "search"), ) .await?; @@ -166,7 +166,7 @@ async fn artifact_lookup_preserves_capture_capabilities() -> anyhow::Result<()> explore_request.explore = Some(ExploreReadOptions::default()); let explore = store .insert( - ArtifactRequestKey::from_collection(&explore_request, SourcePolicy::Direct), + ArtifactRequestKey::from_collection(&explore_request), payload("explore-file", "4:4", "explore"), ) .await?; @@ -179,8 +179,7 @@ async fn reuses_same_request_until_expiry_and_refresh_bypasses_it() -> anyhow::R let clock = Arc::new(FakeClock::default()); clock.set(100); let store = ArtifactStore::with_clock(clock.clone(), limits()); - let key = - ArtifactRequestKey::from_collection(&request("file-one", "1:2"), SourcePolicy::Direct); + let key = ArtifactRequestKey::from_collection(&request("file-one", "1:2")); let calls = AtomicUsize::new(0); let first = store @@ -240,23 +239,20 @@ async fn evicts_lru_entries_by_count_and_aggregate_bytes() -> anyhow::Result<()> let one = store .insert( - ArtifactRequestKey::from_collection(&request("file-one", "1:1"), SourcePolicy::Direct), + ArtifactRequestKey::from_collection(&request("file-one", "1:1")), payload("file-one", "1:1", "a"), ) .await?; let two = store .insert( - ArtifactRequestKey::from_collection(&request("file-two", "2:2"), SourcePolicy::Direct), + ArtifactRequestKey::from_collection(&request("file-two", "2:2")), payload("file-two", "2:2", "b"), ) .await?; store.get(&one.artifact_id).await.expect("touch first"); let three = store .insert( - ArtifactRequestKey::from_collection( - &request("file-three", "3:3"), - SourcePolicy::Direct, - ), + ArtifactRequestKey::from_collection(&request("file-three", "3:3")), payload("file-three", "3:3", "c"), ) .await?; @@ -273,8 +269,7 @@ async fn evicts_lru_entries_by_count_and_aggregate_bytes() -> anyhow::Result<()> #[tokio::test] async fn concurrent_same_key_requests_share_one_acquisition() -> anyhow::Result<()> { let store = ArtifactStore::with_limits(limits()); - let key = - ArtifactRequestKey::from_collection(&request("file-one", "1:2"), SourcePolicy::Direct); + let key = ArtifactRequestKey::from_collection(&request("file-one", "1:2")); let calls = Arc::new(AtomicUsize::new(0)); let barrier = Arc::new(tokio::sync::Barrier::new(3)); @@ -309,14 +304,8 @@ async fn concurrent_same_key_requests_share_one_acquisition() -> anyhow::Result< #[tokio::test] async fn concurrent_related_explore_waits_for_one_compatible_acquisition() -> anyhow::Result<()> { let store = ArtifactStore::with_limits(limits()); - let owner_key = ArtifactRequestKey::from_collection( - &explore_request("file-one", "1:1", 200), - SourcePolicy::Direct, - ); - let follower_key = ArtifactRequestKey::from_collection( - &explore_request("file-one", "1:2", 50), - SourcePolicy::Direct, - ); + let owner_key = ArtifactRequestKey::from_collection(&explore_request("file-one", "1:1", 200)); + let follower_key = ArtifactRequestKey::from_collection(&explore_request("file-one", "1:2", 50)); let calls = Arc::new(AtomicUsize::new(0)); let started = Arc::new(tokio::sync::Notify::new()); let release = Arc::new(tokio::sync::Notify::new()); @@ -364,14 +353,8 @@ async fn concurrent_related_explore_waits_for_one_compatible_acquisition() -> an #[tokio::test] async fn concurrent_uncovered_explore_falls_through_to_its_own_acquisition() -> anyhow::Result<()> { let store = ArtifactStore::with_limits(limits()); - let owner_key = ArtifactRequestKey::from_collection( - &explore_request("file-one", "1:1", 200), - SourcePolicy::Direct, - ); - let follower_key = ArtifactRequestKey::from_collection( - &explore_request("file-one", "9:9", 50), - SourcePolicy::Direct, - ); + let owner_key = ArtifactRequestKey::from_collection(&explore_request("file-one", "1:1", 200)); + let follower_key = ArtifactRequestKey::from_collection(&explore_request("file-one", "9:9", 50)); let calls = Arc::new(AtomicUsize::new(0)); let started = Arc::new(tokio::sync::Notify::new()); let release = Arc::new(tokio::sync::Notify::new()); @@ -419,8 +402,7 @@ async fn concurrent_uncovered_explore_falls_through_to_its_own_acquisition() -> #[tokio::test] async fn cancelled_owner_does_not_poison_later_same_key_acquisitions() -> anyhow::Result<()> { let store = ArtifactStore::with_limits(limits()); - let key = - ArtifactRequestKey::from_collection(&request("file-one", "1:2"), SourcePolicy::Direct); + let key = ArtifactRequestKey::from_collection(&request("file-one", "1:2")); let started = Arc::new(tokio::sync::Notify::new()); let release = Arc::new(tokio::sync::Notify::new()); @@ -507,7 +489,7 @@ async fn set_like_capture_and_section_inputs_share_one_cache_key() -> anyhow::Re let first = store .get_or_acquire( - ArtifactRequestKey::from_collection(&left, SourcePolicy::Direct), + ArtifactRequestKey::from_collection(&left), false, || async { calls.fetch_add(1, Ordering::SeqCst); @@ -517,7 +499,7 @@ async fn set_like_capture_and_section_inputs_share_one_cache_key() -> anyhow::Re .await?; let second = store .get_or_acquire( - ArtifactRequestKey::from_collection(&right, SourcePolicy::Direct), + ArtifactRequestKey::from_collection(&right), false, || async { calls.fetch_add(1, Ordering::SeqCst); @@ -537,10 +519,8 @@ async fn safe_keys_and_stats_never_serialize_url_credentials_or_payloads() -> an let target = FigmaTarget::parse( "https://www.figma.com/design/file-one/Example?node-id=1-2&access_token=super-secret", )?; - let key = ArtifactRequestKey::from_collection( - &CollectionRequest::new(target, CollectionScope::Node), - SourcePolicy::Auto, - ); + let key = + ArtifactRequestKey::from_collection(&CollectionRequest::new(target, CollectionScope::Node)); let key_json = serde_json::to_string(&key)?; assert!(!key_json.contains("super-secret")); assert!(!key_json.contains("access_token")); diff --git a/crates/devup-mcp/tests/call_cache_resume.rs b/crates/devup-mcp/tests/call_cache_resume.rs index 3c6257c..32d549b 100644 --- a/crates/devup-mcp/tests/call_cache_resume.rs +++ b/crates/devup-mcp/tests/call_cache_resume.rs @@ -92,8 +92,7 @@ async fn attempt(upstream: Arc, cache: PathBuf) -> anyhow::Res let arguments: Map = json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=10-1", "allScreens": true, - "outputs": ["rawSnapshot"], - "sourcePolicy": "direct" + "outputs": ["rawSnapshot"] }) .as_object() .cloned() @@ -210,8 +209,7 @@ async fn a_refusal_that_arrives_as_an_answer_is_not_banked() -> anyhow::Result<( let arguments: Map = json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=10-1", "allScreens": true, - "outputs": ["rawSnapshot"], - "sourcePolicy": "direct" + "outputs": ["rawSnapshot"] }) .as_object() .cloned() @@ -253,8 +251,7 @@ async fn nothing_is_banked_unless_a_directory_was_named() -> anyhow::Result<()> let arguments: Map = json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=10-1", "allScreens": true, - "outputs": ["rawSnapshot"], - "sourcePolicy": "direct" + "outputs": ["rawSnapshot"] }) .as_object() .cloned() diff --git a/crates/devup-mcp/tests/composite_export.rs b/crates/devup-mcp/tests/composite_export.rs index 571a668..b21bb78 100644 --- a/crates/devup-mcp/tests/composite_export.rs +++ b/crates/devup-mcp/tests/composite_export.rs @@ -123,8 +123,7 @@ async fn reference_png_is_acquired_once_and_delivered_as_a_binary_resource() -> "devup_figma_export", json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", - "outputs": ["referencePng"], - "sourcePolicy": "direct" + "outputs": ["referencePng"] }), ) .await?; @@ -209,6 +208,54 @@ async fn call_result( .await?) } +/// A clean conversion should carry the code and the grade, and not the +/// paperwork underneath the grade. +/// +/// `fidelity` and `completenessReport` are the drill-down under `quality`: +/// on a clean run the first reports 100% across six axes and the second six +/// empty arrays, so both only repeat what `quality` already said. Measured on +/// this fixture they were 797 bytes of the 1,911 every response carried. +/// They are still sent whenever they disagree with a clean grade, and +/// whenever the caller asks for diagnostics - which the test above covers. +#[tokio::test] +async fn a_clean_conversion_sends_the_code_and_the_grade_but_not_the_paperwork() +-> anyhow::Result<()> { + let upstream = Arc::new(FastFixtureUpstream::complete()); + let server = DevupServer::new(Services::new(Arc::new(ConnectedAuth), upstream)); + let (server_transport, client_transport) = tokio::io::duplex(256 * 1024); + let task = tokio::spawn(async move { + server.serve(server_transport).await?.waiting().await?; + anyhow::Ok(()) + }); + let client = ().serve(client_transport).await?; + + let result = call( + &client, + "devup_figma_export", + json!({ + "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", + "outputs": ["tsx"], + "scope": "node" + }), + ) + .await?; + + assert_eq!(result["status"], "complete"); + assert_eq!(result["quality"]["projection"], "exact"); + assert_eq!(result["quality"]["acquisition"], "complete"); + assert!(result["tsx"].as_str().unwrap().contains("$primary")); + for silent in ["fidelity", "completenessReport"] { + assert!( + result.get(silent).is_none(), + "{silent} says nothing a clean quality has not already said" + ); + } + + client.cancel().await?; + task.await??; + Ok(()) +} + #[tokio::test] async fn one_acquisition_projects_all_outputs_and_artifact_reuse_is_zero_call() -> anyhow::Result<()> { @@ -229,7 +276,6 @@ async fn one_acquisition_projects_all_outputs_and_artifact_reuse_is_zero_call() "url": url, "outputs": ["tsx", "devupJson", "rawSnapshot", "rawPayload", "sourceMap", "assetManifest"], "scope": "node", - "sourcePolicy": "direct", "includeDiagnostics": true }), ) @@ -246,11 +292,27 @@ async fn one_acquisition_projects_all_outputs_and_artifact_reuse_is_zero_call() assert_eq!(first["cache"]["cacheHit"], false); assert!(first["cache"]["artifactId"].as_str().is_some()); assert!(first["tsx"].as_str().unwrap().contains("$primary")); - // devup_figma_export must carry the same unambiguous final-answer - // marker as devup_figma_to_ui when it actually produced a tsx output. + assert_eq!(first["status"], "complete"); + assert_eq!(first["quality"]["projection"], "exact"); + // This call asks for diagnostics, so the drill-down under `quality` is + // exactly what it should get. + assert!(first["fidelity"].is_object()); + assert!(first["completenessReport"].is_object()); + // `status` says the run went well; `deliverable` says which value is the + // answer to implement from. A consumer reported relying on it, so it is + // sent whenever a tsx was actually produced. assert_eq!(first["deliverable"]["kind"], "devup-ui-tsx"); assert_eq!(first["deliverable"]["isFinal"], true); assert!(!first["deliverable"]["note"].as_str().unwrap().is_empty()); + // These restate something already in the response whether diagnostics + // were asked for or not: `imports` and `usedTokens` restate the tsx's own + // import line and its `$token`s. Neither is sent any more. + for restated in ["imports", "usedTokens", "componentImports"] { + assert!( + first.get(restated).is_none(), + "{restated} restates something already in the response and must not be sent" + ); + } assert!(first["devupJson"].as_str().unwrap().contains("\"primary\"")); assert_eq!(first["rawSnapshot"]["roots"], json!(["1:2"])); assert_eq!(first["sourceMap"]["version"], 1); @@ -320,16 +382,20 @@ async fn one_acquisition_projects_all_outputs_and_artifact_reuse_is_zero_call() assert_eq!(manifest["mimeType"], "text/typescript"); assert_eq!(upstream.calls.load(Ordering::SeqCst), 1); + // The same acquisition answers a tsx-only and a theme-only projection + // without going back to Figma, which is what devup_figma_to_ui and + // devup_figma_to_json used to be for before they were folded into this + // one tool as `outputs`. let ui_wrapper = call( &client, - "devup_figma_to_ui", - json!({"url": url, "sourcePolicy": "direct"}), + "devup_figma_export", + json!({"url": url, "outputs": ["tsx"]}), ) .await?; let json_wrapper = call( &client, - "devup_figma_to_json", - json!({"url": url, "scope": "node", "sourcePolicy": "direct"}), + "devup_figma_export", + json!({"url": url, "outputs": ["devupJson"], "scope": "node"}), ) .await?; assert_eq!(ui_wrapper["cache"]["cacheHit"], true); @@ -343,7 +409,6 @@ async fn one_acquisition_projects_all_outputs_and_artifact_reuse_is_zero_call() "url": url, "outputs": ["rawSnapshot"], "scope": "node", - "sourcePolicy": "direct", "refresh": true }), ) @@ -373,8 +438,7 @@ async fn artifact_reuse_rejects_file_theme_beyond_captured_scope() -> anyhow::Re json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", "outputs": ["tsx"], - "scope": "node", - "sourcePolicy": "direct" + "scope": "node" }), ) .await?; @@ -424,7 +488,6 @@ async fn explicit_asset_request_exports_once_and_returns_validated_binary() -> a json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", "outputs": ["tsx", "assetManifest"], - "sourcePolicy": "direct", "assetRequests": [{"assetId":"1:3:fills:0","format":"png","scale":2}] }), ) @@ -463,7 +526,6 @@ async fn resource_asset_manifest_reconstructs_the_exact_independent_binary() -> json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", "outputs": ["assetManifest"], - "sourcePolicy": "direct", "delivery": "resource", "assetRequests": [{ "assetId":"1:3:fills:0", @@ -536,7 +598,6 @@ async fn resource_asset_manifest_reconstructs_the_exact_independent_binary() -> json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", "outputs": ["assetManifest"], - "sourcePolicy": "direct", "delivery": "resource", "assetRequests": [{ "assetId":"1:3:fills:0", @@ -665,7 +726,6 @@ async fn artifact_reuse_rejects_a_different_asset_format_or_scale() -> anyhow::R json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", "outputs": ["assetManifest"], - "sourcePolicy": "direct", "assetRequests": [{"assetId":"1:3:fills:0","format":"png","scale":2}] }), ) @@ -723,7 +783,6 @@ async fn strict_export_rejects_partial_payload_before_projection() -> anyhow::Re json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", "outputs": ["rawSnapshot"], - "sourcePolicy": "direct", "strict": true }) .as_object() @@ -767,7 +826,6 @@ async fn strict_tsx_export_rejects_lossy_projection() -> anyhow::Result<()> { json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", "outputs": ["tsx"], - "sourcePolicy": "direct", "strict": true, "outputPaths": {"tsx": output_path.to_string_lossy()} }) diff --git a/crates/devup-mcp/tests/downstream_integration.rs b/crates/devup-mcp/tests/downstream_integration.rs index f975e97..3d73194 100644 --- a/crates/devup-mcp/tests/downstream_integration.rs +++ b/crates/devup-mcp/tests/downstream_integration.rs @@ -267,8 +267,11 @@ async fn conversion_asks_to_be_logged_in_rather_than_starting_oauth() -> anyhow: let auth = Arc::new(LoginAuth::default()); let error = call_tool_with_auth( auth.clone(), - "devup_figma_to_ui", - json!({"url": "https://figma.com/design/85CgSws3o5XsLv7aAwWJyS/Name?node-id=3879-35481"}), + "devup_figma_export", + json!({ + "url": "https://figma.com/design/85CgSws3o5XsLv7aAwWJyS/Name?node-id=3879-35481", + "outputs": ["tsx"] + }), ) .await .expect_err("a disconnected direct path cannot convert"); @@ -284,9 +287,10 @@ async fn conversion_asks_to_be_logged_in_rather_than_starting_oauth() -> anyhow: #[tokio::test] async fn converts_a_figma_link_to_structured_devup_ui() -> anyhow::Result<()> { let result = call_tool( - "devup_figma_to_ui", + "devup_figma_export", json!({ "url": "https://www.figma.com/design/85CgSws3o5XsLv7aAwWJyS/Name?node-id=3879-35481", + "outputs": ["tsx"], "includeDiagnostics": true }), ) @@ -307,7 +311,10 @@ async fn converts_a_figma_link_to_structured_devup_ui() -> anyhow::Result<()> { assert!(tsx.contains("bg=\"$primary\"")); assert!(!tsx.contains("$colorPrimary")); assert_eq!(result["source"]["nodeId"], "3879:35481"); - assert_eq!(result["snapshot"]["preservedNodeCount"], 1); + assert_eq!( + result["completenessReport"]["snapshot"]["preservedNodeCount"], + 1 + ); Ok(()) } @@ -316,9 +323,10 @@ async fn reports_partial_instead_of_complete_when_a_child_is_missing() -> anyhow let result = call_tool_with_services( Arc::new(ConnectedAuth), Arc::new(PartialFixtureUpstream), - "devup_figma_to_ui", + "devup_figma_export", json!({ "url": "https://www.figma.com/design/85CgSws3o5XsLv7aAwWJyS/Name?node-id=3879-35481", + "outputs": ["tsx"], "includeDiagnostics": true }), ) @@ -345,12 +353,13 @@ async fn converts_figma_variables_to_structured_devup_json() -> anyhow::Result<( std::fs::create_dir_all(&output_root)?; let output_path = output_root.join("devup.json"); let result = call_tool_with_output_roots( - "devup_figma_to_json", + "devup_figma_export", json!({ "url": "https://www.figma.com/design/85CgSws3o5XsLv7aAwWJyS/Name?node-id=3879-35481", + "outputs": ["devupJson"], "scope": "file", "includeDiagnostics": true, - "outputPath": output_path + "outputPaths": {"devupJson": output_path} }), vec![output_root.clone()], ) @@ -374,7 +383,7 @@ async fn converts_figma_variables_to_structured_devup_json() -> anyhow::Result<( std::fs::read_to_string(&output_path)?, result["devupJson"].as_str().unwrap() ); - assert!(result["outputPath"].as_str().is_some()); + assert!(result["outputPaths"]["devupJson"].as_str().is_some()); std::fs::remove_file(output_path)?; std::fs::remove_dir(output_root)?; Ok(()) @@ -383,9 +392,10 @@ async fn converts_figma_variables_to_structured_devup_json() -> anyhow::Result<( #[tokio::test] async fn node_theme_scope_excludes_file_variables_not_used_by_the_node() -> anyhow::Result<()> { let result = call_tool( - "devup_figma_to_json", + "devup_figma_export", json!({ "url": "https://www.figma.com/design/85CgSws3o5XsLv7aAwWJyS/Name?node-id=3879-35481", + "outputs": ["devupJson"], "scope": "node", "includeDiagnostics": true }), diff --git a/crates/devup-mcp/tests/figma_explore.rs b/crates/devup-mcp/tests/figma_explore.rs index 82cc59b..eb8f224 100644 --- a/crates/devup-mcp/tests/figma_explore.rs +++ b/crates/devup-mcp/tests/figma_explore.rs @@ -131,12 +131,11 @@ async fn start_client( Ok((client, task)) } -fn input(source_policy: &str) -> Map { +fn input() -> Map { json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-1", "limit": 50, - "includeTextPreview": true, - "sourcePolicy": source_policy + "includeTextPreview": true }) .as_object() .cloned() @@ -161,13 +160,11 @@ async fn related_nodes_reuse_one_explore_projection_without_changing_the_request let client = ().serve(client_transport).await?; let heading = client - .call_tool( - CallToolRequestParams::new("devup_figma_explore").with_arguments(input("direct")), - ) + .call_tool(CallToolRequestParams::new("devup_figma_explore").with_arguments(input())) .await? .structured_content .unwrap(); - let mut screen_input = input("direct"); + let mut screen_input = input(); screen_input.insert( "url".to_owned(), json!("https://www.figma.com/design/FileKey123/Fixture?node-id=1-2"), @@ -193,7 +190,7 @@ async fn related_nodes_reuse_one_explore_projection_without_changing_the_request assert_eq!(screen["collection"]["figmaToolCalls"], 0); assert_eq!(calls.load(Ordering::SeqCst), 1); - let mut different_projection = input("direct"); + let mut different_projection = input(); different_projection.insert( "url".to_owned(), json!("https://www.figma.com/design/FileKey123/Fixture?node-id=1-3"), @@ -232,20 +229,16 @@ async fn refresh_bypasses_an_exact_explore_cache_hit() -> anyhow::Result<()> { let client = ().serve(client_transport).await?; let first = client - .call_tool( - CallToolRequestParams::new("devup_figma_explore").with_arguments(input("direct")), - ) + .call_tool(CallToolRequestParams::new("devup_figma_explore").with_arguments(input())) .await? .structured_content .unwrap(); let exact = client - .call_tool( - CallToolRequestParams::new("devup_figma_explore").with_arguments(input("direct")), - ) + .call_tool(CallToolRequestParams::new("devup_figma_explore").with_arguments(input())) .await? .structured_content .unwrap(); - let mut refreshed_input = input("direct"); + let mut refreshed_input = input(); refreshed_input.insert("refresh".to_owned(), json!(true)); let refreshed = client .call_tool( @@ -280,8 +273,7 @@ async fn explore_rejects_missing_node_and_out_of_range_limit() -> anyhow::Result .call_tool( CallToolRequestParams::new("devup_figma_explore").with_arguments( json!({ - "url": "https://www.figma.com/design/FileKey123/Fixture", - "sourcePolicy": "direct" + "url": "https://www.figma.com/design/FileKey123/Fixture" }) .as_object() .cloned() @@ -295,8 +287,7 @@ async fn explore_rejects_missing_node_and_out_of_range_limit() -> anyhow::Result CallToolRequestParams::new("devup_figma_explore").with_arguments( json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-1", - "limit": 101, - "sourcePolicy": "direct" + "limit": 101 }) .as_object() .cloned() diff --git a/crates/devup-mcp/tests/rate_limit_patience.rs b/crates/devup-mcp/tests/rate_limit_patience.rs index b7fd533..0fc0e91 100644 --- a/crates/devup-mcp/tests/rate_limit_patience.rs +++ b/crates/devup-mcp/tests/rate_limit_patience.rs @@ -85,8 +85,7 @@ async fn export(upstream: Arc) -> anyhow::Result = json!({ - "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", - "sourcePolicy": "direct" + "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2" }) .as_object() .cloned() diff --git a/crates/devup-mcp/tests/resource_delivery.rs b/crates/devup-mcp/tests/resource_delivery.rs index b1eac5a..dad0419 100644 --- a/crates/devup-mcp/tests/resource_delivery.rs +++ b/crates/devup-mcp/tests/resource_delivery.rs @@ -18,7 +18,7 @@ use devup_mcp::server::{ }; use devup_mcp_figma::{ CollectedPayload, CollectionRequest, CollectionScope, CollectionStats, FigmaTarget, - PayloadCompleteness, ResourceScope, Snapshot, SourcePolicy, + PayloadCompleteness, ResourceScope, Snapshot, }; use rmcp::model::ResourceContents; use serde_json::json; @@ -157,10 +157,7 @@ async fn attached_outputs_are_bounded_hashed_and_share_artifact_lifetime() -> an }); let request = request(); let artifact = store - .insert( - ArtifactRequestKey::from_collection(&request, SourcePolicy::Direct), - payload(), - ) + .insert(ArtifactRequestKey::from_collection(&request), payload()) .await?; let acquisition_hash = artifact.content_hash.clone(); let bytes = vec![0x5a; RESOURCE_CHUNK_BYTES * 2 + 7]; @@ -236,10 +233,7 @@ async fn resource_protocol_lists_manifests_and_round_trips_chunks() -> anyhow::R max_total_bytes: 8 * 1024 * 1024, }); let artifact = store - .insert( - ArtifactRequestKey::from_collection(&request(), SourcePolicy::Direct), - payload(), - ) + .insert(ArtifactRequestKey::from_collection(&request()), payload()) .await?; let original = "€€€".repeat(100_000).into_bytes(); let attached = store @@ -315,10 +309,7 @@ async fn reserved_resources_stay_invisible_until_publication() -> anyhow::Result max_total_bytes: 8 * 1024 * 1024, }); let artifact = store - .insert( - ArtifactRequestKey::from_collection(&request(), SourcePolicy::Direct), - payload(), - ) + .insert(ArtifactRequestKey::from_collection(&request()), payload()) .await?; let root = unique_temp_dir("combined-publication")?; let policy = OutputPolicy::from_roots(vec![root.clone()])?; @@ -386,10 +377,7 @@ async fn failed_file_commit_does_not_publish_or_evict_lru_resources() -> anyhow: max_total_bytes: sample_bytes * 2 + 32, }); let target = store - .insert( - ArtifactRequestKey::from_collection(&request(), SourcePolicy::Direct), - payload(), - ) + .insert(ArtifactRequestKey::from_collection(&request()), payload()) .await?; let mut unrelated_request = request(); unrelated_request.target.file_key = "UnrelatedFile".to_owned(); @@ -398,7 +386,7 @@ async fn failed_file_commit_does_not_publish_or_evict_lru_resources() -> anyhow: unrelated_payload.snapshot.file_key = "UnrelatedFile".to_owned(); let unrelated = store .insert( - ArtifactRequestKey::from_collection(&unrelated_request, SourcePolicy::Direct), + ArtifactRequestKey::from_collection(&unrelated_request), unrelated_payload, ) .await?; diff --git a/crates/devup-mcp/tests/responsive_export.rs b/crates/devup-mcp/tests/responsive_export.rs index b57fd9f..3169a33 100644 --- a/crates/devup-mcp/tests/responsive_export.rs +++ b/crates/devup-mcp/tests/responsive_export.rs @@ -243,7 +243,6 @@ async fn a_screen_with_other_widths_is_exported_as_one_responsive_module() -> an let result = export(json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-20", "outputs": ["tsx"], - "sourcePolicy": "direct", "componentName": "Notice" })) .await?; @@ -293,15 +292,17 @@ async fn a_screen_with_other_widths_is_exported_as_one_responsive_module() -> an assert!(module.contains("} from '@devup-ui/react'")); assert!(module.contains("export default function Notice() {")); assert!(module.trim_end().ends_with('}')); + // The import list used to be repeated back as `responsiveImports` and + // `responsiveComponents`, which is the module's own first line said + // twice. Read it off the module instead, which is the thing that has to + // be right. + let imports = module + .lines() + .next() + .expect("a module opens with its import"); assert_eq!( - result["responsiveImports"], - json!(["Box", "Flex", "Text", "VStack"]), - "only the elements it actually uses" - ); - assert_eq!( - result["responsiveComponents"], - json!([]), - "this screen has no instances, so it imports no components of its own" + imports, "import { Box, Flex, Text, VStack } from '@devup-ui/react'", + "only the elements it actually uses, and no component of its own" ); Ok(()) } @@ -313,7 +314,6 @@ async fn the_responsive_module_can_be_asked_for_on_its_own() -> anyhow::Result<( let result = export(json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-20", "outputs": ["responsiveTsx"], - "sourcePolicy": "direct", "componentName": "Notice" })) .await?; @@ -337,8 +337,7 @@ async fn the_responsive_module_can_be_asked_for_on_its_own() -> anyhow::Result<( async fn the_page_is_named_after_its_section_when_the_caller_gives_no_name() -> anyhow::Result<()> { let result = export(json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-20", - "outputs": ["responsiveTsx"], - "sourcePolicy": "direct" + "outputs": ["responsiveTsx"] })) .await?; assert!( diff --git a/crates/devup-mcp/tests/section_export.rs b/crates/devup-mcp/tests/section_export.rs index 65e46e0..349ab82 100644 --- a/crates/devup-mcp/tests/section_export.rs +++ b/crates/devup-mcp/tests/section_export.rs @@ -80,11 +80,7 @@ async fn section_requires_selection_then_exports_requested_or_all_screens_from_o let client = ().serve(client_transport).await?; let url = "https://www.figma.com/design/FileKey123/Fixture?node-id=10-1"; - let selection = call( - &client, - json!({"url": url, "outputs": ["tsx"], "sourcePolicy": "direct"}), - ) - .await?; + let selection = call(&client, json!({"url": url, "outputs": ["tsx"]})).await?; assert_eq!(selection["status"], "selection_required"); assert_eq!(selection["targetKind"], "section"); assert!(selection.get("tsx").is_none()); @@ -244,8 +240,7 @@ async fn a_thrown_section_error_on_the_direct_path_returns_selectable_screens() &client, json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=10-1", - "outputs": ["tsx"], - "sourcePolicy": "direct" + "outputs": ["tsx"] }), ) .await?; diff --git a/crates/devup-mcp/tests/source_orchestration.rs b/crates/devup-mcp/tests/source_orchestration.rs index c7d4de7..4f522af 100644 --- a/crates/devup-mcp/tests/source_orchestration.rs +++ b/crates/devup-mcp/tests/source_orchestration.rs @@ -121,7 +121,7 @@ async fn call_tool( upstream: Arc, arguments: Value, ) -> anyhow::Result { - call_named_tool(auth, upstream, "devup_figma_to_ui", arguments).await + call_named_tool(auth, upstream, "devup_figma_export", arguments).await } async fn call_named_tool( @@ -181,8 +181,7 @@ async fn search_collects_the_file_and_returns_replayable_node_urls() -> anyhow:: "devup_figma_search", json!({ "url": "https://www.figma.com/design/FileKey123/Fixture", - "query": "syntheticframe", - "sourcePolicy": "direct" + "query": "syntheticframe" }), ) .await?; @@ -216,11 +215,11 @@ async fn ui_output_path_writes_the_generated_artifact_only_when_requested() -> a logins: AtomicUsize::new(0), }), Arc::new(FixtureUpstream::default()), - "devup_figma_to_ui", + "devup_figma_export", json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", - "sourcePolicy": "direct", - "outputPath": path + "outputs": ["tsx"], + "outputPaths": {"tsx": path} }), vec![root.clone()], ) @@ -228,16 +227,16 @@ async fn ui_output_path_writes_the_generated_artifact_only_when_requested() -> a let output = result.structured_content.unwrap(); let written = std::fs::read_to_string(&path)?; assert_eq!(written, output["tsx"].as_str().unwrap()); - assert!(output["outputPath"].as_str().is_some()); + assert!(output["outputPaths"]["tsx"].as_str().is_some()); std::fs::remove_file(path)?; std::fs::remove_dir(root)?; Ok(()) } -fn input(policy: &str) -> Value { +fn input() -> Value { json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", - "sourcePolicy": policy + "outputs": ["tsx"] }) } @@ -282,7 +281,7 @@ async fn direct_disconnected_never_starts_oauth() -> anyhow::Result<()> { logins: AtomicUsize::new(0), }); let upstream = Arc::new(UpstreamProbe::unavailable()); - let result = call_tool(auth.clone(), upstream.clone(), input("direct")).await; + let result = call_tool(auth.clone(), upstream.clone(), input()).await; assert!(result.is_err()); assert_eq!(auth.logins.load(Ordering::SeqCst), 0); @@ -297,12 +296,11 @@ async fn connected_auto_completes_through_the_direct_collector() -> anyhow::Resu logins: AtomicUsize::new(0), }); let upstream = Arc::new(FixtureUpstream::default()); - let result = call_tool(auth.clone(), upstream.clone(), input("auto")).await?; + let result = call_tool(auth.clone(), upstream.clone(), input()).await?; let output = result.structured_content.unwrap(); assert_eq!(output["status"], "complete"); assert_eq!(output["source"]["kind"], "direct"); - assert_eq!(output["rootLayout"], "standalone"); assert!(output["tsx"].as_str().unwrap().contains("SyntheticFrame")); assert_eq!(output["collection"]["figmaToolCalls"], 3); assert_eq!(output["collection"]["transport"], "legacy-cursor"); @@ -310,13 +308,12 @@ async fn connected_auto_completes_through_the_direct_collector() -> anyhow::Resu assert_eq!(upstream.calls.load(Ordering::SeqCst), 3); assert_eq!(auth.logins.load(Ordering::SeqCst), 0); - // The unambiguous final-answer marker: without it, an agent that only - // ever sees intermediate `needs_figma` steps has, in a real observed - // failure, concluded the conversion was "probably done" and started - // hand-interpreting the raw node tree instead of using this `tsx`. - assert_eq!(output["deliverable"]["kind"], "devup-ui-tsx"); - assert_eq!(output["deliverable"]["isFinal"], true); - assert!(!output["deliverable"]["note"].as_str().unwrap().is_empty()); + // `standalone` is the default, so the root keeps the frame's own size. + // The response no longer echoes the parameter back; the tsx is where the + // choice is visible, and where it has to be right. + let tsx = output["tsx"].as_str().unwrap(); + assert!(tsx.contains("w=\"320px\"")); + assert!(tsx.contains("h=\"240px\"")); Ok(()) } @@ -330,14 +327,13 @@ async fn embedded_root_layout_omits_selected_frame_dimensions() -> anyhow::Resul Arc::new(FixtureUpstream::default()), json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", - "sourcePolicy": "direct", + "outputs": ["tsx"], "rootLayout": "embedded" }), ) .await?; let output = result.structured_content.unwrap(); - assert_eq!(output["rootLayout"], "embedded"); let tsx = output["tsx"].as_str().unwrap(); assert!(!tsx.contains("h=\"240px\"")); assert!(!tsx.contains("w=\"320px\"")); @@ -355,7 +351,6 @@ async fn rejects_unknown_root_layout_before_collecting() -> anyhow::Result<()> { upstream.clone(), json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", - "sourcePolicy": "direct", "rootLayout": "fluid" }), ) @@ -375,7 +370,7 @@ async fn direct_fast_call_error_restarts_the_legacy_collector() -> anyhow::Resul logins: AtomicUsize::new(0), }), upstream.clone(), - input("direct"), + input(), ) .await?; let output = result.structured_content.unwrap(); @@ -402,7 +397,7 @@ async fn auto_asks_to_be_logged_in_rather_than_starting_oauth() -> anyhow::Resul logins: AtomicUsize::new(0), }); let upstream = Arc::new(UpstreamProbe::unavailable()); - let error = call_tool(auth.clone(), upstream.clone(), input("auto")) + let error = call_tool(auth.clone(), upstream.clone(), input()) .await .expect_err("a disconnected direct path cannot collect"); @@ -427,7 +422,7 @@ async fn a_refusal_is_reported_as_itself() -> anyhow::Result<()> { let unavailable = Arc::new(UpstreamProbe::unavailable()); assert!( - call_tool(auth.clone(), unavailable.clone(), input("auto")) + call_tool(auth.clone(), unavailable.clone(), input()) .await .is_err() ); @@ -438,7 +433,7 @@ async fn a_refusal_is_reported_as_itself() -> anyhow::Result<()> { error_code: ErrorCode::DevupFigmaRateLimited, }); assert!( - call_tool(auth, rate_limited.clone(), input("auto")) + call_tool(auth, rate_limited.clone(), input()) .await .is_err() ); diff --git a/crates/devup-mcp/tests/stdio_schema_compat_smoke.rs b/crates/devup-mcp/tests/stdio_schema_compat_smoke.rs index 82391d9..c070075 100644 --- a/crates/devup-mcp/tests/stdio_schema_compat_smoke.rs +++ b/crates/devup-mcp/tests/stdio_schema_compat_smoke.rs @@ -209,8 +209,8 @@ fn tools_list_over_raw_stdio_has_no_boolean_schemas_and_object_output_types() -> .expect("tools/list result must contain a tools array"); assert_eq!( tools.len(), - 9, - "expected all 9 devup-mcp tools (6 devup_figma_* + devup_project_context + devup_ui_validate + devup_stack_diff) to be listed: {tools:?}" + 7, + "expected all 7 devup-mcp tools (4 devup_figma_* + devup_project_context + devup_ui_validate + devup_stack_diff) to be listed: {tools:?}" ); let mut boolean_schema_hits = Vec::new(); diff --git a/crates/devup-mcp/tests/stdio_tools.rs b/crates/devup-mcp/tests/stdio_tools.rs index 93405e7..8de0c4d 100644 --- a/crates/devup-mcp/tests/stdio_tools.rs +++ b/crates/devup-mcp/tests/stdio_tools.rs @@ -147,6 +147,10 @@ async fn exposes_the_seven_read_only_devup_figma_tools() -> anyhow::Result<()> { .collect::>(); names.sort(); + // devup_figma_to_ui and devup_figma_to_json were exactly + // devup_figma_export with one entry in `outputs`, and every client paid + // for their schemas in its context on every session while having to work + // out which of the three to call. assert_eq!( names, [ @@ -154,24 +158,12 @@ async fn exposes_the_seven_read_only_devup_figma_tools() -> anyhow::Result<()> { "devup_figma_explore", "devup_figma_export", "devup_figma_search", - "devup_figma_to_json", - "devup_figma_to_ui", "devup_project_context", "devup_stack_diff", "devup_ui_validate", ] ); - let ui = tools - .iter() - .find(|tool| tool.name == "devup_figma_to_ui") - .unwrap(); - let ui_schema = serde_json::to_value(&ui.input_schema)?; - assert!(ui_schema.to_string().contains("sourcePolicy")); - assert!(ui_schema.to_string().contains("scope")); - assert!(ui_schema.to_string().contains("rootLayout")); - assert!(!ui_schema.to_string().contains("code")); - let export = tools .iter() .find(|tool| tool.name == "devup_figma_export") @@ -190,12 +182,28 @@ async fn exposes_the_seven_read_only_devup_figma_tools() -> anyhow::Result<()> { "frameIds", "allScreens", "delivery", - "sourcePolicy", ] { assert!(export_text.contains(field), "missing export field {field}"); } assert!(!export_text.contains("accessToken")); assert!(!export_text.contains("clientSecret")); + // A closed set of accepted values belongs in the schema, so a caller can + // read it once instead of discovering it one rejection at a time. `scope` + // and `rootLayout` used to arrive as bare strings. + for allowed in [ + r#"["node","page","file"]"#, + r#"["standalone","embedded"]"#, + r#"["auto","inline","resource"]"#, + ] { + assert!( + export_text.contains(allowed), + "export schema does not advertise {allowed}" + ); + } + // The one parameter that never branched: auto and direct both meant the + // direct connection, so it only ever gave a caller a decision to get + // wrong. + assert!(!export_text.contains("sourcePolicy")); let explore = tools .iter() @@ -207,7 +215,6 @@ async fn exposes_the_seven_read_only_devup_figma_tools() -> anyhow::Result<()> { assert!(explore_text.contains("limit")); assert!(explore_text.contains("includeTextPreview")); assert!(explore_text.contains("refresh")); - assert!(explore_text.contains("sourcePolicy")); assert!(!explore_text.contains("code")); client.cancel().await?; diff --git a/crates/devup-mcp/tests/upstream_error_surfacing.rs b/crates/devup-mcp/tests/upstream_error_surfacing.rs index fc5aee4..d701f19 100644 --- a/crates/devup-mcp/tests/upstream_error_surfacing.rs +++ b/crates/devup-mcp/tests/upstream_error_surfacing.rs @@ -137,8 +137,7 @@ async fn reported_failure(upstream: Arc) -> anyhow::Result = json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=10-1", - "outputs": ["tsx"], - "sourcePolicy": "direct" + "outputs": ["tsx"] }) .as_object() .cloned() @@ -199,8 +198,7 @@ async fn a_stated_retry_after_is_reported_instead_of_a_guess() -> anyhow::Result let arguments: Map = json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=10-1", - "outputs": ["tsx"], - "sourcePolicy": "direct" + "outputs": ["tsx"] }) .as_object() .cloned() @@ -242,8 +240,7 @@ async fn a_rate_limited_upstream_reports_its_own_reason_not_a_parse_failure() -> let arguments: Map = json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=10-1", - "outputs": ["tsx"], - "sourcePolicy": "direct" + "outputs": ["tsx"] }) .as_object() .cloned()