fix(schema): accept create_* field names on set_solid_fill/set_text_content

[yavon007]

Problem

The create_* tools and the set_* tools use different names for the same concepts:

Concept	create_frame / create_text / create_shape	set_solid_fill / set_text_content
Colour	fillHex	hex
Paint opacity	fillOpacity	opacity
Text content	characters	text

An agent that just called create_text({ characters: "Hi", fillHex: "#000" }) naturally reaches for set_text_content({ characters: "Bye" }) — and gets back a validation error for a field it did supply:

MCP error -32602: Input validation error: [
  { "code": "invalid_type", "expected": "string", "received": "undefined",
    "path": ["text"], "message": "Required" }
]

The message is confusing because the SDK strips the undeclared characters key before validation, so the error names a field the caller never omitted. I hit this repeatedly while driving the bridge to build a multi-screen mockup — every set_solid_fill and set_text_content in a batch failed while the create_* calls beside them succeeded.

Fix

Accept the create_* spelling as an alias on both tools:

• set_solid_fill accepts fillHex / fillOpacity in addition to hex / opacity
• set_text_content accepts characters in addition to text

Aliases are declared on the advertised shape so the MCP SDK keeps them instead of stripping them, then a small acceptAlias zod preprocessor normalises them onto the canonical field before validation.

Deliberately conservative:

• Canonical wins when both spellings are supplied
• Invalid values still rejected — the alias goes through the same createHexColorSchema() validation
• Wire format unchanged — the plugin still receives hex / text, so no plugin update is needed and no version skew between server and plugin
• No behaviour change for existing callers — canonical spellings work exactly as before
• toolInputSchemas.set_text_content now points at the aliasing schema, so validateRpc accepts the same spellings on the follower RPC path

parseToolInput's signature was widened to z.ZodType<T, z.ZodTypeDef, unknown> so it can take preprocessed schemas.

Testing

tsc --noEmit passes clean.

Schema-level, covering both tools:

--- set_solid_fill ---
PASS  canonical hex/opacity        -> {"nodeId":"1:2","hex":"#1B66D2","opacity":0.5}
PASS  alias fillHex/fillOpacity    -> {"nodeId":"1:2","hex":"#1B66D2","opacity":0.5}
PASS  both (canonical wins)        -> {"nodeId":"1:2","hex":"#111111"}
PASS  neither -> correctly rejected ("Required")
PASS  bad alias value -> correctly rejected ("Color must be a hex value like '#FFAA00'")
PASS  target preserved             -> {"nodeId":"1:2","hex":"#ABCDEF","target":"stroke"}

--- set_text_content ---
PASS  canonical text / alias characters / both / empty-string alias
PASS  neither -> correctly rejected

--- regression ---
PASS  create_text still uses characters
PASS  create_frame still uses fillHex
PASS  validateRpc accepts both spellings

End-to-end against a live Figma file through the real stdio MCP transport — the two calls that failed before now succeed, canonical spellings still work, and invalid input is still rejected:

set_solid_fill w/ fillHex : OK {"applied":{"target":"fill","hex":"#1B66D2","opacity":1}}
set_text_content w/ chars: OK {"previousCharacters":"before","characters":"after (alias)"}
set_solid_fill w/ hex    : OK
set_text_content w/ text : OK
set_solid_fill w/ neither: correctly rejected: Required

Note

Happy to take this in a different direction if you'd prefer — e.g. renaming the set_* fields to match create_* as a breaking change, or leaving the schemas alone and only improving the error message to name the alias. This felt like the least disruptive option since it needs no plugin change and breaks nothing.

Summary by CodeRabbit

• New Features
  • Added support for alternate field names (aliases) when setting solid fills and text content, with automatic normalization to the canonical fields.
• Bug Fixes
  • Improved request validation and ensured normalized, validated parameters are consistently forwarded to the design bridge.
  • Tool handlers now return structured errors immediately on invalid input instead of forwarding bad requests.
• Tests
  • Added a regression script to verify consistent alias normalization across both RPC and tool-handler request paths.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds alias-aware Zod schemas for solid-fill and text-content inputs, changes RPC validation to return normalized parameters, updates tool and RPC forwarding paths, and adds a standalone regression script covering RPC and MCP alias handling.

Changes

Alias-aware validation

Layer / File(s)	Summary
Schema alias normalization server/src/schema.ts	Adds exported shapes and transformed schemas that normalize fillHex/fillOpacity and characters into canonical fields, while returning structured RPC validation results and stripping transport-only fields.
Tool handler validation server/src/tools.ts	Validates raw set_text_content and set_solid_fill arguments through the advertised schemas, returns parse errors, and forwards validated values to the bridge.
RPC forwarding and regression coverage server/src/leader.ts, server/scripts/repro-alias-rpc.ts	Forwards normalized RPC parameters, uses them for save_screenshots, and verifies RPC and MCP alias cases through a standalone regression script.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• gethopp/figma-mcp-bridge#23: Both changes update server-side schemas and tool wiring for text-content and solid-fill operations.
• gethopp/figma-mcp-bridge#6: Both changes modify save_screenshots RPC handling in server/src/leader.ts.

Suggested reviewers: konsalex, vladmdgolam

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the core change: accepting create_* aliases for set_solid_fill and set_text_content.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[konsalex]

Hey @yavon007 thanks for the PR 🙏

The underlying problem seems real and worth fixing. The server strips the undeclared key before validation, so the error names a field the caller believes it supplied and the agent has no way to self correct 🥲

Two things I'd want changed before it lands.

1/ The RPC path accepts the alias but never applies it

validateRpc parses, then throws the parsed result away:

const result = toolInputSchemas[name].safeParse(rpcToArgs[name](nodeIds, params));
return result.success ? null : result.error.issues[0].message;

The leader then forwards rpcReq.params, not the normalized object (see leader.ts:135). The preprocessor runs, normalizes for the purposes of validation, and its output goes in the bin.

So an aliased call over HTTP now goes through the gate and reaches the plugin with the wrong key, where it dies on text is required for set_text_content (see code.ts:696) or hex is required (see code.ts:914). Before this PR the same call got a fast, precise 400. That path is worse off than it started imho.

I've attached a script at the end that reproduces it with no live Figma connection needed. It calls the real validateRpc, then mimics the leader's forwarding and the plugin's own guards. Run bun run scripts/repro-alias-rpc.ts, exits 1 while the bug is present:

FAIL  RPC   set_text_content { characters }   -> rejected-by-plugin: text is required for set_text_content
FAIL  RPC   set_solid_fill   { fillHex }      -> rejected-by-plugin: hex is required
...
PASS  TOOL  set_text_content { characters }   -> applied {"text":"hello"}

That last line is the control. Identical input succeeds on the MCP tool path because those handlers forward parsed.data, which is what isolates the cause to the plumbing rather than your preprocessor.

The fix is to have validateRpc return the parsed params and let the leader forward those.

2/ Each schema is written twice

setSolidFillShape and setSolidFillCanonical declare nodeId, hex, opacity and fileKey independently, same story for the text pair. That's a risk for having a drift in the future, and it fails in the worst way: add a field to the advertised shape only, and the canonical z.object strips it during parse. No type error, no runtime error, the parameter just quietly vanishes.

You do need the aliases on the advertised shape, since server.tool takes a raw shape and the SDK strips unknown keys, so no way around that part. But you don't need a second object. preprocess wraps from the outside, which is exactly what forces the duplicate. transform extends from the inside and reuses the shape you already have:

export const setSolidFillInput = setSolidFillShape.transform(
  ({ fillHex, fillOpacity, ...rest }, ctx) => {
    const hex = rest.hex ?? fillHex;
    if (hex === undefined) {
      ctx.addIssue({
        code: z.ZodIssueCode.custom,
        path: ["hex"],
        message: "hex is required (fillHex is accepted as an alias)",
      });
      return z.NEVER;
    }
    return { ...rest, hex, opacity: rest.opacity ?? fillOpacity };
  }
);

I ran your own test matrix against this and it reproduces every case, canonical winning included, with the bad alias value still rejected by createHexColorSchema(). The ctx plus z.NEVER form narrows hex to string in the output type, so you can drop the cast and the explicit z.ZodType<...> annotation.

It also lines up with what the file already does. setTextPropertiesInput at :519 and setEffectsInput at :347 both derive from their shape, so nobody has to learn a second pattern. And the message names the alias, where the current version emits a bare "Required" pointing at hex, which is the same confusing error this PR set out to kill.

3/ Smaller point

Marking hex and text optional means the generated JSON Schema lists only nodeId as required. The "you must supply one of these two" rule then lives only in the English description. Given the premise here is that agents read field names off the schema, I'd put it in the .describe() strings as well.

Not blocking, but worth thinking about

Aliases are a reasonable shim. They do become a treadmill if you add them pairwise every time an agent trips over one. This is the second (#40 does the same for imageBase64), and there's a third case already sitting in the code: the plugin's set_text_properties handler reads params.fillHex and params.fillOpacity (code.ts:800), so the set_* family isn't internally consistent either.

At some point the answer is to pick one vocabulary, apply it across every tool, and do the rename at a version boundary with these aliases kept around as deprecated for a release or two. Which is a decent argument for getting the alias machinery clean now, since it's the migration path later anyway (1-2 patch versions after the shims land).

Test code example:

Testing code

This is heavily AI generated, but illustrated the point. To run:

cd server
bun run scripts/repro-alias-rpc.ts

/**
 * Reproduces the alias normalisation gap on the HTTP RPC path.
 *
 * PR #39 teaches `set_solid_fill` / `set_text_content` to accept the create_*
 * spellings (`fillHex`, `fillOpacity`, `characters`) via a zod preprocessor.
 * That works for the MCP tool handlers, which forward `parsed.data`. It does
 * not work for the HTTP RPC path, because `validateRpc` throws away the parsed
 * result and the leader forwards the caller's raw params to the plugin.
 *
 * Run with:  bun run scripts/repro-alias-rpc.ts
 * Exits 1 while the bug is present, 0 once it is fixed.
 */

import { validateRpc, toolInputSchemas } from "../src/schema.js";

type Outcome =
  | { status: "rejected-by-server"; message: string }
  | { status: "rejected-by-plugin"; message: string }
  | { status: "applied"; payload: Record<string, unknown> };

/**
 * The plugin's own guards, copied from plugin/src/main/code.ts so this script
 * runs without a live Figma connection.
 */
const pluginGuards: Record<
  string,
  (nodeIds: string[], params: Record<string, unknown>) => string | null
> = {
  // plugin/src/main/code.ts:692-697
  set_text_content: (nodeIds, params) => {
    if (!nodeIds[0]) return "nodeIds is required for set_text_content";
    if (typeof params.text !== "string")
      return "text is required for set_text_content";
    return null;
  },
  // plugin/src/main/code.ts:906-915
  set_solid_fill: (nodeIds, params) => {
    if (!nodeIds[0]) return "nodeIds is required for set_solid_fill";
    if (typeof params.hex !== "string") return "hex is required";
    return null;
  },
};

/**
 * Mirrors LeaderServer.handleRPC: validate, then forward the caller's original
 * params. See server/src/leader.ts:94-140 — note that `rpcReq.params` is sent,
 * not the object `validateRpc` parsed internally.
 */
function simulateRpcCall(
  tool: string,
  nodeIds: string[],
  params: Record<string, unknown>
): Outcome {
  const validationError = validateRpc(tool, nodeIds, params);
  if (validationError) {
    return { status: "rejected-by-server", message: validationError };
  }

  const pluginError = pluginGuards[tool](nodeIds, params);
  if (pluginError) {
    return { status: "rejected-by-plugin", message: pluginError };
  }
  return { status: "applied", payload: params };
}

/**
 * Mirrors the MCP tool handlers in server/src/tools.ts, which parse first and
 * forward `parsed.data`. Included as a control: this path handles aliases fine.
 */
function simulateMcpToolCall(
  tool: "set_text_content" | "set_solid_fill",
  args: Record<string, unknown>
): Outcome {
  const parsed = toolInputSchemas[tool].safeParse(args);
  if (!parsed.success) {
    return {
      status: "rejected-by-server",
      message: parsed.error.issues[0].message,
    };
  }

  const { nodeId, fileKey, ...rest } = parsed.data;
  const payload =
    tool === "set_text_content" ? { text: rest.text } : { ...rest };

  const pluginError = pluginGuards[tool]([nodeId], payload);
  if (pluginError) {
    return { status: "rejected-by-plugin", message: pluginError };
  }
  return { status: "applied", payload };
}

function describe(outcome: Outcome): string {
  return outcome.status === "applied"
    ? `applied ${JSON.stringify(outcome.payload)}`
    : `${outcome.status}: ${outcome.message}`;
}

interface Case {
  name: string;
  outcome: Outcome;
  expected: Outcome["status"];
}

const cases: Case[] = [
  // The bug: aliases pass validation, then die inside the plugin.
  {
    name: "RPC   set_text_content { characters }",
    outcome: simulateRpcCall("set_text_content", ["1:2"], {
      characters: "hello",
    }),
    expected: "applied",
  },
  {
    name: "RPC   set_solid_fill   { fillHex }",
    outcome: simulateRpcCall("set_solid_fill", ["1:2"], {
      fillHex: "#1B66D2",
    }),
    expected: "applied",
  },
  {
    name: "RPC   set_solid_fill   { fillHex, fillOpacity }",
    outcome: simulateRpcCall("set_solid_fill", ["1:2"], {
      fillHex: "#1B66D2",
      fillOpacity: 0.5,
    }),
    expected: "applied",
  },

  // Canonical spellings over RPC still work.
  {
    name: "RPC   set_text_content { text }",
    outcome: simulateRpcCall("set_text_content", ["1:2"], { text: "hello" }),
    expected: "applied",
  },
  {
    name: "RPC   set_solid_fill   { hex }",
    outcome: simulateRpcCall("set_solid_fill", ["1:2"], { hex: "#1B66D2" }),
    expected: "applied",
  },

  // Genuinely bad input must still be caught by the server, not the plugin.
  {
    name: "RPC   set_solid_fill   { } (neither spelling)",
    outcome: simulateRpcCall("set_solid_fill", ["1:2"], {}),
    expected: "rejected-by-server",
  },
  {
    name: "RPC   set_solid_fill   { fillHex: 'not-a-hex' }",
    outcome: simulateRpcCall("set_solid_fill", ["1:2"], {
      fillHex: "not-a-hex",
    }),
    expected: "rejected-by-server",
  },

  // Control: the MCP tool path forwards parsed.data, so aliases work there.
  {
    name: "TOOL  set_text_content { characters }",
    outcome: simulateMcpToolCall("set_text_content", {
      nodeId: "1:2",
      characters: "hello",
    }),
    expected: "applied",
  },
  {
    name: "TOOL  set_solid_fill   { fillHex }",
    outcome: simulateMcpToolCall("set_solid_fill", {
      nodeId: "1:2",
      fillHex: "#1B66D2",
    }),
    expected: "applied",
  },
];

const failures = cases.filter((c) => c.outcome.status !== c.expected);

for (const { name, outcome, expected } of cases) {
  const ok = outcome.status === expected;
  console.log(
    `${ok ? "PASS" : "FAIL"}  ${name.padEnd(46)} -> ${describe(outcome)}`
  );
}

console.log();
if (failures.length === 0) {
  console.log("All cases behave as expected — the RPC path is fixed.");
  process.exit(0);
}

console.log(`${failures.length} case(s) reproduce the bug:`);
for (const { name, outcome, expected } of failures) {
  console.log(`  ${name}\n    expected ${expected}, got ${describe(outcome)}`);
}
console.log(
  "\nRoot cause: validateRpc (server/src/schema.ts:981) discards the parsed\n" +
    "result, and the leader (server/src/leader.ts:135) forwards rpcReq.params\n" +
    "verbatim — so the alias is never rewritten to the canonical key."
);
process.exit(1);

[yavon007]

Thanks for the thorough review — all three points were right, and the RPC one was a real hole I'd left. Pushed in d2441f0.

1/ RPC path

Confirmed with your script before touching anything, and it reproduced exactly as you described:

set_text_content {"characters":"hello"}
  validateRpc → null  ✓ passed the gate
  plugin receives raw params → ✗ text is required for set_text_content
set_solid_fill {"fillHex":"#1B66D2"}
  validateRpc → null  ✓ passed the gate
  plugin receives raw params → ✗ hex is required

validateRpc now returns { error, params } and the leader forwards the parsed object. One wrinkle worth flagging: I strip nodeId and fileKey from what gets forwarded. rpcToArgs folds the transport-level nodeIds into a nodeId field so the tool schema can validate it, but the plugin reads node ids off request.nodeIds (code.ts:905), and fileKey is passed as a separate argument to sendWithParams. Without stripping them, both would have appeared inside params for every tool — a payload change well beyond the aliases. save_screenshots now reads the validated params too, since it was pulling from rpcReq.params directly.

Your script passes all 9 cases:

PASS  RPC   set_text_content { characters }          -> applied {"text":"hello"}
PASS  RPC   set_solid_fill   { fillHex }             -> applied {"hex":"#1B66D2"}
PASS  RPC   set_solid_fill   { fillHex, fillOpacity } -> applied {"hex":"#1B66D2","opacity":0.5}
PASS  RPC   set_text_content { text }                -> applied {"text":"hello"}
PASS  RPC   set_solid_fill   { hex }                 -> applied {"hex":"#1B66D2"}
PASS  RPC   set_solid_fill   { } (neither)           -> rejected-by-server: hex is required (fillHex is accepted as an alias)
PASS  RPC   set_solid_fill   { fillHex: 'not-a-hex' } -> rejected-by-server: Color must be a hex value like '#FFAA00'
PASS  TOOL  set_text_content { characters }          -> applied {"text":"hello"}
PASS  TOOL  set_solid_fill   { fillHex }             -> applied {"hex":"#1B66D2"}

I kept it at server/scripts/repro-alias-rpc.ts as a regression check — there's no test runner in the repo, and it needs no live connection. Happy to drop it if you'd rather not carry scripts in-tree.

Since validateRpc's return type is now shared by every tool, I checked the rest didn't regress — same verdicts as before, and no params dropped:

PASS  get_node               pass  | 0 keys kept
PASS  set_node_properties    pass  | 2 keys kept
PASS  create_frame           pass  | 4 keys kept
PASS  delete_nodes           pass  | 2 keys kept
PASS  set_effects            pass  | 1 key kept
PASS  unknown_tool           pass  | no schema, forwards original params
PASS  set_node_properties    fail  | Expected number, received string
PASS  get_screenshot         fail  | Invalid enum value...

2/ Duplicate schemas

Switched to .transform() as you suggested — the second z.object is gone, both inputs derive from their advertised shape. You're right that it lines up with setTextPropertiesInput and setEffectsInput, and the error now names the alias instead of the bare "Required".

One thing didn't work out as hoped: I tried dropping the widened parseToolInput signature, but transform makes input and output types differ, and z.ZodType<T> defaults the input to T:

Argument of type 'ZodEffects<...>' is not assignable to parameter of type 'ZodType<{ text: string; ... }, ZodTypeDef, { text: string; ... }>'.
  The types of '_input.text' are incompatible between these types.
    Type 'string | undefined' is not assignable to type 'string'.

So z.ZodType<T, z.ZodTypeDef, unknown> stays, now with a comment explaining why. The cast and the explicit annotation on the schemas themselves are both gone, as you said they would be.

3/ Describe strings

Added — hex and text now say "Required unless <alias> is given", and the alias fields say "Supply one of the two".

On the treadmill

Agreed, and thanks for spelling out the endgame. #40 turned out to be a duplicate of #36 so I closed it, which leaves set_text_properties reading params.fillHex (code.ts:800) as the remaining inconsistency. Happy to take the unify-and-deprecate pass whenever you want to schedule it against a version boundary — this machinery should be the migration path rather than another shim.

[coderabbitai]

🧹 Nitpick comments (1)

server/scripts/repro-alias-rpc.ts (1)

39-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

fillOpacity alias isn't actually asserted by the regression suite.

set_solid_fill's guard only checks params.hex; it never inspects params.opacity. As a result, the { fillHex, fillOpacity } case (Lines 123-129) can report "applied" even if fillOpacity were silently dropped during normalization — the guard has no way to catch that. Since normalizing fillOpacity is one of the two behaviors this PR is meant to guard against regressing, consider either extending the guard to require typeof params.opacity === "number" when supplied, or asserting outcome.payload.opacity === 0.5 directly in that test case.

Proposed fix

   set_solid_fill: (nodeIds, params) => {
     if (!nodeIds[0]) return "nodeIds is required for set_solid_fill";
     if (typeof params.hex !== "string") return "hex is required";
+    if (params.opacity !== undefined && typeof params.opacity !== "number") {
+      return "opacity must be a number when provided";
+    }
     return null;
   },

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/scripts/repro-alias-rpc.ts` around lines 39 - 44, Strengthen the
regression coverage for the fillOpacity alias in set_solid_fill: require
params.opacity to be a number when the alias is supplied, or directly assert
outcome.payload.opacity equals 0.5 in the { fillHex, fillOpacity } test case.
Ensure the test fails if fillOpacity is dropped during normalization.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@server/scripts/repro-alias-rpc.ts`:
- Around line 39-44: Strengthen the regression coverage for the fillOpacity
alias in set_solid_fill: require params.opacity to be a number when the alias is
supplied, or directly assert outcome.payload.opacity equals 0.5 in the {
fillHex, fillOpacity } test case. Ensure the test fails if fillOpacity is
dropped during normalization.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: faceb053-3e77-4581-821b-ff559905e435

📥 Commits

Reviewing files that changed from the base of the PR and between e05248a and d2441f0.

📒 Files selected for processing (4)

• server/scripts/repro-alias-rpc.ts
• server/src/leader.ts
• server/src/schema.ts
• server/src/tools.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• server/src/tools.ts

[konsalex]

Thanks for the fixes @yavon007 those will be included in 0.0.18