Skip to content

Commit 6c4c43a

Browse files
fix(service-automation,mcp): type a time-relative flow's conditions against the object its sweep scans (#30453)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgJQxgN9AS3TCTXHT5xvkdBN)_ domain:server#2 · os-bill · session_01Utp6CbRQNg8jRbhpBF2rFY Fixes #30396 Clause-②: no — a logged registration advisory (never thrown, never a refusal) now types a time-relative flow's conditions against the object the run binds, and the MCP `validate_expression` prose stops presenting a start the publish door refuses as a supported shape; the tool's input schema and verdicts are unchanged. Metadata-protocol: none ## 维护者速读 - 改了什么:时间相对(定时扫描)流程注册时的字段提示,现在按真正被扫描的对象(`timeRelative.object`)检查条件,不再按开始节点的 `objectName`;MCP `validate_expression` 的说明和 SKILL.md 改为「把被扫描对象作为 objectName 传入,开始节点写别的对象会在发布时被拦(`flow-time-relative-object-mismatch`)」。 - 为什么改:实测旧行为两头都错:发布规则上线前存下的两对象流程,注册日志对被扫描对象的正确字段报 unknown field,对运行时必失败的字段一声不吭;只写 `timeRelative.object` 不写 `objectName` 的流程完全不做字段检查。MCP 文案还在教 agent 写一种现在发布会被拒的流程。 - 风险与回滚:注册这一步只是日志提示,从不抛错、不拦注册;对象取自触发绑定本身(`deriveTriggerBinding`),与运行时同一答案;非时间相对流程不变;MCP 只改文字,输入 schema 与判决不变。回滚即撤 PR。 - 先看:`packages/services/service-automation/src/flow-registration-checks.ts` 的 `validateFlowExpressions` 取对象那几行;`engine.ts` 宿主里新增的一行 `deriveTriggerBinding`;`packages/mcp/src/mcp-http-tools.ts` 的 `sweepObject` 说明。 - 两分钟验证:`pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 src/flow-registration-checks.test.ts`,预期 4 个用例通过。 - 需要你做的:无,席位落地。 ## What changes - **Registration advisory (`flow-registration-checks.ts`, `validateFlowExpressions`).** The schema-aware pass (#1928: logged `unknown field` / typo / type-mismatch warnings, never thrown) took its object from the start node's `config.objectName`. It now takes it from the flow's trigger binding when that binding is `time_relative`: `binding.object`, which `deriveTriggerBinding` resolves as `timeRelative.object` when it is a string, `config.objectName` only otherwise — the object whose row the sweep hands each run. The rule is reused, not restated: the pass calls the engine's own `deriveTriggerBinding` (the same method `validateApiTriggerSecret` already calls at registration). Every other flow keeps `config.objectName`. The binding is derived only on the advisory pass with a resolver wired; the stored pass (`advisory: false`) never reads the hint. - **Engine host (`engine.ts`).** One line: the `validateFlowExpressions` host now passes `deriveTriggerBinding`, plus a sentence in that method's docblock. Outside the claim's expected file list (not a banned path); see Acceptance notes. - **MCP text (`mcp-http-tools.ts`, `skill-md.ts`).** `validate_expression`'s description, its `sweepObject` input text and the SKILL.md line no longer advise passing a `sweepObject` other than `objectName`. They say a time-relative flow's conditions read rows of the swept object, so pass it as `objectName`, and that a start whose `objectName` names another object is refused at publish (`flow-time-relative-object-mismatch`). The `sweepObject` text keeps what the tool answers when the two differ (the gate's advisory type-mismatch warning). No schema change: the other-object arm still mirrors what `@objectstack/lint` grades for such a flow's expressions, so retiring it would move the tool off the gate. - **Tests (`flow-registration-checks.test.ts`, new).** On a stored two-object flow (start `objectName: crm_account`, sweep `crm_contract`): a field of the swept object passes; a field only `crm_account` has warns `unknown field`. A one-object flow is unchanged (clean, and a typo still warns). A sweep-only flow (no `objectName`) is typed against the swept object. - **Changeset** `.changeset/30396-time-relative-condition-object.md`: `patch` on `@objectstack/service-automation` and `@objectstack/mcp`. ## Acceptance notes - **Measurement (on `09720e2c`, before the fix).** The four tests above against the old code: 3 red. The two-object flow logged `[flow 'renewal_alert'] node 'check' (decision) condition: unknown field \`renewal_status\` on \`crm_account\`` for the swept object's real field, and nothing for `record.industry` (a field that fails every run); the sweep-only flow logged nothing for `record.industry` because no object was typed at all. The one-object case was already green. - **Behaviour widening, advisory only.** A sweep-only time-relative flow (start names no `objectName`) now gets the schema advisory where it got none. It is a log line at registration; nothing is refused. - **File surface.** `engine.ts` is outside the claim's expected list: `validateFlowExpressions` receives its host members from the engine (`RegistrationCheckHost` Pick), so reusing the binding means one wiring line there. The alternatives were a third copy of the binding rule beside `deriveTriggerBinding` and `@objectstack/lint`'s, which the order rules out, or calling `deriveTriggerBinding` with a stand-in host. - **Text pins.** The package pins no tool-description text (`skill-md-surface-guard.test.ts` checks tool names only), and hint text is not pinned by convention; no text pin added. - **Restatements elsewhere.** `git grep sweepObject` over `skills/`, `content/`, `docs/`, `apps/`, `ui/`: no hit; no skill page restates the MCP advice. ## Verification Readings on head `9fe18676` (`fa901c10` the change, `35374475` merged with `main` at `fdd06415`, `9fe18676` a comment-only edit) unless named. <details><summary>Build, typecheck, tests</summary> - Build: `pnpm --filter '@objectstack/service-automation^...' --filter '@objectstack/mcp^...' build` under the lock, VERDICT command-exit 0; later `pnpm --filter '@objectstack/dogfood^...' build` on `35374475`, exit 0. - `@objectstack/service-automation`: `pnpm test` Test Files 264/264, Tests 3418/3418 (on `fa901c10`); `pnpm typecheck` 0. - `@objectstack/mcp`: `pnpm test` Test Files 56/56, Tests 678/678 (on `fa901c10`); `pnpm typecheck` 0. - `pnpm lint` (`eslint . --no-inline-config`) on `9fe18676`: exit 0. </details> <details><summary>Ablation (scripts/ablation-replace.mjs; the suite imports `src`, no dist leg)</summary> - The binding read gated off, so the pass types against `config.objectName` again (`const objectName = 'ABLATION_30396' && bound?.triggerType === 'never'`): mutation landed (anchor 1→0, blob `c53973cf` → `488218fa`); 3 red, among them `a stored two-object flow: a field of the swept object passes` with `+ "[flow 'renewal_alert'] node 'check' (decision) condition: unknown field \`renewal_status\` on \`crm_account\`"`; restored, blob == HEAD, `git diff HEAD` empty. </details> <details><summary>Derived gates</summary> - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on `9fe18676`: 62 families (the same list as on `35374475`), all 62 exit 0 (exit code captured per line before any pipe). - One was not measured on the first pass and is recorded at its re-run: `check:dual-build-cjs-loads` (PREREQUISITE NOT MET: seven packages outside the dogfood closure had no `dist/`; built under the lock, then exit 0). `check-plugin-teardown-shape --self-test`'s pinned fixture commit was fetched into this shallow clone before the run. - `--ran` reconciliation: "62 derived famil(ies) accounted for — 62 run, 0 NOT-MEASURED (a DERIVED zero …)". </details> <details><summary>Dogfood (full battery, run as the order asks; no narrowing arm)</summary> - `pnpm --filter '@objectstack/dogfood^...' build && pnpm --filter @objectstack/dogfood test` under the lock, started on `35374475` (no shard, no path): Test Files 419 passed, 3 skipped (422); Tests 3492 passed, 36 skipped (3528); VERDICT command-exit 0. `9fe18676` changes only a code comment in `flow-registration-checks.ts`, written while that build was running. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Utp6CbRQNg8jRbhpBF2rFY --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent eea2603 commit 6c4c43a

6 files changed

Lines changed: 131 additions & 16 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
'@objectstack/mcp': patch
4+
---
5+
6+
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
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
- **`@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.
12+
- **`@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.

‎packages/mcp/src/mcp-http-tools.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,12 +1099,13 @@ export function registerObjectTools(
10991099
'build-time errors (bare field refs, unknown fields, unknown functions) and advisory warnings ' +
11001100
'(text/boolean fields misused in arithmetic, an error at a `validation` site, worded for the rule\'s ' +
11011101
'`severity`, and at a `flow_condition` of a record-triggered or time-relative flow, where it fails the run; ' +
1102-
'a warning on any other flow, a time-relative sweep of another object included; date-equality pitfalls), plus the fields and stdlib ' +
1102+
'a warning on any other flow; date-equality pitfalls), plus the fields and stdlib ' +
11031103
'functions in scope so you can self-correct. `site` says where the expression will live: a `formula` ' +
11041104
'field, a `validation`/predicate, or a `flow_condition` (fields are bound bare in flow conditions). ' +
1105-
'Pass the rule\'s `severity` (validation) or the flow\'s `triggerKind` (flow_condition), and a time-relative ' +
1106-
'flow\'s `sweepObject` when it sweeps another object than objectName, so the verdict is the one the publish ' +
1107-
'gate gives that rule or flow.',
1105+
'Pass the rule\'s `severity` (validation) or the flow\'s `triggerKind` (flow_condition), so the verdict is ' +
1106+
'the one the publish gate gives that rule or flow. A time-relative flow\'s conditions read a row of the object ' +
1107+
'its sweep scans (`timeRelative.object`): pass that object as objectName. A start whose objectName names ' +
1108+
'another object is refused at publish (`flow-time-relative-object-mismatch`).',
11081109
inputSchema: strictToolInput(
11091110
{
11101111
surface: 'this validate_expression call',
@@ -1145,10 +1146,11 @@ export function registerObjectTools(
11451146
.string()
11461147
.optional()
11471148
.describe(
1148-
'site flow_condition with triggerKind time_relative only: the flow\'s `timeRelative.object`, whose rows ' +
1149-
'the sweep hands each run as its record. objectName is the object the conditions are typed against: the ' +
1150-
'start node\'s objectName, else the swept object itself. When sweepObject is another object, a type ' +
1151-
'mismatch is a warning, not an error: the run\'s record is a row of sweepObject. Default: objectName.',
1149+
'site flow_condition with triggerKind time_relative only: the flow\'s `timeRelative.object`, the object ' +
1150+
'the sweep scans and whose rows it hands each run as its record. Its conditions are typed against that ' +
1151+
'object, so it is objectName too. A start whose objectName names another object is refused at publish ' +
1152+
'(`flow-time-relative-object-mismatch`); passed with another objectName, the verdict is the gate\'s ' +
1153+
'advisory one for such a flow, a type mismatch warning. Default: objectName.',
11521154
),
11531155
},
11541156
),

‎packages/mcp/src/skill-md.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,8 +275,10 @@ create/update payload.
275275
\`flow_condition\` | \`template\` (default \`formula\`). Pass the rule's
276276
\`severity\` or the flow's \`triggerKind\` (\`record_change\` by default) to
277277
get the verdict the publish gate gives that rule or flow; for a
278-
\`time_relative\` flow that sweeps another object than \`objectName\`, also
279-
pass its \`timeRelative.object\` as \`sweepObject\` (then a mismatch warns).
278+
\`time_relative\` flow, pass the object its sweep scans (\`timeRelative.object\`)
279+
as \`objectName\`: its conditions read rows of that object, and a start whose
280+
\`objectName\` names another object is refused at publish
281+
(\`flow-time-relative-object-mismatch\`). \`sweepObject\` names that same object.
280282
- **query_records({ objectName, where?, fields?, limit?, offset?, orderBy? })** —
281283
read records. \`where\` is a field→value match, e.g. \`{ "status": "open" }\`.
282284
Results are page-capped; use \`limit\`/\`offset\` to page.

‎packages/services/service-automation/src/engine.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9003,6 +9003,9 @@ export class AutomationEngine implements IAutomationService {
90039003
* (unknown-field refs, likely bare-field typos, text/boolean fields misused
90049004
* in arithmetic) as ADVISORY warnings (logged, never thrown). Flow conditions
90059005
* bind the record's fields flat, so the schema pass uses `flattened` scope.
9006+
* The object they are typed against is the one the run binds: a
9007+
* time-relative flow's swept object, read off {@link deriveTriggerBinding}
9008+
* (#30396).
90069009
* The fatal set is unchanged — a resolver can never break a flow that used to
90079010
* register cleanly. `advisory: false` skips that pass: the stored pass
90089011
* previews refusals ({@link registrationCheckRefusals}), and an advisory
@@ -9022,6 +9025,7 @@ export class AutomationEngine implements IAutomationService {
90229025
logger: this.logger,
90239026
objectSchemaResolver: this.objectSchemaResolver,
90249027
valueEnvelopeRefusals: (...a) => this.valueEnvelopeRefusals(...a),
9028+
deriveTriggerBinding: (...a) => this.deriveTriggerBinding(...a),
90259029
};
90269030
return validateFlowExpressions.call(host, flowName, flow, { advisory });
90279031
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { AutomationEngine } from './engine.js';
5+
6+
/**
7+
* The registration pass's advisory schema check (#1928) types a flow's
8+
* conditions against ONE object. For a time-relative flow that object is the
9+
* one the run binds — `config.timeRelative.object` when it is a string, the
10+
* start node's `config.objectName` only otherwise (`deriveTriggerBinding`) —
11+
* because the sweep launches each run with a row of THAT object as its record
12+
* (#30396). A start whose `objectName` names another object is refused at
13+
* publish (`flow-time-relative-object-mismatch`), but a flow stored before
14+
* that rule still registers, so the advisory must point at the scanned object.
15+
*/
16+
function loggerCapturing(warns: string[]) {
17+
const l: any = {
18+
info: () => {}, error: () => {}, debug: () => {},
19+
warn: (m: string) => warns.push(m),
20+
child: () => l,
21+
};
22+
return l;
23+
}
24+
25+
const SCHEMAS: Record<string, { fields: string[]; fieldTypes: Record<string, string> }> = {
26+
// The start node's objectName in the two-object flow — NOT what the run binds.
27+
crm_account: { fields: ['industry'], fieldTypes: { industry: 'text' } },
28+
// The swept object: every run's record is one of its rows.
29+
crm_contract: { fields: ['end_date', 'renewal_status'], fieldTypes: { end_date: 'date', renewal_status: 'select' } },
30+
};
31+
32+
function timeRelativeFlow(condition: string, startObject: string | undefined) {
33+
return {
34+
name: 'renewal_alert', label: 'Renewal Alert', type: 'schedule',
35+
nodes: [
36+
{
37+
id: 'start', type: 'start', label: 'Start',
38+
config: {
39+
...(startObject ? { objectName: startObject } : {}),
40+
timeRelative: { object: 'crm_contract', dateField: 'end_date', offsetDays: [30] },
41+
schedule: { type: 'cron', expression: '0 8 * * *' },
42+
},
43+
},
44+
{ id: 'check', type: 'decision', label: 'Check', config: { condition } },
45+
{ id: 'end', type: 'end', label: 'End' },
46+
],
47+
edges: [
48+
{ id: 'e1', source: 'start', target: 'check' },
49+
{ id: 'e2', source: 'check', target: 'end' },
50+
],
51+
};
52+
}
53+
54+
function register(flow: unknown): string[] {
55+
const warns: string[] = [];
56+
const engine = new AutomationEngine(loggerCapturing(warns));
57+
engine.setObjectSchemaResolver((name) => SCHEMAS[name]);
58+
expect(() => engine.registerFlow('renewal_alert', flow as never)).not.toThrow();
59+
return warns.filter((m) => /unknown field|did you mean|type mismatch/i.test(m));
60+
}
61+
62+
describe('registration advisory: a time-relative flow is typed against the swept object (#30396)', () => {
63+
it('a stored two-object flow: a field of the swept object passes', () => {
64+
expect(register(timeRelativeFlow('record.renewal_status == "pending"', 'crm_account'))).toEqual([]);
65+
});
66+
67+
it('a stored two-object flow: a field only the start node\'s objectName has warns', () => {
68+
const warns = register(timeRelativeFlow('record.industry == "tech"', 'crm_account'));
69+
expect(warns).toHaveLength(1);
70+
expect(warns[0]).toMatch(/unknown field `industry`/);
71+
});
72+
73+
it('a one-object flow (objectName equals timeRelative.object) is unchanged', () => {
74+
expect(register(timeRelativeFlow('record.renewal_status == "pending"', 'crm_contract'))).toEqual([]);
75+
expect(register(timeRelativeFlow('record.renewl_status == "pending"', 'crm_contract')).join('\n'))
76+
.toMatch(/unknown field `renewl_status`/);
77+
});
78+
79+
it('a sweep-only flow (no objectName) is typed against the swept object', () => {
80+
expect(register(timeRelativeFlow('record.renewal_status == "pending"', undefined))).toEqual([]);
81+
expect(register(timeRelativeFlow('record.industry == "tech"', undefined)).join('\n'))
82+
.toMatch(/unknown field `industry`/);
83+
});
84+
});

‎packages/services/service-automation/src/flow-registration-checks.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ export function collectUndeclaredConfigKeys(
435435

436436
/** The body of {@link AutomationEngine.validateFlowExpressions}, moved unedited; that method's docblock is its contract. */
437437
export function validateFlowExpressions(
438-
this: Pick<RegistrationCheckHost, 'logger' | 'objectSchemaResolver' | 'valueEnvelopeRefusals'>,
438+
this: Pick<RegistrationCheckHost, 'logger' | 'objectSchemaResolver' | 'valueEnvelopeRefusals' | 'deriveTriggerBinding'>,
439439
flowName: string,
440440
flow: FlowParsed,
441441
{ advisory }: { advisory: boolean },
@@ -448,12 +448,23 @@ export function validateFlowExpressions(
448448
sites.push({ path, from: JSON.stringify(String(source)), detail: `${where}: ${message}` });
449449
};
450450

451-
// Resolve the flow's record-change target object (start node's
452-
// `config.objectName`) so `record.*`/bare field refs can be checked
453-
// against the real schema. Absent resolver / non-record flow → the hint
454-
// is undefined and only the fatal syntax/bare-ref pass runs (unchanged).
451+
// Resolve the object whose row each run binds as its record, so
452+
// `record.*`/bare field refs can be checked against the real schema: the
453+
// start node's `config.objectName`, except on a time-relative flow, whose
454+
// run binds the object its sweep scans (#30396). That answer is read off
455+
// the trigger binding itself (`deriveTriggerBinding`: `timeRelative.object`
456+
// when it is a string, `config.objectName` only otherwise), not restated
457+
// here — the readiness rule `flow-time-relative-object-mismatch` refuses a
458+
// start naming another object at publish, but a flow stored before it
459+
// still registers, and its advisory must speak of the scanned object.
460+
// Absent resolver / no object → the hint is undefined and only the fatal
461+
// syntax/bare-ref pass runs (unchanged); the stored pass (`advisory:
462+
// false`) never reads the hint, so it derives no binding.
455463
const startNode = flow.nodes.find((n) => n.type === 'start');
456-
const objectName = ((startNode?.config ?? {}) as Record<string, unknown>).objectName;
464+
const bound = advisory && this.objectSchemaResolver ? this.deriveTriggerBinding(flowName, flow) : undefined;
465+
const objectName = bound?.triggerType === 'time_relative'
466+
? bound.binding.object
467+
: ((startNode?.config ?? {}) as Record<string, unknown>).objectName;
457468
const schemaHint = typeof objectName === 'string'
458469
? (() => {
459470
const s = this.objectSchemaResolver?.(objectName);

0 commit comments

Comments
 (0)