Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/28715-mcp-validate-expression-label-targets.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"@objectstack/mcp": patch
"@objectstack/runtime": patch
---

fix(mcp): `validate_expression` refuses the `label()` call the object save door refuses

The object save door refuses a formula whose `label(record.FIELD)` names a field that declares no options (422 `expression-invalid`, code `label-field-no-options`). The MCP `validate_expression` tool judges with the same validator, but the object schema it read carried field names and types only, so it answered `ok: true` for that formula and the agent learned of the refusal at save.

Clause-②: yes (widening) — the published `McpDataBridge` interface gains one optional member, `optionFieldsOf(name)`. A bridge that omits it behaves exactly as before. No tool's input or output shape changes: `describe_object` serves the same projection, and `validate_expression`'s new verdict is an `errors[]` entry with an existing code.

Metadata-protocol: none (nothing a metadata author can write is added, refused or changed in meaning; the door already refused this formula)

- **Now.** On the `formula` site, `validate_expression` hands the validator the object's option-bearing fields (`ExprSchemaHint.optionFields`), so `label(record.first)` on a text field answers `ok: false` with one `label-field-no-options` error naming `first`, in the sentence the door carries.
- **Unchanged.** `label()` on a field with inline `options`, or bound to a shared `picklist`, reads ok. An unknown field keeps its `unknown-field` refusal and gets no second one. The `validation`, `flow_condition` and `template` sites answer as before: the door judges `label()` targets on a formula field's `expression` only.
- **How.** Both bridges (the stdio bridge and the runtime's HTTP bridge) answer `optionFieldsOf` from the object `describeObject` reads, the protocol's served object, through `@objectstack/formula`'s `declaresOptions`: on the served object a picklist-bound field already carries its list's resolved `options`.
39 changes: 35 additions & 4 deletions packages/mcp/src/mcp-http-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,25 @@ export interface McpDataBridge {
*/
listObjectsDiagnosed?(): Promise<DiagnosedObjectListing<McpObjectSummary>>;
describeObject(name: string): Promise<unknown | null>;
/**
* [#28715] The fields of `name` that declare options — the fields a
* formula's `label(record.<field>)` can answer on — or `null` when no such
* object is served. Read from the same object {@link describeObject} reads,
* through `@objectstack/formula`'s `declaresOptions`, the predicate the
* engine builds `label()`'s map with: both bridges read the protocol's
* served object, where a field bound to a shared `picklist` already carries
* the list's resolved `options`.
*
* `validate_expression` hands it to the shared validator as
* `ExprSchemaHint.optionFields`, so the tool refuses the `label()` call the
* object save door refuses, in the same words. It is a member of its own,
* not a key on {@link describeObject}'s answer, so `describe_object` serves
* what it served before.
*
* OPTIONAL, like {@link listObjectsDiagnosed}: a bridge that omits it gets
* no `label()` target check, exactly as before (the validator's opt-in).
*/
optionFieldsOf?(name: string): Promise<readonly string[] | null>;
query(
object: string,
opts: {
Expand Down Expand Up @@ -580,12 +599,18 @@ function jsonText(value: unknown): string {
* (`TemplateExpressionInputSchema`). The tool parses with it first, so whatever
* the gate refuses at the envelope level — today a source that is blank after
* trimming — the tool refuses too, with the gate's own issue.
*
* [#28715] `optionLabels`: the site where the gate judges what a `label()`
* call names (`ExprSchemaHint.optionFields`). `@objectstack/lint`'s formula
* pass hands the validator the object's option-bearing fields on a formula
* field's `expression` and nowhere else, so the tool asks the bridge for them
* on the `formula` site only and answers every other site as the gate does.
*/
const VALIDATE_SITE_MAP: Record<
string,
{ role: FieldRole; scope: 'record' | 'flattened'; slot: z.ZodType }
{ role: FieldRole; scope: 'record' | 'flattened'; slot: z.ZodType; optionLabels?: true }
> = {
formula: { role: 'value', scope: 'record', slot: EvaluatedExpressionInputSchema },
formula: { role: 'value', scope: 'record', slot: EvaluatedExpressionInputSchema, optionLabels: true },
validation: { role: 'predicate', scope: 'record', slot: EvaluatedExpressionInputSchema },
flow_condition: { role: 'predicate', scope: 'flattened', slot: EvaluatedExpressionInputSchema },
template: { role: 'template', scope: 'record', slot: TemplateExpressionInputSchema },
Expand Down Expand Up @@ -1036,8 +1061,14 @@ export function registerObjectTools(
fields.push(f.name);
if (typeof f?.type === 'string') fieldTypes[f.name] = f.type;
}
const { role, scope, slot } = VALIDATE_SITE_MAP[site ?? 'formula'];
const hint = { objectName, fields, fieldTypes, scope } as const;
const { role, scope, slot, optionLabels } = VALIDATE_SITE_MAP[site ?? 'formula'];
// [#28715] The object's option-bearing fields, where the gate reads
// them: `label(record.<field>)` on a declared field outside this list
// is refused as `label-field-no-options` by the one validator.
const optionFields = optionLabels && bridge.optionFieldsOf
? await bridge.optionFieldsOf(objectName)
: null;
const hint = { objectName, fields, fieldTypes, scope, ...(optionFields ? { optionFields } : {}) } as const;
// The gate's envelope verdict first: `validateExpression` treats a
// blank source as "not authored" and answers ok, while the save gate
// refuses it. Each issue is the spec schema's own (code and message),
Expand Down
204 changes: 204 additions & 0 deletions packages/mcp/src/mcp-validate-expression.option-labels.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#28715] `validate_expression` refuses the `label()` call the object save
* door refuses: `label(record.<field>)` on a field that declares no options.
*
* ## The state this closes
*
* The object door hands the shared validator the object's option-bearing
* fields (`ExprSchemaHint.optionFields`) on a formula field's `expression`, so
* `label(record.first)` on a text field is refused there as
* `label-field-no-options`. The MCP tool judged the same expression with the
* same validator, but its hint carried only field names and types: both
* bridges project fields to name/type/label/required. So the tool answered
* `ok: true` for a formula the door then refused at save.
*
* ## The fix, and what is pinned here
*
* Each bridge answers `optionFieldsOf(name)` from the object `describeObject`
* reads, through `@objectstack/formula`'s `declaresOptions`, and the tool
* hands that list to the validator on the `formula` site, the one site the
* door judges it on. `describe_object` serves what it served before.
*
* Which predicate: measured on real kernels (`ObjectKernel` + `MetadataPlugin`
* + `ObjectQLPlugin`, with and without an `environmentId`), the protocol's
* merged object listing, which both bridges read, and the metadata service's
* by-name read, the fallback on a host with no merged read, serve a field
* bound to a shared picklist with the list's resolved `options` beside its
* `picklist` name. That is the served form `declaresOptions` reads; the door
* reads the AUTHORED body (no `options` on a picklist-bound field) and so
* uses `declaresOptionsOnceServed`. {@link SERVED} below is that measured
* shape and {@link authored} the body an author saves.
*
* This file drives the stdio bridge (`createStdioDataBridge`) through the
* shared tool handler; the HTTP bridge's answer is pinned in
* `packages/runtime/src/domains/mcp-option-fields-read.test.ts` on the same
* shape. Refusals assert `code` and the named field; the parity case asserts
* the tool's sentence is the one the door carries.
*/

import { describe, it, expect } from 'vitest';
import type { ExecutionContext } from '@objectstack/spec/kernel';
import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts';
import { runRuntimeAuthoringRules } from '@objectstack/lint';

import { MCPServerRuntime } from './mcp-server-runtime.js';
import type { McpDataBridge } from './mcp-http-tools.js';
import { createStdioDataBridge, type StdioProtocolObjectReads } from './stdio-data-bridge.js';

const OBJECT = 'qaformula_p4';

/** The fields an author saves (the door judges this body). */
const AUTHORED_FIELDS = {
first: { type: 'text', label: 'First' },
kind: { type: 'select', label: 'Kind', options: [{ value: 'clean', label: 'Clean' }] },
stage: { type: 'radio', label: 'Stage', options: [{ value: 'draft', label: 'Draft' }] },
// Bound to a shared picklist: authored with no `options`.
tier: { type: 'select', label: 'Tier', picklist: 'qa_tiers' },
};

/** The shared list `tier` names. */
const QA_TIERS = { name: 'qa_tiers', label: 'Tiers', options: [{ value: 'gold', label: 'Gold' }] };

/** The same object as the protocol serves it: `tier` carries its list's options (measured). */
const SERVED = {
name: OBJECT,
label: 'P4',
fields: {
...AUTHORED_FIELDS,
tier: { ...AUTHORED_FIELDS.tier, options: QA_TIERS.options },
},
};

/** The authored object with a formula field `pf` holding `expression`. */
const authored = (expression: string) => ({
name: OBJECT,
label: 'P4',
// Keeps `security-owd-unset` quiet, so a refusal is the expression rule's.
sharingModel: 'private',
fields: { ...AUTHORED_FIELDS, pf: { type: 'formula', label: 'PF', expression } },
});

const PRINCIPAL = { userId: 'u1', isSystem: false } as unknown as ExecutionContext;

/** The stdio bridge over a protocol that serves {@link SERVED}. */
function stdioBridge(): McpDataBridge {
const protocol: StdioProtocolObjectReads = {
getMetaItems: async () => ({ type: 'object', items: [SERVED] }),
getMetaItem: async ({ name }: { name: string }) => ({ type: 'object', name, item: name === OBJECT ? SERVED : undefined }),
} as unknown as StdioProtocolObjectReads;
return createStdioDataBridge({
engine: {} as unknown as IDataEngine,
metadataService: { listObjects: async () => [], getObject: async () => undefined } as unknown as IMetadataService,
protocol: () => protocol,
resolvePrincipal: async () => PRINCIPAL,
});
}

/** The same bridge with `optionFieldsOf` taken away (a bridge predating it). */
function withoutOptionFields(bridge: McpDataBridge): McpDataBridge {
const { optionFieldsOf: _omit, ...rest } = bridge as McpDataBridge & Record<string, unknown>;
return rest as McpDataBridge;
}

async function callTool(bridge: McpDataBridge, name: string, args: Record<string, unknown>) {
const runtime = new MCPServerRuntime({ name: 'objectstack-test', version: '9.9.9' });
const body = { jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name, arguments: args } };
const res = await runtime.handleHttpRequest(
new Request('http://localhost/mcp', {
method: 'POST',
headers: { 'content-type': 'application/json', accept: 'application/json, text/event-stream' },
body: JSON.stringify(body),
}),
{ bridge, parsedBody: body } as never,
);
const json = (await res.json()) as { result: { content: Array<{ text: string }> } };
return JSON.parse(json.result.content[0]!.text);
}

const validate = (bridge: McpDataBridge, expression: string, site?: string) =>
callTool(bridge, 'validate_expression', { objectName: OBJECT, expression, ...(site ? { site } : {}) });

const codes = (r: { errors: Array<{ code?: string }> }) => r.errors.map((e) => e.code);

describe('validate_expression judges label() targets as the object door does (#28715)', () => {
it('the stdio bridge answers the served object\'s option-bearing fields, picklist-bound included', async () => {
const bridge = stdioBridge();
expect(await bridge.optionFieldsOf!(OBJECT)).toEqual(['kind', 'stage', 'tier']);
expect(await bridge.optionFieldsOf!('qa_absent')).toBeNull();
});

for (const expression of ['label(record.first)', 'label(record.kind) + label(record.first)']) {
it(`⭐ LIT — \`${expression}\` is refused, naming \`first\``, async () => {
const r = await validate(stdioBridge(), expression);
expect(r.ok).toBe(false);
expect(r.errors).toHaveLength(1);
expect(r.errors[0]).toMatchObject({ code: 'label-field-no-options', params: { field: 'first' } });
});
}

for (const expression of [
'label(record.kind)',
"label(record.kind) + ' · ' + label(record.stage)",
'record.first + label(record.kind)',
// Bound to a shared picklist: the served field carries the list's options.
'label(record.tier)',
"label(record.tier) + ' / ' + label(record.kind)",
]) {
it(`CONTROL — \`${expression}\` (option-bearing fields) reads ok`, async () => {
const r = await validate(stdioBridge(), expression);
expect(r.errors).toEqual([]);
expect(r.ok).toBe(true);
});
}

it('CONTROL — an unknown field keeps its unknown-field refusal and gets no second one', async () => {
const r = await validate(stdioBridge(), 'label(record.nope)');
expect(codes(r)).toEqual(['unknown-field']);
});

it('CONTROL — every other site answers exactly what it answered before (the door judges label() targets on a formula only)', async () => {
for (const [site, expression] of [
['validation', "label(record.first) == 'x'"],
['flow_condition', "label(first) == 'x'"],
['template', '{{ label(record.first) }}'],
] as const) {
const before = await validate(withoutOptionFields(stdioBridge()), expression, site);
const after = await validate(stdioBridge(), expression, site);
expect(after, `${site}: ${expression}`).toEqual(before);
expect(codes(after)).not.toContain('label-field-no-options');
}
});

it('CONTROL — a bridge without optionFieldsOf answers as before: no label() target check', async () => {
const r = await validate(withoutOptionFields(stdioBridge()), 'label(record.first)');
expect(codes(r)).not.toContain('label-field-no-options');
expect(r.ok).toBe(true);
});

it('describe_object serves the same projection as before: no options reading on its answer', async () => {
const answer = await callTool(stdioBridge(), 'describe_object', { objectName: OBJECT });
expect(Object.keys(answer).sort()).toEqual(['enableFeatures', 'fields', 'label', 'name']);
for (const field of answer.fields) expect(Object.keys(field).sort()).toEqual(['label', 'name', 'required', 'type']);
});

it('PARITY — the tool and the object door give one verdict, in one sentence, for each expression', async () => {
for (const expression of [
'label(record.first)',
'label(record.kind) + label(record.first)',
'label(record.kind)',
'label(record.tier)',
'label(record.nope)',
]) {
const tool = await validate(stdioBridge(), expression);
const door = runRuntimeAuthoringRules({ type: 'object', item: authored(expression), context: { objects: [] } });
const doorErrors = door.errors.filter((f) => f.rule === 'expression-invalid');
expect(tool.ok, expression).toBe(doorErrors.length === 0);
expect(tool.errors.length, expression).toBe(doorErrors.length);
tool.errors.forEach((e: { message: string }, i: number) => {
expect(doorErrors[i]!.message, expression).toContain(e.message);
});
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ const RECIPES: Record<string, Recipe> = {
// share is what keeps a stored field off this response in the first place.
listObjectsDiagnosed: { invoke: (b) => b.listObjectsDiagnosed(), writesRecords: false },
describeObject: { invoke: (b) => b.describeObject('vault'), writesRecords: false },
// [#28715] Field NAMES off the object definition `describeObject` reads: no
// engine read, no record, so nothing stored can ride this answer.
optionFieldsOf: { invoke: (b) => b.optionFieldsOf('vault'), writesRecords: false },
query: { invoke: (b) => b.query('vault', {}), writesRecords: false },
get: { invoke: (b) => b.get('vault', 'row-1'), writesRecords: false },
aggregate: { invoke: (b) => b.aggregate('vault', {}), writesRecords: false },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@ const BRIDGE_MEMBERS: Record<string, string> = {
listObjects: 'no row body — object definitions from the metadata service',
listObjectsDiagnosed: 'no row body — object definitions from the metadata service',
describeObject: 'no row body — one object definition from the metadata service',
optionFieldsOf: 'no row body — field names from the same object definition describeObject reads',
query: 'seam — serveStoredMetadataRows; filter / sort on the body column refused',
get: 'seam — serveStoredMetadataRow',
aggregate: 'refuses — group / filter / aggregate member on the body column',
Expand Down
14 changes: 14 additions & 0 deletions packages/mcp/src/stdio-data-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ import { recordNotFoundError } from '@objectstack/core';
// asks beside its exposure gate (`RestServer.enforceApiAccess`, the runtime
// dispatcher's `refuseUnexposedDataAction`), asked here by the write arms.
import { apiKeyManagementCredentialRefusal } from '@objectstack/core';
// [#28715] The engine's own reading of "this field declares options" (its
// `label()` map is built from it), never a second spelling here.
import { declaresOptions } from '@objectstack/formula';
import type { McpDataBridge, McpObjectSummary } from './mcp-http-tools.js';
import { metaItemsArray, type McpObjectView } from './mcp-server-runtime.js';
import {
Expand Down Expand Up @@ -942,6 +945,17 @@ export function createStdioDataBridge(deps: StdioDataBridgeDeps): McpDataBridge
};
},

// [#28715] From the object `describeObject` reads: the protocol's served
// object, where a picklist-bound field carries its list's resolved
// `options`, so the served-form predicate is the one that matches.
async optionFieldsOf(name: string): Promise<readonly string[] | null> {
const def = (await objectView.getObject(name)) as ObjectDef | undefined;
if (!def) return null;
return Object.entries(def.fields ?? {})
.filter(([, f]) => declaresOptions(f))
.map(([key]) => key);
},

async query(object, opts) {
const context = await resolvePrincipal();
await enforceApiExposure(reads, object, GATED_ACTIONS.query, context);
Expand Down
Loading
Loading