From fa901c10826472aff3e400e6e52b9bbd6cbe0f3a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 11:00:26 +0000 Subject: [PATCH 1/2] fix(service-automation,mcp): type a time-relative flow's conditions against the swept object The registration advisory read the start node's objectName; the run binds timeRelative.object. Read it off deriveTriggerBinding instead, and reword the validate_expression sweepObject advice to name the publish refusal. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CbRQNg8jRbhpBF2rFY --- .../30396-time-relative-condition-object.md | 12 +++ packages/mcp/src/mcp-http-tools.ts | 18 ++-- packages/mcp/src/skill-md.ts | 6 +- .../services/service-automation/src/engine.ts | 4 + .../src/flow-registration-checks.test.ts | 84 +++++++++++++++++++ .../src/flow-registration-checks.ts | 22 +++-- 6 files changed, 130 insertions(+), 16 deletions(-) create mode 100644 .changeset/30396-time-relative-condition-object.md create mode 100644 packages/services/service-automation/src/flow-registration-checks.test.ts diff --git a/.changeset/30396-time-relative-condition-object.md b/.changeset/30396-time-relative-condition-object.md new file mode 100644 index 00000000000..5041eaae8c0 --- /dev/null +++ b/.changeset/30396-time-relative-condition-object.md @@ -0,0 +1,12 @@ +--- +'@objectstack/service-automation': patch +'@objectstack/mcp': patch +--- + +fix(service-automation,mcp): a time-relative flow's conditions are typed against the object its sweep scans, at registration and in the `validate_expression` advice + +Clause-②: no +Metadata-protocol: none + +- **`@objectstack/service-automation`**: the registration pass's advisory schema check (the logged `unknown field` / typo / type-mismatch warnings) typed a time-relative flow's conditions against the start node's `config.objectName`, while the run binds a row of `config.timeRelative.object`. A flow stored with a start naming another object kept a warning on a field that is right for the scanned object and none on a field that fails at run time; a start naming no `objectName` got no schema check at all. The pass now reads the object off the flow's trigger binding, the same answer the run uses: `timeRelative.object` when it is a string, `config.objectName` only otherwise. Every other flow is typed as before. The warnings stay logged, never thrown. +- **`@objectstack/mcp`**: `validate_expression`'s description, its `sweepObject` input text and the SKILL.md line no longer advise passing a `sweepObject` other than `objectName` as a supported shape. They say to pass the scanned object as `objectName`, and that a start whose `objectName` names another object is refused at publish (`flow-time-relative-object-mismatch`). The tool's inputs and verdicts are unchanged. diff --git a/packages/mcp/src/mcp-http-tools.ts b/packages/mcp/src/mcp-http-tools.ts index d6592578d35..4c7ae858f77 100644 --- a/packages/mcp/src/mcp-http-tools.ts +++ b/packages/mcp/src/mcp-http-tools.ts @@ -1099,12 +1099,13 @@ export function registerObjectTools( 'build-time errors (bare field refs, unknown fields, unknown functions) and advisory warnings ' + '(text/boolean fields misused in arithmetic, an error at a `validation` site, worded for the rule\'s ' + '`severity`, and at a `flow_condition` of a record-triggered or time-relative flow, where it fails the run; ' + - 'a warning on any other flow, a time-relative sweep of another object included; date-equality pitfalls), plus the fields and stdlib ' + + 'a warning on any other flow; date-equality pitfalls), plus the fields and stdlib ' + 'functions in scope so you can self-correct. `site` says where the expression will live: a `formula` ' + 'field, a `validation`/predicate, or a `flow_condition` (fields are bound bare in flow conditions). ' + - 'Pass the rule\'s `severity` (validation) or the flow\'s `triggerKind` (flow_condition), and a time-relative ' + - 'flow\'s `sweepObject` when it sweeps another object than objectName, so the verdict is the one the publish ' + - 'gate gives that rule or flow.', + 'Pass the rule\'s `severity` (validation) or the flow\'s `triggerKind` (flow_condition), so the verdict is ' + + 'the one the publish gate gives that rule or flow. A time-relative flow\'s conditions read a row of the object ' + + 'its sweep scans (`timeRelative.object`): pass that object as objectName. A start whose objectName names ' + + 'another object is refused at publish (`flow-time-relative-object-mismatch`).', inputSchema: strictToolInput( { surface: 'this validate_expression call', @@ -1145,10 +1146,11 @@ export function registerObjectTools( .string() .optional() .describe( - 'site flow_condition with triggerKind time_relative only: the flow\'s `timeRelative.object`, whose rows ' + - 'the sweep hands each run as its record. objectName is the object the conditions are typed against: the ' + - 'start node\'s objectName, else the swept object itself. When sweepObject is another object, a type ' + - 'mismatch is a warning, not an error: the run\'s record is a row of sweepObject. Default: objectName.', + 'site flow_condition with triggerKind time_relative only: the flow\'s `timeRelative.object`, the object ' + + 'the sweep scans and whose rows it hands each run as its record. Its conditions are typed against that ' + + 'object, so it is objectName too. A start whose objectName names another object is refused at publish ' + + '(`flow-time-relative-object-mismatch`); passed with another objectName, the verdict is the gate\'s ' + + 'advisory one for such a flow, a type mismatch warning. Default: objectName.', ), }, ), diff --git a/packages/mcp/src/skill-md.ts b/packages/mcp/src/skill-md.ts index 5b1d1afc524..bcefe9b5ec8 100644 --- a/packages/mcp/src/skill-md.ts +++ b/packages/mcp/src/skill-md.ts @@ -275,8 +275,10 @@ create/update payload. \`flow_condition\` | \`template\` (default \`formula\`). Pass the rule's \`severity\` or the flow's \`triggerKind\` (\`record_change\` by default) to get the verdict the publish gate gives that rule or flow; for a - \`time_relative\` flow that sweeps another object than \`objectName\`, also - pass its \`timeRelative.object\` as \`sweepObject\` (then a mismatch warns). + \`time_relative\` flow, pass the object its sweep scans (\`timeRelative.object\`) + as \`objectName\`: its conditions read rows of that object, and a start whose + \`objectName\` names another object is refused at publish + (\`flow-time-relative-object-mismatch\`). \`sweepObject\` names that same object. - **query_records({ objectName, where?, fields?, limit?, offset?, orderBy? })** — read records. \`where\` is a field→value match, e.g. \`{ "status": "open" }\`. Results are page-capped; use \`limit\`/\`offset\` to page. diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index c937469c883..660a2912efa 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -9003,6 +9003,9 @@ export class AutomationEngine implements IAutomationService { * (unknown-field refs, likely bare-field typos, text/boolean fields misused * in arithmetic) as ADVISORY warnings (logged, never thrown). Flow conditions * bind the record's fields flat, so the schema pass uses `flattened` scope. + * The object they are typed against is the one the run binds: a + * time-relative flow's swept object, read off {@link deriveTriggerBinding} + * (#30396). * The fatal set is unchanged — a resolver can never break a flow that used to * register cleanly. `advisory: false` skips that pass: the stored pass * previews refusals ({@link registrationCheckRefusals}), and an advisory @@ -9022,6 +9025,7 @@ export class AutomationEngine implements IAutomationService { logger: this.logger, objectSchemaResolver: this.objectSchemaResolver, valueEnvelopeRefusals: (...a) => this.valueEnvelopeRefusals(...a), + deriveTriggerBinding: (...a) => this.deriveTriggerBinding(...a), }; return validateFlowExpressions.call(host, flowName, flow, { advisory }); } diff --git a/packages/services/service-automation/src/flow-registration-checks.test.ts b/packages/services/service-automation/src/flow-registration-checks.test.ts new file mode 100644 index 00000000000..3589c9a8f47 --- /dev/null +++ b/packages/services/service-automation/src/flow-registration-checks.test.ts @@ -0,0 +1,84 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { AutomationEngine } from './engine.js'; + +/** + * The registration pass's advisory schema check (#1928) types a flow's + * conditions against ONE object. For a time-relative flow that object is the + * one the run binds — `config.timeRelative.object` when it is a string, the + * start node's `config.objectName` only otherwise (`deriveTriggerBinding`) — + * because the sweep launches each run with a row of THAT object as its record + * (#30396). A start whose `objectName` names another object is refused at + * publish (`flow-time-relative-object-mismatch`), but a flow stored before + * that rule still registers, so the advisory must point at the scanned object. + */ +function loggerCapturing(warns: string[]) { + const l: any = { + info: () => {}, error: () => {}, debug: () => {}, + warn: (m: string) => warns.push(m), + child: () => l, + }; + return l; +} + +const SCHEMAS: Record }> = { + // The start node's objectName in the two-object flow — NOT what the run binds. + crm_account: { fields: ['industry'], fieldTypes: { industry: 'text' } }, + // The swept object: every run's record is one of its rows. + crm_contract: { fields: ['end_date', 'renewal_status'], fieldTypes: { end_date: 'date', renewal_status: 'select' } }, +}; + +function timeRelativeFlow(condition: string, startObject: string | undefined) { + return { + name: 'renewal_alert', label: 'Renewal Alert', type: 'schedule', + nodes: [ + { + id: 'start', type: 'start', label: 'Start', + config: { + ...(startObject ? { objectName: startObject } : {}), + timeRelative: { object: 'crm_contract', dateField: 'end_date', offsetDays: [30] }, + schedule: { type: 'cron', expression: '0 8 * * *' }, + }, + }, + { id: 'check', type: 'decision', label: 'Check', config: { condition } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'check' }, + { id: 'e2', source: 'check', target: 'end' }, + ], + }; +} + +function register(flow: unknown): string[] { + const warns: string[] = []; + const engine = new AutomationEngine(loggerCapturing(warns)); + engine.setObjectSchemaResolver((name) => SCHEMAS[name]); + expect(() => engine.registerFlow('renewal_alert', flow as never)).not.toThrow(); + return warns.filter((m) => /unknown field|did you mean|type mismatch/i.test(m)); +} + +describe('registration advisory: a time-relative flow is typed against the swept object (#30396)', () => { + it('a stored two-object flow: a field of the swept object passes', () => { + expect(register(timeRelativeFlow('record.renewal_status == "pending"', 'crm_account'))).toEqual([]); + }); + + it('a stored two-object flow: a field only the start node\'s objectName has warns', () => { + const warns = register(timeRelativeFlow('record.industry == "tech"', 'crm_account')); + expect(warns).toHaveLength(1); + expect(warns[0]).toMatch(/unknown field `industry`/); + }); + + it('a one-object flow (objectName equals timeRelative.object) is unchanged', () => { + expect(register(timeRelativeFlow('record.renewal_status == "pending"', 'crm_contract'))).toEqual([]); + expect(register(timeRelativeFlow('record.renewl_status == "pending"', 'crm_contract')).join('\n')) + .toMatch(/unknown field `renewl_status`/); + }); + + it('a sweep-only flow (no objectName) is typed against the swept object', () => { + expect(register(timeRelativeFlow('record.renewal_status == "pending"', undefined))).toEqual([]); + expect(register(timeRelativeFlow('record.industry == "tech"', undefined)).join('\n')) + .toMatch(/unknown field `industry`/); + }); +}); diff --git a/packages/services/service-automation/src/flow-registration-checks.ts b/packages/services/service-automation/src/flow-registration-checks.ts index 7203a69543b..c53973cf905 100644 --- a/packages/services/service-automation/src/flow-registration-checks.ts +++ b/packages/services/service-automation/src/flow-registration-checks.ts @@ -435,7 +435,7 @@ export function collectUndeclaredConfigKeys( /** The body of {@link AutomationEngine.validateFlowExpressions}, moved unedited; that method's docblock is its contract. */ export function validateFlowExpressions( - this: Pick, + this: Pick, flowName: string, flow: FlowParsed, { advisory }: { advisory: boolean }, @@ -448,12 +448,22 @@ export function validateFlowExpressions( sites.push({ path, from: JSON.stringify(String(source)), detail: `${where}: ${message}` }); }; - // Resolve the flow's record-change target object (start node's - // `config.objectName`) so `record.*`/bare field refs can be checked - // against the real schema. Absent resolver / non-record flow → the hint - // is undefined and only the fatal syntax/bare-ref pass runs (unchanged). + // Resolve the object whose row each run binds as its record, so + // `record.*`/bare field refs can be checked against the real schema: the + // start node's `config.objectName`, except on a time-relative flow, whose + // run binds the object its sweep scans (#30396). That answer is read off + // the trigger binding itself (`deriveTriggerBinding`: `timeRelative.object` + // when it is a string, `config.objectName` only otherwise), not restated + // here — the readiness rule `flow-time-relative-object-mismatch` refuses a + // start naming another object at publish, but a flow stored before it + // still registers, and its advisory must speak of the scanned object. + // Absent resolver / no object / the stored pass (`advisory: false`) → the + // hint is undefined and only the fatal syntax/bare-ref pass runs. const startNode = flow.nodes.find((n) => n.type === 'start'); - const objectName = ((startNode?.config ?? {}) as Record).objectName; + const bound = advisory && this.objectSchemaResolver ? this.deriveTriggerBinding(flowName, flow) : undefined; + const objectName = bound?.triggerType === 'time_relative' + ? bound.binding.object + : ((startNode?.config ?? {}) as Record).objectName; const schemaHint = typeof objectName === 'string' ? (() => { const s = this.objectSchemaResolver?.(objectName); From 9fe18676371e491a5b842d595750eee0a645cf53 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 11:08:21 +0000 Subject: [PATCH 2/2] docs(service-automation): say why the stored pass derives no binding Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CbRQNg8jRbhpBF2rFY --- .../service-automation/src/flow-registration-checks.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/services/service-automation/src/flow-registration-checks.ts b/packages/services/service-automation/src/flow-registration-checks.ts index c53973cf905..1fa77c4f508 100644 --- a/packages/services/service-automation/src/flow-registration-checks.ts +++ b/packages/services/service-automation/src/flow-registration-checks.ts @@ -457,8 +457,9 @@ export function validateFlowExpressions( // here — the readiness rule `flow-time-relative-object-mismatch` refuses a // start naming another object at publish, but a flow stored before it // still registers, and its advisory must speak of the scanned object. - // Absent resolver / no object / the stored pass (`advisory: false`) → the - // hint is undefined and only the fatal syntax/bare-ref pass runs. + // Absent resolver / no object → the hint is undefined and only the fatal + // syntax/bare-ref pass runs (unchanged); the stored pass (`advisory: + // false`) never reads the hint, so it derives no binding. const startNode = flow.nodes.find((n) => n.type === 'start'); const bound = advisory && this.objectSchemaResolver ? this.deriveTriggerBinding(flowName, flow) : undefined; const objectName = bound?.triggerType === 'time_relative'