Skip to content

production_runMRP fails for every non-UI caller — dispatcher can't inject the userId its own service function requires #1633

Description

@layne-sygnet

The bug

Calling production_runMRP through Carbon's generic MCP/API dispatch layer — with exactly the arguments its own published schema asks for — fails every time with:

{"name":"ZodError","message":"[\n  {\n    \"expected\": \"string\",\n    \"code\": \"invalid_type\",\n    \"path\": [\n      \"userId\"\n    ],\n    \"message\": \"Invalid input: expected string, received undefined\"\n  }\n]"}

The tool's own advertised input schema (via describe_tool) is:

{"type":"object","properties":{"type":{"type":"string","enum":["company","location","job","salesOrder","item","purchaseOrder"]},"id":{"type":"string"}},"required":["type","id"]}

userId is not a documented input. A caller cannot supply what they aren't told to supply, and cannot pass validation with only the documented fields — but it's the platform's own auto-injection that's supposed to fill it in, and for this one tool, that injection mechanism structurally cannot.

Root cause — confirmed from source, with exact citations (verified against main @ 71eaa6f, 2026-09-14)

  1. The underlying function requires userId as a field inside its third parameter:

    apps/erp/app/modules/production/production.service.ts:2588-2602

    export async function runMRP(
      client: SupabaseClient<Database>,
      db: Kysely<KyselyDatabase>,
      params: {
        type: "company" | "location" | "job" | "salesOrder" | "item" | "purchaseOrder";
        id: string;
        companyId: string;
        userId: string;
      }
    ) {
  2. The tool's manifest entry injects the wrong field name:

    apps/erp/app/routes/api+/mcp+/lib/tool-manifest.digest.json:788

    {"name":"production_runMRP","classification":"WRITE","paramCount":2,"schema":"6e039fba26f6","response":"9e056a59d695","injectAuth":"companyId+updatedBy","permission":"production:update","paginates":false}

    injectAuth is companyId+updatedBy — it stamps updatedBy, never userId.

  3. The generic auth-stamping function has no vocabulary for userId at all:

    packages/api/src/manifest-types.ts:10-14

    export type AuthField =
      | "companyId"
      | "companyGroupId"
      | "createdBy"
      | "updatedBy";

    apps/erp/app/routes/api+/v1+/lib/dispatch.server.ts (enrichWithAuthContext) only knows how to write createdBy, updatedBy, companyId, companyGroupId onto the payload object — there is no case that can ever produce a key literally named userId. So for any tool whose underlying function's merged-params object requires a field named userId (not createdBy/updatedBy), this mechanism cannot satisfy it, structurally, for any caller — API key, OAuth, or otherwise.

  4. There IS a working, separate mechanism for exactly this — but runMRP doesn't use its calling convention. apps/erp/app/routes/api+/v1+/lib/dispatch.server.ts (~line 243):

    const functionArgs: any[] = [];
    for (const paramName of meta.serviceParams) {
      if (paramName === "client") {
        functionArgs.push(context.client);
      } else if (paramName === "db") {
        functionArgs.push(getDatabaseClient());
      } else if (paramName === "userId") {
        functionArgs.push(context.userId);
      } else if (paramName === "companyId") {
        functionArgs.push(context.companyId);
      }
      ...

    When a service function declares companyId/userId as their own top-level positional parameters (not merged into an object), the dispatcher injects them correctly, unconditionally, with no injectAuth/AuthField involvement at all. runMRP(client, db, params) doesn't use this shape — userId is buried inside params, which is why it falls through to the broken enrichWithAuthContext path instead.

Established fix pattern already in the same module

apps/erp/app/modules/production/production.mcp.server.ts exists specifically to give MCP-exposed production operations the right calling convention, and already solves this exact class of problem for a sibling operation, scheduleJob: it takes companyId/userId as top-level positional parameters (matching the dispatch mechanism in point 4), then merges them into the object the real underlying function needs.

I'm opening a PR with a fix that adds the same wrapper pattern for runMRP — will link it here once created.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions